9:44 a.m.
Refactor checking of disk bus and type for disk unplug and fix checking
whether a virtio disk has a running block job by deleting the
almost-identical implementation specific to virtio disks.
Peter Krempa (4):
qemu: hotplug: Remove 'ret' variable in qemuDomainDetachDeviceDiskLive
qemu: hotplug: Use typecasted enum in qemuDomainDetachDeviceDiskLive
qemu: hotplug: Use switch statement for selecting disk bus function
qemu: hotplug: Merge virtio and non-virtio disk unplug code
src/qemu/qemu_hotplug.c | 87 ++++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 48 deletions(-)
--
2.20.1
9:44 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Remove 'ret' variable in qemuDomainDetachDeviceDiskLive
We don't have any cleanup section, we can return the value directly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 811fdf6c3a..0825b03a0d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5366,7 +5366,6 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
bool async)
{
virDomainDiskDefPtr disk;
- int ret = -1;
int idx;
if ((idx = qemuFindDisk(vm->def, dev->data.disk->dst)) < 0) {
@@ -5380,10 +5379,10 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
case VIR_DOMAIN_DISK_DEVICE_DISK:
case VIR_DOMAIN_DISK_DEVICE_LUN:
if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
- ret = qemuDomainDetachVirtioDiskDevice(driver, vm, disk, async);
+ return qemuDomainDetachVirtioDiskDevice(driver, vm, disk, async);
else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
disk->bus == VIR_DOMAIN_DISK_BUS_USB)
- ret = qemuDomainDetachDiskDevice(driver, vm, disk, async);
+ return qemuDomainDetachDiskDevice(driver, vm, disk, async);
else
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("This type of disk cannot be hot unplugged"));
@@ -5395,7 +5394,7 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
break;
}
- return ret;
+ return -1;
}
--
2.20.1
9:53 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Remove 'ret' variable in qemuDomainDetachDeviceDiskLive
On Fri, Mar 15, 2019 at 03:44:55PM +0100, Peter Krempa wrote:
We don't have any cleanup section, we can return the value
directly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:44 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: hotplug: Use typecasted enum in qemuDomainDetachDeviceDiskLive
Use the correct type in switch and populate the missing cases.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0825b03a0d..92af85c3d3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5375,7 +5375,7 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
}
disk = vm->def->disks[idx];
- switch (disk->device) {
+ switch ((virDomainDiskDevice) disk->device) {
case VIR_DOMAIN_DISK_DEVICE_DISK:
case VIR_DOMAIN_DISK_DEVICE_LUN:
if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
@@ -5387,11 +5387,18 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("This type of disk cannot be hot unplugged"));
break;
- default:
+
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("disk device type '%s' cannot be detached"),
virDomainDiskDeviceTypeToString(disk->device));
break;
+
+ case VIR_DOMAIN_DISK_DEVICE_LAST:
+ default:
+ virReportEnumRangeError(virDomainDiskDevice, disk->device);
+ break;
}
return -1;
--
2.20.1
9:54 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: hotplug: Use typecasted enum in qemuDomainDetachDeviceDiskLive
On Fri, Mar 15, 2019 at 03:44:56PM +0100, Peter Krempa wrote:
Use the correct type in switch and populate the missing cases.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:44 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: hotplug: Use switch statement for selecting disk bus function
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 92af85c3d3..ffe0031362 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5378,14 +5378,30 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
switch ((virDomainDiskDevice) disk->device) {
case VIR_DOMAIN_DISK_DEVICE_DISK:
case VIR_DOMAIN_DISK_DEVICE_LUN:
- if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
+
+ switch ((virDomainDiskBus) disk->bus) {
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
return qemuDomainDetachVirtioDiskDevice(driver, vm, disk, async);
- else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
- disk->bus == VIR_DOMAIN_DISK_BUS_USB)
+
+ case VIR_DOMAIN_DISK_BUS_USB:
+ case VIR_DOMAIN_DISK_BUS_SCSI:
return qemuDomainDetachDiskDevice(driver, vm, disk, async);
- else
+
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ case VIR_DOMAIN_DISK_BUS_XEN:
+ case VIR_DOMAIN_DISK_BUS_UML:
+ case VIR_DOMAIN_DISK_BUS_SATA:
+ case VIR_DOMAIN_DISK_BUS_SD:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("This type of disk cannot be hot unplugged"));
+ break;
+
+ case VIR_DOMAIN_DISK_BUS_LAST:
+ default:
+ virReportEnumRangeError(virDomainDiskBus, disk->bus);
+ break;
+ }
break;
case VIR_DOMAIN_DISK_DEVICE_CDROM:
--
2.20.1
9:54 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: hotplug: Use switch statement for selecting disk bus function
On Fri, Mar 15, 2019 at 03:44:57PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:44 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: hotplug: Merge virtio and non-virtio disk unplug code
The functions do basically exactly the same thing modulo few checks.
In case of virtio disks we check that the device is not multifunction as
that can't be unplugged at once. In case of USB and SCSI disks we
checked that no active block job is running.
The check for running blockjobs should have also been done for virtio
disks. By moving the multifunction check into the common function we fix
this case and also simplify the code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 49 ++++++++---------------------------------
1 file changed, 9 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ffe0031362..41d60277d1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5276,43 +5276,6 @@ qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
}
-static int
-qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainDiskDefPtr detach,
- bool async)
-{
- int ret = -1;
-
- if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("cannot hot unplug multifunction PCI device: %s"),
- detach->dst);
- goto cleanup;
- }
-
- if (!async)
- qemuDomainMarkDeviceForRemoval(vm, &detach->info);
-
- if (qemuDomainDeleteDevice(vm, detach->info.alias) < 0) {
- if (virDomainObjIsActive(vm))
- virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
- goto cleanup;
- }
-
- if (async) {
- ret = 0;
- } else {
- if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
- ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
- }
-
- cleanup:
- if (!async)
- qemuDomainResetDeviceRemoval(vm);
- return ret;
-}
-
static int
qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -5322,7 +5285,15 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
int ret = -1;
if (qemuDomainDiskBlockJobIsActive(detach))
- goto cleanup;
+ return -1;
+
+ if (detach->bus == VIR_DOMAIN_DISK_BUS_VIRTIO &&
+ qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("cannot hot unplug multifunction PCI device: %s"),
+ detach->dst);
+ return -1;
+ }
if (!async)
qemuDomainMarkDeviceForRemoval(vm, &detach->info);
@@ -5381,8 +5352,6 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
switch ((virDomainDiskBus) disk->bus) {
case VIR_DOMAIN_DISK_BUS_VIRTIO:
- return qemuDomainDetachVirtioDiskDevice(driver, vm, disk, async);
-
case VIR_DOMAIN_DISK_BUS_USB:
case VIR_DOMAIN_DISK_BUS_SCSI:
return qemuDomainDetachDiskDevice(driver, vm, disk, async);
--
2.20.1
9:56 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: hotplug: Merge virtio and non-virtio disk unplug code
On Fri, Mar 15, 2019 at 03:44:58PM +0100, Peter Krempa wrote:
The functions do basically exactly the same thing modulo few checks.
In case of virtio disks we check that the device is not multifunction as
that can't be unplugged at once. In case of USB and SCSI disks we
checked that no active block job is running.
The check for running blockjobs should have also been done for virtio
disks. By moving the multifunction check into the common function we fix
this case and also simplify the code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 49 ++++++++---------------------------------
1 file changed, 9 insertions(+), 40 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2078
days inactive
2078
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa