Skip to content

Commit

Permalink
Address code review
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
levkropp committed Jan 7, 2025
1 parent d29c1fe commit 13c9059
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 115 deletions.
2 changes: 2 additions & 0 deletions include/multipass/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class Platform : public Singleton<Platform>
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);
Expand Down
5 changes: 0 additions & 5 deletions src/client/gui/lib/catalogue/launch_form.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<DaemonInfoReply>((ref) {
return ref.watch(grpcClientProvider).daemonInfo();
});

final launchingImageProvider = StateProvider<ImageInfo>((_) => ImageInfo());

final randomNameProvider = Provider.autoDispose(
Expand Down
4 changes: 4 additions & 0 deletions src/client/gui/lib/providers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ final daemonAvailableProvider = Provider((ref) {
return false;
});

final daemonInfoProvider = FutureProvider<DaemonInfoReply>((ref) {
return ref.watch(grpcClientProvider).daemonInfo();
});

class AllVmInfosNotifier extends Notifier<List<DetailedInfoItem>> {
@override
List<DetailedInfoItem> build() {
Expand Down
12 changes: 4 additions & 8 deletions src/client/gui/lib/vm_details/cpus_slider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> onSaved;

final int maxCpus;

const CpusSlider({
super.key,
this.initialValue,
required this.onSaved,
this.maxCpus=1,
this.maxCpus = 1,
});

@override
ConsumerState<CpusSlider> createState() => _CpusSliderState();
State<CpusSlider> createState() => _CpusSliderState();
}

class _CpusSliderState extends ConsumerState<CpusSlider> {


class _CpusSliderState extends State<CpusSlider> {
final min = 1;

late final controller = TextEditingController(
Expand Down
25 changes: 7 additions & 18 deletions src/client/gui/lib/vm_details/disk_slider.dart
Original file line number Diff line number Diff line change
@@ -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<int> 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;

Expand Down
14 changes: 5 additions & 9 deletions src/client/gui/lib/vm_details/ram_slider.dart
Original file line number Diff line number Diff line change
@@ -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<int> 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',
Expand Down
72 changes: 47 additions & 25 deletions src/client/gui/lib/vm_details/vm_details_resources.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class _ResourcesDetailsState extends ConsumerState<ResourcesDetails> {
final stopped = ref.watch(vmInfoProvider(widget.name).select((info) {
return info.instanceStatus.status == Status.STOPPED;
}));
final daemonInfo = ref.watch(daemonInfoProvider);

if (!stopped) editing = false;

Expand All @@ -53,45 +54,66 @@ class _ResourcesDetailsState extends ConsumerState<ResourcesDetails> {
'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
? Text(
'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
? Text(
'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(
Expand Down
39 changes: 2 additions & 37 deletions src/daemon/daemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down Expand Up @@ -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;
}
13 changes: 1 addition & 12 deletions src/daemon/daemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <multipass/delayed_shutdown_timer.h>
#include <multipass/format.h>
#include <multipass/mount_handler.h>
#include <multipass/settings/settings_handler.h> // Add this include
#include <multipass/virtual_machine.h>
#include <multipass/vm_specs.h>
#include <multipass/vm_status_monitor.h>
Expand All @@ -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
Expand Down
36 changes: 36 additions & 0 deletions src/platform/platform_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <fstream>

namespace mp = multipass;
namespace mpl = multipass::logging;
Expand Down Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion src/rpc/multipass.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 13c9059

Please sign in to comment.