Re: [PATCH] - make virDomainDetachDeviceFlags respect VIR_DOMAIN_DEVICE_MODIFY_FORCE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]