[libvirt] [PATCH 10/11] Integrate the QEMU driver with the lock manager infrastructure

Daniel P. Berrange berrange at redhat.com
Mon Jan 24 15:13:09 UTC 2011


The QEMU integrates with the lock manager instructure in a number
of key places

 * During startup, a lock is acquired in between the fork & exec
 * During startup, the libvirtd process acquires a lock before
   setting file labelling
 * During shutdown, the libvirtd process acquires a lock
   before restoring file labelling
 * During hotplug, unplug & media change the libvirtd process
   holds a lock while setting/restoring labels

The main content lock is only ever held by the QEMU child process,
or libvirtd during VM shutdown. The rest of the operations only
require libvirtd to hold the metadata locks, relying on the active
QEMU still holding the content lock.

* src/qemu/qemu_conf.c, src/qemu/qemu_conf.h,
  src/qemu/libvirtd_qemu.aug, src/qemu/test_libvirtd_qemu.aug:
  Add config parameter for configuring lock managers
* src/qemu/qemu_driver.c: Add calls to the lock manager
---
 src/qemu/libvirtd_qemu.aug      |    2 +
 src/qemu/qemu.conf              |   16 ++++
 src/qemu/qemu_conf.c            |   28 +++++++
 src/qemu/qemu_conf.h            |    5 +
 src/qemu/qemu_driver.c          |  106 +++++++++++++++++++++---
 src/qemu/qemu_hotplug.c         |  170 +++++++++++++++++++++++++++++++++------
 src/qemu/test_libvirtd_qemu.aug |    6 ++
 7 files changed, 293 insertions(+), 40 deletions(-)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 2f37015..55eb22c 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -44,6 +44,8 @@ module Libvirtd_qemu =
                  | bool_entry "clear_emulator_capabilities"
                  | bool_entry "allow_disk_format_probing"
                  | bool_entry "set_process_name"
+                 | str_entry "content_lock_manager"
+                 | str_entry "metadata_lock_manager"
 
    (* Each enty in the config is one of the following three ... *)
    let entry = vnc_entry
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 66310d4..5c53e4d 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -272,3 +272,19 @@
 # its arguments) appear in process listings.
 #
 # set_process_name = 1
+
+
+# To enable strict 'fcntl' based locking of the file
+# content (to prevent two VMs writing to the same
+# disk), start the 'virtlockd' service, and uncomment
+# this
+#
+# content_lock_manager = "fcntl"
+
+
+# To enable strict 'fcntl' based locking of the file
+# metadata (to prevent two libvirtd daemons on different
+# hosts doing conflicting metadata changes), start the
+# 'virtlockd' service, and uncomment this
+#
+# metadata_lock_manager = "fcntl"
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 9f9e99e..af952ba 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -115,6 +115,14 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
     }
 #endif
 
+    if (!(driver->contentLockManager =
+          virLockManagerPluginNew("nop",
+                                  VIR_LOCK_MANAGER_MODE_CONTENT)))
+        VIR_ERROR0(_("Failed to load lock manager nop"));
+    if (!(driver->metadataLockManager =
+          virLockManagerPluginNew("nop",
+                                  VIR_LOCK_MANAGER_MODE_METADATA)))
+        VIR_ERROR0(_("Failed to load lock manager nop"));
 
     /* Just check the file is readable before opening it, otherwise
      * libvirt emits an error.
@@ -423,6 +431,26 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
     CHECK_TYPE ("set_process_name", VIR_CONF_LONG);
     if (p) driver->setProcessName = p->l;
 
+     p = virConfGetValue (conf, "content_lock_manager");
+     CHECK_TYPE ("content_lock_manager", VIR_CONF_STRING);
+     if (p && p->str) {
+         virLockManagerPluginUnref(driver->contentLockManager);
+         if (!(driver->contentLockManager =
+               virLockManagerPluginNew(p->str,
+                                       VIR_LOCK_MANAGER_MODE_CONTENT)))
+             VIR_ERROR(_("Failed to load lock manager %s"), p->str);
+     }
+
+     p = virConfGetValue (conf, "metadata_lock_manager");
+     CHECK_TYPE ("metadata_lock_manager", VIR_CONF_STRING);
+     if (p && p->str) {
+         virLockManagerPluginUnref(driver->metadataLockManager);
+         if (!(driver->metadataLockManager =
+               virLockManagerPluginNew(p->str,
+                                       VIR_LOCK_MANAGER_MODE_METADATA)))
+             VIR_ERROR(_("Failed to load lock manager %s"), p->str);
+     }
+
     virConfFree (conf);
     return 0;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index af1be2e..1746e85 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -44,6 +44,7 @@
 # include "macvtap.h"
 # include "command.h"
 # include "threadpool.h"
+# include "locking/lock_manager.h"
 
 # define QEMUD_CPUMASK_LEN CPU_SETSIZE
 
@@ -127,6 +128,10 @@ struct qemud_driver {
     virBitmapPtr reservedVNCPorts;
 
     virSysinfoDefPtr hostsysinfo;
+
+    /* These two might point to the same instance */
+    virLockManagerPluginPtr contentLockManager;
+    virLockManagerPluginPtr metadataLockManager;
 };
 
 typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 34cc29f..096e771 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -85,6 +85,7 @@
 #include "fdstream.h"
 #include "configmake.h"
 #include "threadpool.h"
+#include "locking/domain_lock.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -1329,6 +1330,15 @@ qemudStartup(int privileged) {
     }
     VIR_FREE(driverConf);
 
+    /* We should always at least have the 'nop' manager, so
+     * NULLs here are a fatal error
+     */
+    if (!qemu_driver->contentLockManager ||
+        !qemu_driver->metadataLockManager) {
+        VIR_ERROR0(_("Missing content/metadata lock managers"));
+        goto error;
+    }
+
     if (qemuSecurityInit(qemu_driver) < 0)
         goto error;
 
@@ -1573,6 +1583,9 @@ qemudShutdown(void) {
 
     virCgroupFree(&qemu_driver->cgroup);
 
+    virLockManagerPluginUnref(qemu_driver->contentLockManager);
+    virLockManagerPluginUnref(qemu_driver->metadataLockManager);
+
     qemuDriverUnlock(qemu_driver);
     virMutexDestroy(&qemu_driver->lock);
     virThreadPoolFree(qemu_driver->workerPool);
@@ -1761,7 +1774,7 @@ qemudFindCharDevicePTYs(virDomainObjPtr vm,
     int ret, i;
 
     /* The order in which QEMU prints out the PTY paths is
-       the order in which it procsses its serial and parallel
+       the order in which it processes its serial and parallel
        device args. This code must match that ordering.... */
 
     /* first comes the serial devices */
@@ -2554,26 +2567,51 @@ struct qemudHookData {
     virConnectPtr conn;
     virDomainObjPtr vm;
     struct qemud_driver *driver;
+    char *lockState;
 };
 
 static int qemudSecurityHook(void *data) {
     struct qemudHookData *h = data;
+    virDomainLockPtr lock;
+    int ret = -1;
+
+    /* Some later calls want pid present */
+    h->vm->pid = getpid();
+
+    VIR_DEBUG0("Obtaining domain lock");
+    if (!(lock = virDomainLockForExec(h->driver->contentLockManager,
+                                      h->lockState,
+                                      h->vm)))
+        goto cleanup;
 
     /* This must take place before exec(), so that all QEMU
      * memory allocation is on the correct NUMA node
      */
+    VIR_DEBUG0("Moving procss to cgroup");
     if (qemuAddToCgroup(h->driver, h->vm->def) < 0)
-        return -1;
+        goto cleanup;
 
     /* This must be done after cgroup placement to avoid resetting CPU
      * affinity */
+    VIR_DEBUG0("Setup CPU affinity");
     if (qemudInitCpuAffinity(h->vm) < 0)
-        return -1;
+        goto cleanup;
 
+    VIR_DEBUG0("Setting up security labeling");
     if (virSecurityManagerSetProcessLabel(h->driver->securityManager, h->vm) < 0)
-        return -1;
+        goto cleanup;
 
-    return 0;
+    ret = 0;
+
+cleanup:
+    /* Free, but don't release the lock. The object lock
+     * must remain for lifetime of QEMU process.
+     */
+    VIR_DEBUG0("Releasing domain lock");
+    virDomainLockFree(lock);
+
+    VIR_DEBUG("Hook complete ret=%d", ret);
+    return ret;
 }
 
 static int
@@ -2619,11 +2657,13 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     char *timestamp;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virCommandPtr cmd = NULL;
+    virDomainLockPtr lock = NULL;
 
     struct qemudHookData hookData;
     hookData.conn = conn;
     hookData.vm = vm;
     hookData.driver = driver;
+    hookData.lockState = NULL; /* XXX */
 
     DEBUG0("Beginning VM startup process");
 
@@ -2662,11 +2702,6 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     }
     qemuDomainSecurityLabelAudit(vm, true);
 
-    DEBUG0("Generating setting domain security labels (if required)");
-    if (virSecurityManagerSetAllLabel(driver->securityManager,
-                                      vm, stdin_path) < 0)
-        goto cleanup;
-
     /* Ensure no historical cgroup for this VM is lying around bogus
      * settings */
     DEBUG0("Ensuring no historical cgroup is lying around");
@@ -2847,10 +2882,14 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     virCommandNonblockingFDs(cmd);
     virCommandSetPidFile(cmd, pidfile);
     virCommandDaemonize(cmd);
+    virCommandRequireHandshake(cmd);
 
     ret = virCommandRun(cmd, NULL);
-    VIR_WARN("Executing done %s", vm->def->emulator);
     VIR_FREE(pidfile);
+    /* QEMU isn't actually running properly yet. The child process
+     * exists, but is paused in the hook waiting for handshake to
+     * complete
+     */
 
     /* wait for qemu process to to show up */
     if (ret == 0) {
@@ -2863,7 +2902,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     } else if (ret == -2) {
         /*
          * XXX this is bogus. It isn't safe to set vm->pid = child
-         * because the child no longer exists.
+         * because the child no longer exists. Also the QEMU
+         * isn't really running properly
          */
 
         /* The virExec process that launches the daemon failed. Pending on
@@ -2878,6 +2918,31 @@ static int qemudStartVMDaemon(virConnectPtr conn,
 #endif
     }
 
+    VIR_DEBUG0("Waiting for handshake from child");
+    if (virCommandHandshakeWait(cmd) < 0) {
+        ret = -1;
+        goto cleanup;
+    }
+
+    VIR_DEBUG("Started handshake, doing locking %s", vm->def->emulator);
+    if (!(lock = virDomainLockForStartup(driver->contentLockManager,
+                                         driver->metadataLockManager,
+                                         NULL, /* XXX lock state */
+                                         vm)))
+        goto cleanup;
+
+    VIR_DEBUG0("Setting domain security labels");
+    if (virSecurityManagerSetAllLabel(driver->securityManager,
+                                      vm, stdin_path) < 0)
+        goto cleanup;
+
+    VIR_DEBUG0("Labelling done, completing handshake to child");
+    if (virCommandHandshakeNotify(cmd) < 0) {
+        ret = -1;
+        goto cleanup;
+    }
+    VIR_DEBUG0("Handshake complete, child running");
+
     if (migrateFrom)
         start_paused = true;
     vm->state = start_paused ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
@@ -2933,11 +2998,17 @@ static int qemudStartVMDaemon(virConnectPtr conn,
         goto cleanup;
 
     virCommandFree(cmd);
+    virDomainLockReleaseAndFree(lock);
     VIR_FORCE_CLOSE(logfile);
 
     return 0;
 
 cleanup:
+    /* Must release before we call into ShutdownVMDaemon() because
+     * that will re-aquire the lock in order to perform relabelling
+     */
+    virDomainLockReleaseAndFree(lock);
+
     /* We jump here if we failed to start the VM for any reason, or
      * if we failed to initialize the now running VM. kill it off and
      * pretend we never started it */
@@ -2960,6 +3031,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
     int logfile = -1;
     char *timestamp;
     char ebuf[1024];
+    virDomainLockPtr lock = NULL;
 
     VIR_DEBUG("Shutting down VM '%s' pid=%d migrated=%d",
               vm->def->name, vm->pid, migrated);
@@ -3040,8 +3112,14 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
     }
 
     /* Reset Security Labels */
-    virSecurityManagerRestoreAllLabel(driver->securityManager,
-                                      vm, migrated);
+    if ((lock = virDomainLockForShutdown(driver->contentLockManager,
+                                         driver->metadataLockManager,
+                                         vm)) != NULL) {
+        virSecurityManagerRestoreAllLabel(driver->securityManager,
+                                          vm, migrated);
+        virDomainLockReleaseAndFree(lock);
+        lock = NULL;
+    }
     virSecurityManagerReleaseLabel(driver->securityManager, vm);
 
     /* Clear out dynamically assigned labels */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8be993b..125d9b5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -38,6 +38,7 @@
 #include "pci.h"
 #include "files.h"
 #include "qemu_cgroup.h"
+#include "locking/domain_lock.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -51,6 +52,8 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
     int i;
     int ret;
     char *driveAlias = NULL;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    virDomainLockPtr lock;
 
     origdisk = NULL;
     for (i = 0 ; i < vm->def->ndisks ; i++) {
@@ -83,14 +86,28 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
         return -1;
     }
 
+    if (!(lock = virDomainLockForModify(driver->contentLockManager,
+                                        driver->metadataLockManager,
+                                        vm)))
+        return -1;
+
+    if (virDomainLockBeginDiskAttach(lock, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
+        return -1;
+    }
+
     if (virSecurityManagerSetImageLabel(driver->securityManager,
-                                        vm, disk) < 0)
+                                        vm, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
         return -1;
+    }
+
+    if (virDomainLockEndDiskAttach(lock, disk) < 0)
+        VIR_WARN("Unable to release lock on disk %s", disk->src);
 
     if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, qemuCmdFlags)))
         goto error;
 
-    qemuDomainObjPrivatePtr priv = vm->privateData;
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     if (disk->src) {
         const char *format = NULL;
@@ -113,9 +130,13 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
     if (ret < 0)
         goto error;
 
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, origdisk) < 0)
-        VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src);
+    if (virDomainLockBeginDiskDetach(lock, origdisk) >= 0) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, origdisk) < 0)
+            VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src);
+        if (virDomainLockEndDiskDetach(lock, origdisk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", origdisk->src);
+    }
 
     VIR_FREE(origdisk->src);
     origdisk->src = disk->src;
@@ -125,14 +146,22 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
     VIR_FREE(driveAlias);
 
     virDomainDiskDefFree(disk);
+    virDomainLockReleaseAndFree(lock);
 
     return ret;
 
 error:
     VIR_FREE(driveAlias);
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, disk) < 0)
-        VIR_WARN("Unable to restore security label on new media %s", disk->src);
+
+    if (virDomainLockBeginDiskDetach(lock, disk) >= 0) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, disk) < 0)
+            VIR_WARN("Unable to restore security label on new media %s", disk->src);
+        if (virDomainLockEndDiskDetach(lock, disk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", disk->src);
+    }
+    virDomainLockReleaseAndFree(lock);
+
     return -1;
 }
 
@@ -147,6 +176,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *devstr = NULL;
     char *drivestr = NULL;
+    virDomainLockPtr lock = NULL;
 
     for (i = 0 ; i < vm->def->ndisks ; i++) {
         if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -156,9 +186,24 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
         }
     }
 
+    if (!(lock = virDomainLockForModify(driver->contentLockManager,
+                                        driver->metadataLockManager,
+                                        vm)))
+        return -1;
+
+    if (virDomainLockBeginDiskAttach(lock, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
+        return -1;
+    }
+
     if (virSecurityManagerSetImageLabel(driver->securityManager,
-                                        vm, disk) < 0)
+                                        vm, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
         return -1;
+    }
+
+    if (virDomainLockEndDiskAttach(lock, disk) < 0)
+        VIR_WARN("Unable to release lock on disk %s", disk->src);
 
     if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
         if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0)
@@ -212,6 +257,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
 
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
+    virDomainLockReleaseAndFree(lock);
 
     return 0;
 
@@ -224,9 +270,14 @@ error:
         qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0)
         VIR_WARN("Unable to release PCI address on %s", disk->src);
 
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, disk) < 0)
-        VIR_WARN("Unable to restore security label on %s", disk->src);
+    if (virDomainLockBeginDiskDetach(lock, disk) >= 0) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, disk) < 0)
+            VIR_WARN("Unable to restore security label on %s", disk->src);
+        if (virDomainLockEndDiskDetach(lock, disk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", disk->src);
+    }
+    virDomainLockReleaseAndFree(lock);
 
     return -1;
 }
@@ -355,6 +406,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
     char *drivestr = NULL;
     char *devstr = NULL;
     int ret = -1;
+    virDomainLockPtr lock = NULL;
 
     for (i = 0 ; i < vm->def->ndisks ; i++) {
         if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -364,10 +416,24 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
         }
     }
 
+    if (!(lock = virDomainLockForModify(driver->contentLockManager,
+                                        driver->metadataLockManager,
+                                        vm)))
+        return -1;
+
+    if (virDomainLockBeginDiskAttach(lock, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
+        return -1;
+    }
 
     if (virSecurityManagerSetImageLabel(driver->securityManager,
-                                        vm, disk) < 0)
+                                        vm, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
         return -1;
+    }
+
+    if (virDomainLockEndDiskAttach(lock, disk) < 0)
+        VIR_WARN("Unable to release lock on disk %s", disk->src);
 
     /* We should have an address already, so make sure */
     if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
@@ -445,6 +511,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
 
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
+    virDomainLockReleaseAndFree(lock);
 
     return 0;
 
@@ -452,9 +519,14 @@ error:
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
 
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, disk) < 0)
-        VIR_WARN("Unable to restore security label on %s", disk->src);
+    if (virDomainLockBeginDiskDetach(lock, disk) >= 0) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, disk) < 0)
+            VIR_WARN("Unable to restore security label on %s", disk->src);
+        if (virDomainLockEndDiskDetach(lock, disk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", disk->src);
+    }
+    virDomainLockReleaseAndFree(lock);
 
     return -1;
 }
@@ -469,6 +541,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
     int i, ret;
     char *drivestr = NULL;
     char *devstr = NULL;
+    virDomainLockPtr lock = NULL;
 
     for (i = 0 ; i < vm->def->ndisks ; i++) {
         if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -478,10 +551,26 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
         }
     }
 
+    if (!(lock = virDomainLockForModify(driver->contentLockManager,
+                                        driver->metadataLockManager,
+                                        vm)))
+        return -1;
+
+    if (virDomainLockBeginDiskAttach(lock, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
+        return -1;
+    }
+
     if (virSecurityManagerSetImageLabel(driver->securityManager,
-                                        vm, disk) < 0)
+                                        vm, disk) < 0) {
+        virDomainLockReleaseAndFree(lock);
         return -1;
+    }
+
+    if (virDomainLockEndDiskAttach(lock, disk) < 0)
+        VIR_WARN("Unable to release lock on disk %s", disk->src);
 
+    /* XXX not correct once we allow attaching a USB CDROM */
     if (!disk->src) {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
                         "%s", _("disk source path is missing"));
@@ -528,6 +617,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
 
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
+    virDomainLockReleaseAndFree(lock);
 
     return 0;
 
@@ -535,9 +625,14 @@ error:
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
 
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, disk) < 0)
-        VIR_WARN("Unable to restore security label on %s", disk->src);
+    if (virDomainLockBeginDiskDetach(lock, disk) >= 0) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, disk) < 0)
+            VIR_WARN("Unable to restore security label on %s", disk->src);
+        if (virDomainLockEndDiskDetach(lock, disk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", disk->src);
+    }
+    virDomainLockReleaseAndFree(lock);
 
     return -1;
 }
@@ -1137,6 +1232,12 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virCgroupPtr cgroup = NULL;
     char *drivestr = NULL;
+    virDomainLockPtr lock = NULL;
+
+    if (!(lock = virDomainLockForModify(driver->contentLockManager,
+                                        driver->metadataLockManager,
+                                        vm)))
+        return -1;
 
     i = qemuFindDisk(vm->def, dev->data.disk->dst);
 
@@ -1201,9 +1302,13 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
 
     virDomainDiskDefFree(detach);
 
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, dev->data.disk) < 0)
-        VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
+    if (virDomainLockBeginDiskDetach(lock, dev->data.disk)) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, dev->data.disk) < 0)
+            VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
+        if (virDomainLockEndDiskDetach(lock, dev->data.disk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", dev->data.disk->src);
+    }
 
     if (cgroup != NULL) {
         if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
@@ -1215,6 +1320,8 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
 
 cleanup:
     VIR_FREE(drivestr);
+    virCgroupFree(&cgroup);
+    virDomainLockReleaseAndFree(lock);
     return ret;
 }
 
@@ -1228,6 +1335,12 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virCgroupPtr cgroup = NULL;
     char *drivestr = NULL;
+    virDomainLockPtr lock = NULL;
+
+    if (!(lock = virDomainLockForModify(driver->contentLockManager,
+                                        driver->metadataLockManager,
+                                        vm)))
+        return -1;
 
     i = qemuFindDisk(vm->def, dev->data.disk->dst);
 
@@ -1279,9 +1392,13 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
 
     virDomainDiskDefFree(detach);
 
-    if (virSecurityManagerRestoreImageLabel(driver->securityManager,
-                                            vm, dev->data.disk) < 0)
-        VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
+    if (virDomainLockBeginDiskDetach(lock, dev->data.disk)) {
+        if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+                                                vm, dev->data.disk) < 0)
+            VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
+        if (virDomainLockEndDiskDetach(lock, dev->data.disk) < 0)
+            VIR_WARN("Unable to release lock on disk %s", dev->data.disk->src);
+    }
 
     if (cgroup != NULL) {
         if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
@@ -1294,6 +1411,7 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
 cleanup:
     VIR_FREE(drivestr);
     virCgroupFree(&cgroup);
+    virDomainLockReleaseAndFree(lock);
     return ret;
 }
 
diff --git a/src/qemu/test_libvirtd_qemu.aug b/src/qemu/test_libvirtd_qemu.aug
index b4d8833..a3a669c 100644
--- a/src/qemu/test_libvirtd_qemu.aug
+++ b/src/qemu/test_libvirtd_qemu.aug
@@ -109,6 +109,9 @@ vnc_allow_host_audio = 1
 clear_emulator_capabilities = 0
 
 allow_disk_format_probing = 1
+
+content_lock_manager = \"fcntl\"
+metadata_lock_manager = \"fcntl\"
 "
 
    test Libvirtd_qemu.lns get conf =
@@ -228,3 +231,6 @@ allow_disk_format_probing = 1
 { "clear_emulator_capabilities" = "0" }
 { "#empty" }
 { "allow_disk_format_probing" = "1" }
+{ "#empty" }
+{ "content_lock_manager" = "fcntl" }
+{ "metadata_lock_manager" = "fcntl" }
-- 
1.7.3.4




More information about the libvir-list mailing list