While not the main motivation, this has the nice side effect of
removing a call from QemuConfig to the QemuServer main module.
This is in preparation to introduce a RunState module which does not
call back into the main QemuServer module. In particular, qm_suspend()
will be moved to RunState which needs to call find_vmstate_storage().
Intuitively, the StateFile module seems like the most natural place
for find_vmstate_storage(), but moving find_vmstate_storage() requires
moving foreach_storage_used_by_vm() too and that function calls into
QemuConfig. Now, QemuConfig also calls find_vmstate_storage(), meaning
a cyclic dependency would result.
Note that foreach_storage_used_by_vm(), is related to foreach_volume()
and also uses foreach_volume(), so QemuConfig is the natural place for
that function.
So the arguments for moving find_vmstate_storage() to QemuConfig are:
1. most natural way to avoid cylcic dependencies.
2. related function foreach_storage_used_by_vm() belongs there too.
3. qm_suspend() and other functions relating to the run state already
call other QemuConfig methods.
Signed-off-by: Fiona Ebner <f.eb...@proxmox.com>
---
src/PVE/API2/Qemu.pm | 2 +-
src/PVE/QemuConfig.pm | 52 ++++++++++++++++++++++++++++++++++++++++++-
src/PVE/QemuServer.pm | 52 +------------------------------------------
3 files changed, 53 insertions(+), 53 deletions(-)
diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 747edb62..7f55998e 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -3922,7 +3922,7 @@ __PACKAGE__->register_method({
if (!$statestorage) {
# get statestorage from config if none is given
my $storecfg = PVE::Storage::config();
- $statestorage = PVE::QemuServer::find_vmstate_storage($conf,
$storecfg);
+ $statestorage = PVE::QemuConfig::find_vmstate_storage($conf,
$storecfg);
}
$rpcenv->check($authuser, "/storage/$statestorage",
['Datastore.AllocateSpace']);
diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
index 500b4c0b..957c875a 100644
--- a/src/PVE/QemuConfig.pm
+++ b/src/PVE/QemuConfig.pm
@@ -221,7 +221,7 @@ sub __snapshot_save_vmstate {
my $target = $statestorage;
if (!$target) {
- $target = PVE::QemuServer::find_vmstate_storage($conf, $storecfg);
+ $target = find_vmstate_storage($conf, $storecfg);
}
my $mem_size = get_current_memory($conf->{memory});
@@ -712,4 +712,54 @@ sub cleanup_fleecing_images {
record_fleecing_images($vmid, $failed);
}
+sub foreach_storage_used_by_vm {
+ my ($conf, $func) = @_;
+
+ my $sidhash = {};
+
+ PVE::QemuConfig->foreach_volume(
+ $conf,
+ sub {
+ my ($ds, $drive) = @_;
+ return if PVE::QemuServer::Drive::drive_is_cdrom($drive);
+
+ my $volid = $drive->{file};
+
+ my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
+ $sidhash->{$sid} = $sid if $sid;
+ },
+ );
+
+ foreach my $sid (sort keys %$sidhash) {
+ &$func($sid);
+ }
+}
+
+# NOTE: if this logic changes, please update docs & possibly gui logic
+sub find_vmstate_storage {
+ my ($conf, $storecfg) = @_;
+
+ # first, return storage from conf if set
+ return $conf->{vmstatestorage} if $conf->{vmstatestorage};
+
+ my ($target, $shared, $local);
+
+ foreach_storage_used_by_vm(
+ $conf,
+ sub {
+ my ($sid) = @_;
+ my $scfg = PVE::Storage::storage_config($storecfg, $sid);
+ my $dst = $scfg->{shared} ? \$shared : \$local;
+ $$dst = $sid if !$$dst || $scfg->{path}; # prefer file based
storage
+ },
+ );
+
+ # second, use shared storage where VM has at least one disk
+ # third, use local storage where VM has at least one disk
+ # fall back to local storage
+ $target = $shared // $local // 'local';
+
+ return $target;
+}
+
1;
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 02dcc02c..d24dc7eb 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -6322,7 +6322,7 @@ sub vm_suspend {
my $date = strftime("%Y-%m-%d", localtime(time()));
$storecfg = PVE::Storage::config();
if (!$statestorage) {
- $statestorage = find_vmstate_storage($conf, $storecfg);
+ $statestorage =
PVE::QemuConfig::find_vmstate_storage($conf, $storecfg);
# check permissions for the storage
my $rpcenv = PVE::RPCEnvironment::get();
if ($rpcenv->{type} ne 'cli') {
@@ -7877,29 +7877,6 @@ sub restore_tar_archive {
warn $@ if $@;
}
-sub foreach_storage_used_by_vm {
- my ($conf, $func) = @_;
-
- my $sidhash = {};
-
- PVE::QemuConfig->foreach_volume(
- $conf,
- sub {
- my ($ds, $drive) = @_;
- return if drive_is_cdrom($drive);
-
- my $volid = $drive->{file};
-
- my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
- $sidhash->{$sid} = $sid if $sid;
- },
- );
-
- foreach my $sid (sort keys %$sidhash) {
- &$func($sid);
- }
-}
-
my $qemu_snap_storage = {
rbd => 1,
};
@@ -8558,33 +8535,6 @@ sub resolve_dst_disk_format {
return $format;
}
-# NOTE: if this logic changes, please update docs & possibly gui logic
-sub find_vmstate_storage {
- my ($conf, $storecfg) = @_;
-
- # first, return storage from conf if set
- return $conf->{vmstatestorage} if $conf->{vmstatestorage};
-
- my ($target, $shared, $local);
-
- foreach_storage_used_by_vm(
- $conf,
- sub {
- my ($sid) = @_;
- my $scfg = PVE::Storage::storage_config($storecfg, $sid);
- my $dst = $scfg->{shared} ? \$shared : \$local;
- $$dst = $sid if !$$dst || $scfg->{path}; # prefer file based
storage
- },
- );
-
- # second, use shared storage where VM has at least one disk
- # third, use local storage where VM has at least one disk
- # fall back to local storage
- $target = $shared // $local // 'local';
-
- return $target;
-}
-
sub generate_uuid {
my ($uuid, $uuid_str);
UUID::generate($uuid);
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
