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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list