From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 95FDB1FF285
for <inbox@lore.proxmox.com>; Wed, 25 Jun 2025 18:00:59 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 51C9319A60;
Wed, 25 Jun 2025 17:58:41 +0200 (CEST)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Wed, 25 Jun 2025 17:56:37 +0200
Message-ID: <20250625155751.268047-15-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.47.2
In-Reply-To: <20250625155751.268047-1-f.ebner@proxmox.com>
References: <20250625155751.268047-1-f.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.030 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pve-devel] [PATCH qemu-server 14/31] move find_vmstate_storage()
helper to QemuConfig module
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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.ebner@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