[pve-devel] [PATCH qemu-server v2 24/32] qm: showcmd: never reserve PCI devices
Fiona Ebner
f.ebner at proxmox.com
Wed Jun 18 15:02:01 CEST 2025
Never reserve PCI devices when config_to_command() is called for
'showcmd'. Previously, choose_hostpci_devices() only skipped reserving
PCI devices when the VM was already running as a heuristic. Make it
explicit via a new parameter. Adapt the config-to-command test to
never expect actual reservation. There is a dedicated test for this
since commit "test: add tests for PCI reservations".
Suggested-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
New in v2.
src/PVE/QemuServer.pm | 19 +++++--------------
src/PVE/QemuServer/PCI.pm | 15 ++++++---------
.../q35-linux-hostpci-mapping.conf.cmd | 2 +-
src/test/run_config2command_tests.pl | 14 ++------------
4 files changed, 14 insertions(+), 36 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 245e1a26..843db76a 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -3528,8 +3528,8 @@ my sub get_vga_properties {
sub config_to_command {
my ($storecfg, $vmid, $conf, $defaults, $options) = @_;
- my ($forcemachine, $forcecpu, $live_restore_backing) =
- $options->@{qw(force-machine force-cpu live-restore-backing)};
+ my ($forcemachine, $forcecpu, $live_restore_backing, $dry_run) =
+ $options->@{qw(force-machine force-cpu live-restore-backing dry-run)};
# minimize config for templates, they can only start for backup,
# so most options besides the disks are irrelevant
@@ -3738,7 +3738,7 @@ sub config_to_command {
# host pci device passthrough
my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) =
PVE::QemuServer::PCI::print_hostpci_devices(
- $vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder,
+ $vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder, $dry_run,
);
# usb devices
@@ -6325,7 +6325,7 @@ sub vm_commandline {
my $conf = PVE::QemuConfig->load_config($vmid);
- my $options = {};
+ my $options = { 'dry-run' => 1 };
if ($snapname) {
my $snapshot = $conf->{snapshots}->{$snapname};
die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
@@ -6350,18 +6350,9 @@ sub vm_commandline {
PVE::Storage::activate_volumes($storecfg, $volumes);
}
- my $cmd;
- eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $options); };
- my $err = $@;
-
- # If the vm is not running, need to clean up the reserved/created devices.
# There might be concurrent operations on the volumes, so do not deactivate.
- if (!$running) {
- eval { cleanup_pci_devices($vmid, $conf) };
- warn $@ if $@;
- }
- die $err if $err;
+ my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $options);
return PVE::Tools::cmd2string($cmd);
}
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 2dbc87a7..ddf2f028 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -577,10 +577,7 @@ my sub create_nvidia_device {
# mdev devices must be chosen later when we actually allocate it, but we
# flatten the inner list since there can only be one device per alternative anyway
sub choose_hostpci_devices {
- my ($devices, $vmid) = @_;
-
- # if the vm is running, we must be in 'showcmd', so don't actually reserve or create anything
- my $is_running = PVE::QemuServer::Helpers::vm_running_locally($vmid) ? 1 : 0;
+ my ($devices, $vmid, $dry_run) = @_;
my $used = {};
@@ -606,7 +603,7 @@ sub choose_hostpci_devices {
# we only have one alternative, use that
$device->{ids} = $device->{ids}->[0];
$add_used_device->($device->{ids});
- if ($device->{nvidia} && !$is_running) {
+ if ($device->{nvidia} && !$dry_run) {
reserve_pci_usage($device->{ids}->[0]->{id}, $vmid, 10, undef);
create_nvidia_device($device->{ids}->[0]->{id}, $device->{nvidia});
}
@@ -618,12 +615,12 @@ sub choose_hostpci_devices {
my $ids = [map { $_->{id} } @$alternative];
next if grep { defined($used->{$_}) } @$ids; # already used
- if (!$is_running) {
+ if (!$dry_run) {
eval { reserve_pci_usage($ids, $vmid, 10, undef) };
next if $@;
}
- if ($device->{nvidia} && !$is_running) {
+ if ($device->{nvidia} && !$dry_run) {
eval { create_nvidia_device($ids->[0], $device->{nvidia}) };
if (my $err = $@) {
warn $err;
@@ -645,14 +642,14 @@ sub choose_hostpci_devices {
}
sub print_hostpci_devices {
- my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder) = @_;
+ my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder, $dry_run) = @_;
my $kvm_off = 0;
my $gpu_passthrough = 0;
my $legacy_igd = 0;
my $pciaddr;
- my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
+ my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid, $dry_run);
for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++) {
my $id = "hostpci$i";
diff --git a/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd b/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
index 533e1983..6df8b5f2 100644
--- a/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
+++ b/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
@@ -29,7 +29,7 @@
-device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
-device 'vfio-pci,host=0000:07:10.0,id=hostpci0,bus=pci.0,addr=0x10' \
-device 'vfio-pci,sysfsdev=/sys/bus/mdev/devices/00000001-0000-0000-0000-000000008006,id=hostpci1,bus=pci.0,addr=0x11' \
- -device 'vfio-pci,host=0000:07:10.4,id=hostpci2,bus=pci.0,addr=0x1b' \
+ -device 'vfio-pci,host=0000:07:10.1,id=hostpci2,bus=pci.0,addr=0x1b' \
-device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
-device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
-iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
diff --git a/src/test/run_config2command_tests.pl b/src/test/run_config2command_tests.pl
index cbd8cd8b..9f4ecabf 100755
--- a/src/test/run_config2command_tests.pl
+++ b/src/test/run_config2command_tests.pl
@@ -474,20 +474,10 @@ $mapping_pci_module->mock(
my $pci_module = Test::MockModule->new("PVE::QemuServer::PCI");
$pci_module->mock(
reserve_pci_usage => sub {
- my ($ids, $vmid, $timeout, $pid, $dryrun) = @_;
-
- $ids = [$ids] if !ref($ids);
-
- for my $id (@$ids) {
- if ($id eq "0000:07:10.1") {
- die "reserved";
- }
- }
-
- return undef;
+ die "reserve_pci_usage should not be called for 'qm showcmd'\n";
},
create_nvidia_device => sub {
- return 1;
+ die "create_nvidia_device should not be called for 'qm showcmd'\n";
},
);
--
2.39.5
More information about the pve-devel
mailing list