[PATCH 0/4] qemu: hotplug: Fix check whether controler is used
Patch 2/4 is the actual fix, other patches are additional cleanups. Peter Krempa (4): qemuDomain(Disk)ControllerIsBusy: Fix function header format qemuDomainDiskControllerIsBusy: Fix logic of matching disk bus to controller type qemuDomainDiskControllerIsBusy: Optimize checking for SCSI hostdevs qemuDomainControllerIsBusy: Fully populate switch statement src/qemu/qemu_hotplug.c | 92 ++++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 23 deletions(-) -- 2.28.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 81ec44ffcd..00d908912f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5312,8 +5312,9 @@ qemuDomainDetachPrepDisk(virDomainObjPtr vm, } -static bool qemuDomainDiskControllerIsBusy(virDomainObjPtr vm, - virDomainControllerDefPtr detach) +static bool +qemuDomainDiskControllerIsBusy(virDomainObjPtr vm, + virDomainControllerDefPtr detach) { size_t i; virDomainDiskDefPtr disk; @@ -5352,8 +5353,10 @@ static bool qemuDomainDiskControllerIsBusy(virDomainObjPtr vm, return false; } -static bool qemuDomainControllerIsBusy(virDomainObjPtr vm, - virDomainControllerDefPtr detach) + +static bool +qemuDomainControllerIsBusy(virDomainObjPtr vm, + virDomainControllerDefPtr detach) { switch (detach->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: @@ -5372,6 +5375,7 @@ static bool qemuDomainControllerIsBusy(virDomainObjPtr vm, } } + static int qemuDomainDetachPrepController(virDomainObjPtr vm, virDomainControllerDefPtr match, -- 2.28.0
The tests which match the disk bus to the controller type were backwards in this function. This meant that any disk bus type (such as VIR_DOMAIN_DISK_BUS_SATA) would not skip the controller index comparison even if the removed controller was of a different type. Switch the internals to a switch statement with selects the controller type in the first place and a proper type so that new controller types are added in the future. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870072 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 44 +++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 00d908912f..90ed59a0ee 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5327,16 +5327,48 @@ qemuDomainDiskControllerIsBusy(virDomainObjPtr vm, continue; /* check whether the disk uses this type controller */ - if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE && - detach->type != VIR_DOMAIN_CONTROLLER_TYPE_IDE) + switch ((virDomainControllerType) detach->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_IDE: + if (disk->bus != VIR_DOMAIN_DISK_BUS_IDE) + continue; + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC) + continue; + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) + continue; + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: + if (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) + continue; + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: + /* xenbus is not supported by the qemu driver */ continue; - if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC && - detach->type != VIR_DOMAIN_CONTROLLER_TYPE_FDC) + + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + /* virtio-serial does not host any disks */ continue; - if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && - detach->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) + + case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_USB: + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: + /* These buses have (also) other device types too so they need to + * be checked elsewhere */ continue; + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: + default: + continue; + } + if (disk->info.addr.drive.controller == detach->idx) return true; } -- 2.28.0
Iterate through hostdevs only when the controller type is VIR_DOMAIN_CONTROLLER_TYPE_SCSI. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 90ed59a0ee..124f60912f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5373,13 +5373,15 @@ qemuDomainDiskControllerIsBusy(virDomainObjPtr vm, return true; } - for (i = 0; i < vm->def->nhostdevs; i++) { - hostdev = vm->def->hostdevs[i]; - if (!virHostdevIsSCSIDevice(hostdev) || - detach->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) - continue; - if (hostdev->info->addr.drive.controller == detach->idx) - return true; + if (detach->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + for (i = 0; i < vm->def->nhostdevs; i++) { + hostdev = vm->def->hostdevs[i]; + if (!virHostdevIsSCSIDevice(hostdev)) + continue; + + if (hostdev->info->addr.drive.controller == detach->idx) + return true; + } } return false; -- 2.28.0
Typecast the controller type variable to the appropriate type and add the missing controller types for future extension. Note that we currently allow only unplug of VIR_DOMAIN_CONTROLLER_TYPE_SCSI thus the other controller types which are not implemented return false now. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 124f60912f..2c12ef60af 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5392,20 +5392,28 @@ static bool qemuDomainControllerIsBusy(virDomainObjPtr vm, virDomainControllerDefPtr detach) { - switch (detach->type) { + switch ((virDomainControllerType) detach->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: return qemuDomainDiskControllerIsBusy(vm, detach); - case VIR_DOMAIN_CONTROLLER_TYPE_SATA: case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_USB: + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: + /* detach of the controller types above is not yet supported */ + return false; + + case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: + /* qemu driver doesn't support xenbus */ + return false; + + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: default: - /* libvirt does not support sata controller, and does not support to - * detach virtio and smart card controller. - */ - return true; + return false; } } -- 2.28.0
On a Friday in 2020, Peter Krempa wrote:
Patch 2/4 is the actual fix, other patches are additional cleanups.
Peter Krempa (4): qemuDomain(Disk)ControllerIsBusy: Fix function header format qemuDomainDiskControllerIsBusy: Fix logic of matching disk bus to controller type qemuDomainDiskControllerIsBusy: Optimize checking for SCSI hostdevs qemuDomainControllerIsBusy: Fully populate switch statement
src/qemu/qemu_hotplug.c | 92 ++++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 23 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa