[pve-devel] [PATCH qemu-server] fix #3608: unbreak removal of scsi controller

[pve-devel] [PATCH qemu-server] fix #3608: unbreak removal of scsi controller

[Fabian Grünbichler]

the assumption that the index of the controller matches that of the last
removed drive only holds for virtio-scsi-single controller, which makes
the old code print a warning when removing the last drive of a
non-virtio-scsi-single controller except when the indices line up by
chance.

we can simply only call a simplified qemu_iothread_del when removing a
scsi disk of a VM with the virtio-scsi-single controller, and skip the
call for the other controllers which don't support io-threads anyway.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
alternatively we could keep the old version but conditionally only call
it for virtioscsi.* and not for scsihw.*, but the now aligned signature
for adding and removing iothread objects seems like a nice bonus to me
;)

 PVE/QemuServer.pm | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index cc73af8..c5232d6 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4038,24 +4038,28 @@ sub vm_deviceunplug {
 	#qemu_devicedelverify($vmid, $deviceid);
 
     } elsif ($deviceid =~ m/^(virtio)(\d+)$/) {
+	my $device = parse_drive($deviceid, $conf->{$deviceid});
 
         qemu_devicedel($vmid, $deviceid);
         qemu_devicedelverify($vmid, $deviceid);
         qemu_drivedel($vmid, $deviceid);
-	qemu_iothread_del($conf, $vmid, $deviceid);
+	qemu_iothread_del($vmid, $deviceid, $device);
 
     } elsif ($deviceid =~ m/^(virtioscsi|scsihw)(\d+)$/) {
 
 	qemu_devicedel($vmid, $deviceid);
 	qemu_devicedelverify($vmid, $deviceid);
-	qemu_iothread_del($conf, $vmid, $deviceid);
 
     } elsif ($deviceid =~ m/^(scsi)(\d+)$/) {
+	my $device = parse_drive($deviceid, $conf->{$deviceid});
 
         qemu_devicedel($vmid, $deviceid);
         qemu_drivedel($vmid, $deviceid);
 	qemu_deletescsihw($conf, $vmid, $deviceid);
 
+	qemu_iothread_del($vmid, "virtioscsi$device->{index}", $device)
+	    if $conf->{scsihw} && ($conf->{scsihw} eq 'virtio-scsi-single');
+
     } elsif ($deviceid =~ m/^(net)(\d+)$/) {
 
         qemu_devicedel($vmid, $deviceid);
@@ -4094,13 +4098,8 @@ sub qemu_iothread_add {
 }
 
 sub qemu_iothread_del {
-    my($conf, $vmid, $deviceid) = @_;
+    my($vmid, $deviceid, $device) = @_;
 
-    my $confid = $deviceid;
-    if ($deviceid =~ m/^(?:virtioscsi|scsihw)(\d+)$/) {
-	$confid = 'scsi' . $1;
-    }
-    my $device = parse_drive($confid, $conf->{$confid});
     if ($device->{iothread}) {
 	my $iothreads = vm_iothreads_list($vmid);
 	qemu_objectdel($vmid, "iothread-$deviceid") if $iothreads->{"iothread-$deviceid"};
-- 
2.30.2

[pve-devel] applied: [PATCH qemu-server] fix #3608: unbreak removal of scsi controller

[Thomas Lamprecht]

On 09.09.21 14:40, Fabian Grünbichler wrote:
> the assumption that the index of the controller matches that of the last
> removed drive only holds for virtio-scsi-single controller, which makes
> the old code print a warning when removing the last drive of a
> non-virtio-scsi-single controller except when the indices line up by
> chance.
> 
> we can simply only call a simplified qemu_iothread_del when removing a
> scsi disk of a VM with the virtio-scsi-single controller, and skip the
> call for the other controllers which don't support io-threads anyway.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> alternatively we could keep the old version but conditionally only call
> it for virtioscsi.* and not for scsihw.*, but the now aligned signature
> for adding and removing iothread objects seems like a nice bonus to me
> ;)
> 
>  PVE/QemuServer.pm | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
>

applied, thanks! Thrown in some white space/indentation fixes I noticed in the
surrounding code when checking out this patch more closely.