From 13c9059f8d5132a07690e578274180303a492838 Mon Sep 17 00:00:00 2001 From: levkropp Date: Tue, 7 Jan 2025 16:38:58 -0500 Subject: [PATCH] Address code review * cpus is now a uint32 in multipass.proto * removed LinuxSystemInfo struct and replaced with two functions * get_cpus() and get_total_ram() are now platform functions with a linux implementation * configuring a VM's resources now uses daemonInfoProvider properly * cpu, ram, and disk sliders are no longer consumers --- include/multipass/platform.h | 2 + src/client/gui/lib/catalogue/launch_form.dart | 5 -- src/client/gui/lib/providers.dart | 4 ++ .../gui/lib/vm_details/cpus_slider.dart | 12 ++-- .../gui/lib/vm_details/disk_slider.dart | 25 ++----- src/client/gui/lib/vm_details/ram_slider.dart | 14 ++-- .../lib/vm_details/vm_details_resources.dart | 72 ++++++++++++------- src/daemon/daemon.cpp | 39 +--------- src/daemon/daemon.h | 13 +--- src/platform/platform_linux.cpp | 36 ++++++++++ src/rpc/multipass.proto | 2 +- 11 files changed, 109 insertions(+), 115 deletions(-) diff --git a/include/multipass/platform.h b/include/multipass/platform.h index a5c153d9de..68e98e5111 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -77,6 +77,8 @@ class Platform : public Singleton virtual QString default_privileged_mounts() const; virtual bool is_image_url_supported() const; [[nodiscard]] virtual std::string bridge_nomenclature() const; + virtual int get_cpus() const; + virtual long long get_total_ram() const; }; QString interpret_setting(const QString& key, const QString& val); diff --git a/src/client/gui/lib/catalogue/launch_form.dart b/src/client/gui/lib/catalogue/launch_form.dart index c4bca1b742..1f7f20e090 100644 --- a/src/client/gui/lib/catalogue/launch_form.dart +++ b/src/client/gui/lib/catalogue/launch_form.dart @@ -18,11 +18,6 @@ import '../vm_details/mount_points.dart'; import '../vm_details/ram_slider.dart'; import '../vm_details/spec_input.dart'; -// Define the daemonInfoProvider here -final daemonInfoProvider = FutureProvider.autoDispose((ref) { - return ref.watch(grpcClientProvider).daemonInfo(); -}); - final launchingImageProvider = StateProvider((_) => ImageInfo()); final randomNameProvider = Provider.autoDispose( diff --git a/src/client/gui/lib/providers.dart b/src/client/gui/lib/providers.dart index 0cfc0fb721..b01bec91e3 100644 --- a/src/client/gui/lib/providers.dart +++ b/src/client/gui/lib/providers.dart @@ -72,6 +72,10 @@ final daemonAvailableProvider = Provider((ref) { return false; }); +final daemonInfoProvider = FutureProvider((ref) { + return ref.watch(grpcClientProvider).daemonInfo(); +}); + class AllVmInfosNotifier extends Notifier> { @override List build() { diff --git a/src/client/gui/lib/vm_details/cpus_slider.dart b/src/client/gui/lib/vm_details/cpus_slider.dart index 9b428ca78f..03e2a19edc 100644 --- a/src/client/gui/lib/vm_details/cpus_slider.dart +++ b/src/client/gui/lib/vm_details/cpus_slider.dart @@ -2,28 +2,24 @@ import 'dart:math' as math; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -class CpusSlider extends ConsumerStatefulWidget { +class CpusSlider extends StatefulWidget { final int? initialValue; final FormFieldSetter onSaved; - final int maxCpus; const CpusSlider({ super.key, this.initialValue, required this.onSaved, - this.maxCpus=1, + this.maxCpus = 1, }); @override - ConsumerState createState() => _CpusSliderState(); + State createState() => _CpusSliderState(); } -class _CpusSliderState extends ConsumerState { - - +class _CpusSliderState extends State { final min = 1; late final controller = TextEditingController( diff --git a/src/client/gui/lib/vm_details/disk_slider.dart b/src/client/gui/lib/vm_details/disk_slider.dart index 915f720c56..fc9daa462e 100644 --- a/src/client/gui/lib/vm_details/disk_slider.dart +++ b/src/client/gui/lib/vm_details/disk_slider.dart @@ -1,38 +1,27 @@ import 'dart:math' as math; - import 'package:flutter/material.dart' hide Tooltip; -import 'package:flutter_riverpod/flutter_riverpod.dart'; - -import '../providers.dart'; -import '../tooltip.dart'; import 'mapping_slider.dart'; +import '../extensions.dart'; import 'memory_slider.dart'; +import '../tooltip.dart'; -final diskSizeProvider = FutureProvider((ref) { - return ref - .watch(grpcClientProvider) - .daemonInfo() - .then((r) => r.availableSpace.toInt()); -}); - -class DiskSlider extends ConsumerWidget { +class DiskSlider extends StatelessWidget { final int? initialValue; final int min; final FormFieldSetter onSaved; - final int maxDisk; - DiskSlider({ + const DiskSlider({ super.key, int? min, this.initialValue, required this.onSaved, - this.maxDisk=1, + this.maxDisk = 1, }) : min = min ?? 1.gibi; @override - Widget build(BuildContext context, WidgetRef ref) { - final disk = maxDisk ?? min; + Widget build(BuildContext context) { + final disk = maxDisk; final max = math.max(initialValue ?? 0, disk); final enabled = min != max; diff --git a/src/client/gui/lib/vm_details/ram_slider.dart b/src/client/gui/lib/vm_details/ram_slider.dart index 00efb12c6f..4a633be3e5 100644 --- a/src/client/gui/lib/vm_details/ram_slider.dart +++ b/src/client/gui/lib/vm_details/ram_slider.dart @@ -1,30 +1,26 @@ import 'dart:math' as math; import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'mapping_slider.dart'; +import '../extensions.dart'; import 'memory_slider.dart'; -class RamSlider extends ConsumerWidget { - - +class RamSlider extends StatelessWidget { final int? initialValue; final FormFieldSetter onSaved; - final int maxRam; const RamSlider({ super.key, this.initialValue, required this.onSaved, - this.maxRam=1, + this.maxRam = 1, }); @override - Widget build(BuildContext context, WidgetRef ref) { - - final ram = maxRam ?? 512.mebi; + Widget build(BuildContext context) { + final ram = maxRam; return MemorySlider( label: 'Memory', diff --git a/src/client/gui/lib/vm_details/vm_details_resources.dart b/src/client/gui/lib/vm_details/vm_details_resources.dart index f1f0e28535..3d74793341 100644 --- a/src/client/gui/lib/vm_details/vm_details_resources.dart +++ b/src/client/gui/lib/vm_details/vm_details_resources.dart @@ -45,6 +45,7 @@ class _ResourcesDetailsState extends ConsumerState { final stopped = ref.watch(vmInfoProvider(widget.name).select((info) { return info.instanceStatus.status == Status.STOPPED; })); + final daemonInfo = ref.watch(daemonInfoProvider); if (!stopped) editing = false; @@ -53,14 +54,21 @@ class _ResourcesDetailsState extends ConsumerState { 'CPUs ${cpus?.toString() ?? '…'}', style: TextStyle(fontSize: 16), ) - : CpusSlider( - key: Key('cpus-$cpus'), - initialValue: cpus, - onSaved: (value) { - if (value == null || value == cpus) return; - ref.read(cpusProvider.notifier).set('$value').onError( - ref.notifyError((error) => 'Failed to set CPUs : $error')); - }, + : daemonInfo.when( + data: (info) => CpusSlider( + key: Key('cpus-$cpus'), + initialValue: cpus, + maxCpus: info.cpus.toInt(), + onSaved: (value) { + if (value == null || value == cpus) return; + ref + .read(cpusProvider.notifier) + .set('$value') + .onError(ref.notifyError((error) => 'Failed to set CPUs : $error')); + }, + ), + loading: () => CircularProgressIndicator(), + error: (error, stack) => Text('Error: $error'), ); final ramResource = !editing @@ -68,14 +76,21 @@ class _ResourcesDetailsState extends ConsumerState { 'Memory ${ram.map(humanReadableMemory) ?? '…'}', style: TextStyle(fontSize: 16), ) - : RamSlider( - key: Key('ram-$ram'), - initialValue: ram, - onSaved: (value) { - if (value == null || value == ram) return; - ref.read(ramProvider.notifier).set('${value}B').onError( - ref.notifyError((e) => 'Failed to set memory size: $e')); - }, + : daemonInfo.when( + data: (info) => RamSlider( + key: Key('ram-$ram'), + initialValue: ram, + maxRam: info.memory.toInt(), + onSaved: (value) { + if (value == null || value == ram) return; + ref + .read(ramProvider.notifier) + .set('${value}B') + .onError(ref.notifyError((e) => 'Failed to set memory size: $e')); + }, + ), + loading: () => CircularProgressIndicator(), + error: (error, stack) => Text('Error: $error'), ); final diskResource = !editing @@ -83,15 +98,22 @@ class _ResourcesDetailsState extends ConsumerState { 'Disk ${disk.map(humanReadableMemory) ?? '…'}', style: TextStyle(fontSize: 16), ) - : DiskSlider( - key: Key('disk-$disk'), - min: disk, - initialValue: disk, - onSaved: (value) { - if (value == null || value == disk) return; - ref.read(diskProvider.notifier).set('${value}B').onError( - ref.notifyError((e) => 'Failed to set disk size: $e')); - }, + : daemonInfo.when( + data: (info) => DiskSlider( + key: Key('disk-$disk'), + min: disk, + initialValue: disk, + maxDisk: info.availableSpace.toInt(), + onSaved: (value) { + if (value == null || value == disk) return; + ref + .read(diskProvider.notifier) + .set('${value}B') + .onError(ref.notifyError((e) => 'Failed to set disk size: $e')); + }, + ), + loading: () => CircularProgressIndicator(), + error: (error, stack) => Text('Error: $error'), ); final saveButton = TextButton( diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index be7a830f2d..9675858258 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -2789,11 +2789,8 @@ try // clang-format on QStorageInfo storage_info{config->data_directory}; response.set_available_space(storage_info.bytesTotal()); -#if defined(__linux__) - auto sys_info = read_linux_system_info(); - response.set_cpus(sys_info.cpus); - response.set_memory(sys_info.memory); -#endif + response.set_cpus(MP_PLATFORM.get_cpus()); + response.set_memory(MP_PLATFORM.get_total_ram()); server->Write(response); status_promise->set_value(grpc::Status{}); @@ -3801,35 +3798,3 @@ void mp::Daemon::add_bridged_interface(const std::string& instance_name) throw mp::BridgeFailureException("Cannot update instance settings", instance_name, preferred_net); } } - -mp::LinuxSystemInfo mp::read_linux_system_info() -{ - LinuxSystemInfo info{}; -#if defined(__linux__) - // Simple parsing from /proc files: - { - std::ifstream cpuinfo{"/proc/cpuinfo"}; - std::string line; - while (std::getline(cpuinfo, line)) - { - if (line.rfind("processor", 0) == 0) - info.cpus++; - } - } - { - std::ifstream meminfo{"/proc/meminfo"}; - std::string line; - while (std::getline(meminfo, line)) - { - if (line.rfind("MemTotal:", 0) == 0) - { - long kb = 0; - std::sscanf(line.c_str(), "MemTotal: %ld kB", &kb); - info.memory = kb * 1024LL; - break; - } - } - } -#endif - return info; -} diff --git a/src/daemon/daemon.h b/src/daemon/daemon.h index 6e42613a34..57bf6a59eb 100644 --- a/src/daemon/daemon.h +++ b/src/daemon/daemon.h @@ -25,6 +25,7 @@ #include #include #include +#include // Add this include #include #include #include @@ -41,18 +42,6 @@ namespace multipass { -struct DaemonConfig; -class SettingsHandler; - -struct LinuxSystemInfo -{ - int cpus; - long long memory; - long long disk; -}; - -LinuxSystemInfo read_linux_system_info(); - class Daemon : public QObject, public multipass::VMStatusMonitor { Q_OBJECT diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index 97d7baaa5c..3531ca6947 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -62,6 +62,7 @@ #include #include #include +#include namespace mp = multipass; namespace mpl = multipass::logging; @@ -471,3 +472,38 @@ std::string multipass::platform::host_version() return mpu::in_multipass_snap() ? multipass::platform::detail::read_os_release() : fmt::format("{}-{}", QSysInfo::productType(), QSysInfo::productVersion()); } + +int mp::platform::Platform::get_cpus() const +{ + int cpus = 0; + std::ifstream cpuinfo{"/proc/cpuinfo"}; + std::string line; + + while (std::getline(cpuinfo, line)) + { + if (line.rfind("processor", 0) == 0) + cpus++; + } + + return cpus; +} + +long long mp::platform::Platform::get_total_ram() const +{ + long long memory = 0; + std::ifstream meminfo{"/proc/meminfo"}; + std::string line; + + while (std::getline(meminfo, line)) + { + if (line.rfind("MemTotal:", 0) == 0) + { + long kb = 0; + std::sscanf(line.c_str(), "MemTotal: %ld kB", &kb); + memory = kb * 1024LL; + break; + } + } + + return memory; +} diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index 2a97a7cf95..8ee6b9c2b1 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -547,6 +547,6 @@ message DaemonInfoRequest { message DaemonInfoReply { string log_line = 1; uint64 available_space = 2; - int32 cpus = 3; + uint32 cpus = 3; uint64 memory = 4; }