[libvirt] [PATCH] - make virDomainDetachDeviceFlags respect VIR_DOMAIN_DEVICE_MODIFY_FORCE
Martin Kletzander
mkletzan at redhat.com
Tue Oct 21 09:45:57 UTC 2014
On Fri, Oct 10, 2014 at 09:05:01AM +0200, Marcin Gibuła wrote:
>Hi,
>
>currently, there is no way to force disk detach from KVM guest if guest
>does not cooperate. This patch makes virDomainDetachDeviceFlags() respect
>VIR_DOMAIN_DEVICE_MODIFY_FORCE flag. When it's on, libvirt will always
>call drive_del, regardless if guest responsed to ACPI unplug request or not.
>
>---
>
>diff -ru libvirt-1.2.6-orig/src/qemu/qemu_driver.c libvirt-1.2.6/src/qemu/qemu_driver.c
>--- libvirt-1.2.6-orig/src/qemu/qemu_driver.c 2014-07-02 05:35:47.000000000 +0200
>+++ libvirt-1.2.6/src/qemu/qemu_driver.c 2014-10-09 13:37:27.863897583 +0200
Hi there,
thank you for taking the time to submit the patch. However, I need to
point out few things. The patch is based on libvirt-1.2.6 and hence
does not apply on top of current master branch. Also, you might find
git pretty useful for creating the patches. I wanted to redirect you
to our contributor guidelines [1], but they seem a bit off right from
the start, I'll see if someone will accept my proposal to modernize it
a bit.
Feel free to clone the current repository [2] and base your patch on
that, git format-patch will probably speed up your workflow as well.
To comment on the patch itself, have you tried that it really does
what you claim? Looking at the patch I feel like it doesn't. The
function qemuDomainDetachDeviceFlags(), that is the only one that
calls qemuDomainDetachDeviceLive(), also calls virCheckFlags() without
VIR_DOMAIN_DEVICE_MODIFY_FORCE, which means that call to that function
will end with error.
Do you think this is safe for all disks? Are you trying to solve some
particular problem with this patch?
Martin
[1] http://libvirt.org/hacking.html
[2] located at git://libvirt.org/libvirt.git
>@@ -6525,14 +6525,15 @@
> static int
> qemuDomainDetachDeviceLive(virDomainObjPtr vm,
> virDomainDeviceDefPtr dev,
>- virDomainPtr dom)
>+ virDomainPtr dom,
>+ int flags)
> {
> virQEMUDriverPtr driver = dom->conn->privateData;
> int ret = -1;
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
>- ret = qemuDomainDetachDeviceDiskLive(driver, vm, dev);
>+ ret = qemuDomainDetachDeviceDiskLive(driver, vm, dev, flags);
> break;
> case VIR_DOMAIN_DEVICE_CONTROLLER:
> ret = qemuDomainDetachDeviceControllerLive(driver, vm, dev);
>@@ -7257,7 +7258,8 @@
> virCapsPtr caps = NULL;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>- VIR_DOMAIN_AFFECT_CONFIG, -1);
>+ VIR_DOMAIN_AFFECT_CONFIG |
>+ VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
>
> cfg = virQEMUDriverGetConfig(driver);
>
>@@ -7343,7 +7345,7 @@
> VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0)
> goto endjob;
>
>- if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0)
>+ if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom, flags)) < 0)
> goto endjob;
> /*
> * update domain status forcibly because the domain status may be
>diff -ru libvirt-1.2.6-orig/src/qemu/qemu_hotplug.c libvirt-1.2.6/src/qemu/qemu_hotplug.c
>--- libvirt-1.2.6-orig/src/qemu/qemu_hotplug.c 2014-06-27 05:50:18.000000000 +0200
>+++ libvirt-1.2.6/src/qemu/qemu_hotplug.c 2014-10-09 13:35:55.566375716 +0200
>@@ -2906,7 +2906,8 @@
> static int
> qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
>- virDomainDiskDefPtr detach)
>+ virDomainDiskDefPtr detach,
>+ int flags)
> {
> int ret = -1;
> qemuDomainObjPrivatePtr priv = vm->privateData;
>@@ -2958,7 +2959,7 @@
> qemuDomainObjExitMonitor(driver, vm);
>
> rc = qemuDomainWaitForDeviceRemoval(vm);
>- if (rc == 0 || rc == 1)
>+ if (rc == 0 || rc == 1 || (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE))
> ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
> else
> ret = 0;
>@@ -2971,7 +2972,8 @@
> static int
> qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
>- virDomainDiskDefPtr detach)
>+ virDomainDiskDefPtr detach,
>+ int flags)
> {
> int ret = -1;
> qemuDomainObjPrivatePtr priv = vm->privateData;
>@@ -3003,7 +3005,7 @@
> qemuDomainObjExitMonitor(driver, vm);
>
> rc = qemuDomainWaitForDeviceRemoval(vm);
>- if (rc == 0 || rc == 1)
>+ if (rc == 0 || rc == 1 || (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE))
> ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
> else
> ret = 0;
>@@ -3030,7 +3032,8 @@
> int
> qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
>- virDomainDeviceDefPtr dev)
>+ virDomainDeviceDefPtr dev,
>+ int flags)
> {
> virDomainDiskDefPtr disk;
> int ret = -1;
>@@ -3047,10 +3050,10 @@
> 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);
>+ ret = qemuDomainDetachVirtioDiskDevice(driver, vm, disk, flags);
> else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
> disk->bus == VIR_DOMAIN_DISK_BUS_USB)
>- ret = qemuDomainDetachDiskDevice(driver, vm, disk);
>+ ret = qemuDomainDetachDiskDevice(driver, vm, disk, flags);
> else
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> _("This type of disk cannot be hot unplugged"));
>diff -ru libvirt-1.2.6-orig/src/qemu/qemu_hotplug.h libvirt-1.2.6/src/qemu/qemu_hotplug.h
>--- libvirt-1.2.6-orig/src/qemu/qemu_hotplug.h 2014-06-25 13:25:52.000000000 +0200
>+++ libvirt-1.2.6/src/qemu/qemu_hotplug.h 2014-10-09 13:36:22.255659117 +0200
>@@ -71,7 +71,8 @@
> int linkstate);
> int qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
>- virDomainDeviceDefPtr dev);
>+ virDomainDeviceDefPtr dev,
>+ int flags);
> int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainDeviceDefPtr dev);
>
>
>--
>mg
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141021/bec4e6eb/attachment-0001.sig>
More information about the libvir-list
mailing list