From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id DB7C48B658
 for <pve-devel@lists.proxmox.com>; Thu, 25 Aug 2022 11:25:22 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 0C5B922F4C
 for <pve-devel@lists.proxmox.com>; Thu, 25 Aug 2022 11:24:51 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Thu, 25 Aug 2022 11:24:47 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2618D43FB7
 for <pve-devel@lists.proxmox.com>; Thu, 25 Aug 2022 11:24:42 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Thu, 25 Aug 2022 11:24:15 +0200
Message-Id: <20220825092440.1810328-7-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.30.2
In-Reply-To: <20220825092440.1810328-1-d.csapak@proxmox.com>
References: <20220825092440.1810328-1-d.csapak@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.095 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 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
 T_SCC_BODY_TEXT_LINE    -0.01 -
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [qemuserver.pm, pci.pm]
Subject: [pve-devel] [PATCH qemu-server v2 01/13] cleanup pci devices in
 more situations
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>
X-List-Received-Date: Thu, 25 Aug 2022 09:25:22 -0000

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'

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm     | 34 ++++++++++++++++++----------------
 PVE/QemuServer/PCI.pm | 12 +++++++-----
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c706653..b3c3ce0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5609,7 +5609,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;
     }
@@ -5705,7 +5705,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";
     }
@@ -5870,6 +5872,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) = @_;
 
@@ -5901,20 +5916,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 3d0e70e..788ab2a 100644
--- 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 $@;
-- 
2.30.2