vm start/stop: cleanup passed-through pci devices in more situations

if the preparing of PCI devices or the start of the VM fails, we need
to cleanup the PCI devices (reservations *and* mdevs), or else it
might happen that there are leftovers which must be manually removed.

to include also mdevs now, refactor the cleanup code from
'vm_stop_cleanup' into it's own function, and call that instead of
only 'remove_pci_reservation'

also simplifies the code, such that it now removes all PCI ids
reserved for that VMID, since we cannot have multiple VMs with the
same VMID anyway

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index baee8bc1be2e22c907db09a9c50ba54f90ab4722..9b9f0b0ecc41afd9bb6a436e1a51a8703243271e 100644 (file)
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5632,7 +5632,7 @@ sub vm_start_nolock {
        push @$cmd, '-uuid', $uuid if defined($uuid);
     };
     if (my $err = $@) {
-       eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
+       eval { cleanup_pci_devices($vmid, $conf) };
        warn $@ if $@;
        die $err;
     }
@@ -5728,7 +5728,9 @@ sub vm_start_nolock {
     if (my $err = $@) {
        # deactivate volumes if start fails
        eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
-       eval { PVE::QemuServer::PCI::remove_pci_reservation($pci_id_list) };
+       warn $@ if $@;
+       eval { cleanup_pci_devices($vmid, $conf) };
+       warn $@ if $@;
 
        die "start failed: $err";
     }
@@ -5893,6 +5895,19 @@ sub get_vm_volumes {
     return $vollist;
 }
 
+sub cleanup_pci_devices {
+    my ($vmid, $conf) = @_;
+
+    foreach my $key (keys %$conf) {
+       next if $key !~ m/^hostpci(\d+)$/;
+       my $hostpciindex = $1;
+       my $uuid = PVE::SysFSTools::generate_mdev_uuid($vmid, $hostpciindex);
+       my $d = parse_hostpci($conf->{$key});
+       PVE::SysFSTools::pci_cleanup_mdev_device($uuid) if $d->{mdev};
+    }
+    PVE::QemuServer::PCI::remove_pci_reservation($vmid);
+}
+
 sub vm_stop_cleanup {
     my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes) = @_;
 
@@ -5924,20 +5939,7 @@ sub vm_stop_cleanup {
            unlink '/dev/shm/pve-shm-' . ($ivshmem->{name} // $vmid);
        }
 
-       my $ids = [];
-       foreach my $key (keys %$conf) {
-           next if $key !~ m/^hostpci(\d+)$/;
-           my $hostpciindex = $1;
-           my $d = parse_hostpci($conf->{$key});
-           my $uuid = PVE::SysFSTools::generate_mdev_uuid($vmid, $hostpciindex);
-
-           foreach my $pci (@{$d->{pciid}}) {
-               my $pciid = $pci->{id};
-               push @$ids, $pci->{id};
-               PVE::SysFSTools::pci_cleanup_mdev_device($pciid, $uuid);
-           }
-       }
-       PVE::QemuServer::PCI::remove_pci_reservation($ids);
+       cleanup_pci_devices($vmid, $conf);
 
        vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
     };

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 3d0e70ea2b0a6e01b7a0b8470d163dfad5321667..788ab2ad209a5aeb5081e13a34964bf102e21258 100644 (file)
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -552,15 +552,17 @@ my $write_pci_reservation_unlocked = sub {
     PVE::Tools::file_set_contents($PCIID_RESERVATION_FILE, $data);
 };
 
+# removes all pci reservations of the given vmid
 sub remove_pci_reservation {
-    my ($dropped_ids) = @_;
-
-    $dropped_ids = [ $dropped_ids ] if !ref($dropped_ids);
-    return if !scalar(@$dropped_ids); # do nothing for empty list
+    my ($vmid) = @_;
 
     PVE::Tools::lock_file($PCIID_RESERVATION_LOCK, 2, sub {
        my $reservation_list = $parse_pci_reservation_unlocked->();
-       delete $reservation_list->@{$dropped_ids->@*};
+       for my $id (keys %$reservation_list) {
+           my $reservation = $reservation_list->{$id};
+           next if $reservation->{vmid} != $vmid;
+           delete $reservation_list->{$id};
+       }
        $write_pci_reservation_unlocked->($reservation_list);
     });
     die $@ if $@;