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 411C51FF170 for <inbox@lore.proxmox.com>; Thu, 12 Jun 2025 16:04:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 08B341F50C; Thu, 12 Jun 2025 16:03:12 +0200 (CEST) From: Fiona Ebner <f.ebner@proxmox.com> To: pve-devel@lists.proxmox.com Date: Thu, 12 Jun 2025 16:02:46 +0200 Message-Id: <20250612140253.106555-16-f.ebner@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250612140253.106555-1-f.ebner@proxmox.com> References: <20250612140253.106555-1-f.ebner@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.032 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 15/22] vm start/commandline: activate volumes before config_to_command() 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> With '-blockdev', it is necessary to activate the volumes to generate the command line, because it can be necessary to check whether the volume is a block device or a regular file. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> --- PVE/QemuServer.pm | 61 +++++++++++++++++++++++--------- test/run_config2command_tests.pl | 10 ++++++ 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 934adf60..82304096 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -3844,7 +3844,6 @@ sub config_to_command { push @$devices, '-watchdog-action', $wdopts->{action} if $wdopts->{action}; } - my $vollist = []; my $scsicontroller = {}; my $ahcicontroller = {}; my $scsihw = defined($conf->{scsihw}) ? $conf->{scsihw} : $defaults->{scsihw}; @@ -3857,11 +3856,6 @@ sub config_to_command { PVE::QemuConfig->foreach_volume($conf, sub { my ($ds, $drive) = @_; - if (PVE::Storage::parse_volume_id($drive->{file}, 1)) { - check_volume_storage_type($storecfg, $drive->{file}); - push @$vollist, $drive->{file}; - } - # ignore efidisk here, already added in bios/fw handling code above return if $drive->{interface} eq 'efidisk'; # similar for TPM @@ -4035,7 +4029,6 @@ sub config_to_command { if (my $vmstate = $conf->{vmstate}) { my $statepath = PVE::Storage::path($storecfg, $vmstate); - push @$vollist, $vmstate; push @$cmd, '-loadstate', $statepath; print "activating and using '$vmstate' as vmstate\n"; } @@ -4051,7 +4044,7 @@ sub config_to_command { push @$cmd, @$aa; } - return wantarray ? ($cmd, $vollist, $spice_port, $pci_devices, $conf) : $cmd; + return wantarray ? ($cmd, $spice_port, $pci_devices, $conf) : $cmd; } sub spice_port { @@ -5649,12 +5642,18 @@ sub vm_start_nolock { my $memory = get_current_memory($conf->{memory}); my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $memory, $resume); - my ($cmd, $vollist, $spice_port); + my $vollist = get_current_vm_volumes($storecfg, $conf); + push $vollist->@*, $statefile if $statefile_is_a_volume; + + my ($cmd, $spice_port); my $pci_reserve_list = []; eval { + # With -blockdev, it is necessary to activate the volumes before generating the command line + PVE::Storage::activate_volumes($storecfg, $vollist); + # Note that for certain cases like templates, the configuration is minimized, so need to ensure # the rest of the function here uses the same configuration that was used to build the command - ($cmd, $vollist, $spice_port, my $pci_devices, $conf) = config_to_command( + ($cmd, $spice_port, my $pci_devices, $conf) = config_to_command( $storecfg, $vmid, $conf, @@ -5665,7 +5664,6 @@ sub vm_start_nolock { ); push $cmd->@*, $state_cmdline->@*; - push @$vollist, $statefile if $statefile_is_a_volume; for my $device (values $pci_devices->%*) { next if $device->{mdev}; # we don't reserve for mdev devices @@ -5707,14 +5705,13 @@ sub vm_start_nolock { push @$cmd, '-uuid', $uuid if defined($uuid); }; if (my $err = $@) { + eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); }; + warn $@ if $@; eval { cleanup_pci_devices($vmid, $conf) }; warn $@ if $@; die $err; } - PVE::Storage::activate_volumes($storecfg, $vollist); - - my %silence_std_outs = (outfunc => sub {}, errfunc => sub {}); eval { run_command(['/bin/systemctl', 'reset-failed', "$vmid.scope"], %silence_std_outs) }; eval { run_command(['/bin/systemctl', 'stop', "$vmid.scope"], %silence_std_outs) }; @@ -5982,14 +5979,25 @@ sub vm_commandline { my $defaults = load_defaults(); + my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid); + my $volumes = []; + + # With -blockdev, it is necessary to activate the volumes before generating the command line + if (!$running) { + $volumes = get_current_vm_volumes($storecfg, $conf); + PVE::Storage::activate_volumes($storecfg, $volumes); + } + my $cmd; eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu); }; my $err = $@; - # if the vm is not running, we need to clean up the reserved/created devices - if (!PVE::QemuServer::Helpers::vm_running_locally($vmid)) { + # if the vm is not running, need to clean up the reserved/created devices and activated volumes + if (!$running) { + eval { PVE::Storage::deactivate_volumes($storecfg, $volumes); }; + warn $@ if $@; eval { cleanup_pci_devices($vmid, $conf) }; warn $@ if $@; } @@ -6030,6 +6038,27 @@ sub get_vm_volumes { return $vollist; } +# Get volumes defined in the current VM configuration, including the VM state file. +sub get_current_vm_volumes { + my ($storecfg, $conf) = @_; + + my $volumes = []; + + PVE::QemuConfig->foreach_volume($conf, sub { + my ($ds, $drive) = @_; + + if (PVE::Storage::parse_volume_id($drive->{file}, 1)) { + check_volume_storage_type($storecfg, $drive->{file}); + push $volumes->@*, $drive->{file}; + } + }); + if (my $vmstate = $conf->{vmstate}) { + push $volumes->@*, $vmstate; + } + + return $volumes; +} + sub cleanup_pci_devices { my ($vmid, $conf) = @_; diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl index 7c581ad2..5217a020 100755 --- a/test/run_config2command_tests.pl +++ b/test/run_config2command_tests.pl @@ -255,6 +255,16 @@ $qemu_server_module->mock( }, ); +my $storage_module = Test::MockModule->new("PVE::Storage"); +$storage_module->mock( + activate_volumes => sub { + return; + }, + deactivate_volumes => sub { + return; + }, +); + my $zfsplugin_module = Test::MockModule->new("PVE::Storage::ZFSPlugin"); $zfsplugin_module->mock( zfs_get_lu_name => sub { -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel