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

Re: [libvirt] [PATCH 10/10] qemu: Error out when domain starting if the cdbfilter setting conflicts



On 2012年12月04日 21:17, Osier Yang wrote:
This prevents the domain starting if the shared disk's setting
conflicts with other active domain(s), E.g. A domain with
"cdbfilter" set as "yes", however, another active domain is using
it set as "no".

* src/conf/domain_conf.h: (Declare helper virDomainDiskFindByPath)
* src/conf/domain_conf.c: (Implement virDomainDiskFindByPath)
* src/libvirt_private.syms (export virDomainDiskFindByPath)
* src/qemu/qemu_process.c: (Error out if the shared disk's cdbfilter
                             conflicts with others)
---
  src/conf/domain_conf.c   |   13 ++++++++++++
  src/conf/domain_conf.h   |    2 +
  src/libvirt_private.syms |    1 +
  src/qemu/qemu_process.c  |   50 ++++++++++++++++++++++++++++++++++++++++-----
  4 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 50d853e..1d6bb1f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3062,6 +3062,19 @@ virDomainDiskFindControllerModel(virDomainDefPtr def,
      return model;
  }

+virDomainDiskDefPtr
+virDomainDiskFindByPath(virDomainDefPtr def,
+                        const char *path)
+{
+    int i;
+
+    for (i = 0; i<  def->ndisks; i++)
+        if (STREQ_NULLABLE(def->disks[i]->src, path))
+            return def->disks[i];
+
+    return NULL;
+}
+
  int
  virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def)
  {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d7c9b6b..2f24a3f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1897,6 +1897,8 @@ void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def);
  int virDomainDiskFindControllerModel(virDomainDefPtr def,
                                       virDomainDiskDefPtr disk,
                                       int controllerType);
+virDomainDiskDefPtr virDomainDiskFindByPath(virDomainDefPtr def,
+                                            const char *path);
  void virDomainControllerDefFree(virDomainControllerDefPtr def);
  void virDomainFSDefFree(virDomainFSDefPtr def);
  void virDomainActualNetDefFree(virDomainActualNetDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c2b01b9..41e4143 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -352,6 +352,7 @@ virDomainDiskDefFree;
  virDomainDiskDeviceTypeToString;
  virDomainDiskErrorPolicyTypeFromString;
  virDomainDiskErrorPolicyTypeToString;
+virDomainDiskFindByPath;
  virDomainDiskFindControllerModel;
  virDomainDiskGeometryTransTypeFromString;
  virDomainDiskGeometryTransTypeToString;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5cb068e..fac7d57 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3704,12 +3704,50 @@ int qemuProcessStart(virConnectPtr conn,
          if (disk->rawio == 1)
              virCommandAllowCap(cmd, CAP_SYS_RAWIO);

-        /* Add to qemud_driver->sharedDisks list if the disk is shared */
-        if (disk->shared&&
-            (qemuSharedDiskListAdd(driver->sharedDisks,
-                                   disk->src,
-                                   vm->def->name)<  0)) {
-            goto cleanup;
+        if (disk->shared) {
+            /* Error out if the cdbfilter setting is different with what
+             * other domain(s) uses.
+             */
+            qemuSharedDiskPtr entry = NULL;
+
+            if ((entry = qemuSharedDiskListFind(driver->sharedDisks,
+                                                disk->src,
+                                                NULL,
+                                                NULL))) {
+                for (i = 0; i<  entry->ndomains; i++) {
+                    virDomainObjPtr domobj = NULL;
+                    virDomainDiskDefPtr diskdef = NULL;
+
+                    if (!(domobj = virDomainFindByName(&driver->domains,
+                                                       entry->domains[i])))
+                        goto cleanup;
+
+                    if (!(diskdef = virDomainDiskFindByPath(domobj->def,
+                                                            disk->src))) {
+                        virDomainObjUnlock(domobj);
+                        goto cleanup;
+                    }
+
+                    /* XXX: Can be abstracted into a function when there
+                     * are more stuffs to check in future.
+                     */
+                    if (diskdef->cdbfilter != disk->cdbfilter) {
+                        virReportError(VIR_ERR_INTERNAL_ERROR,
+                                       _("cdbfilter of shared disk '%s' "
+                                         "conflicts with other active "
+                                         "domains"), disk->src);
+                        virDomainObjUnlock(domobj);
+                        goto cleanup;
+                    }
+                    virDomainObjUnlock(domobj);
+                }
+            }
+
+            /* Add to qemud_driver->sharedDisks list if the disk is shared */
+            if (qemuSharedDiskListAdd(driver->sharedDisks,
+                                      disk->src,
+                                      vm->def->name)<  0)
+                goto cleanup;
          }

          if (!disk->cdbfilter)

With the following patch squashed in (to avoid the deadlock,
it's not that comfortable to have nest lock/unlock, but I don't
have better idea temporally).

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fac7d57..5284c74 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3714,17 +3714,21 @@ int qemuProcessStart(virConnectPtr conn,
                                                 disk->src,
                                                 NULL,
                                                 NULL))) {
+                virDomainObjUnlock(vm);
                 for (i = 0; i < entry->ndomains; i++) {
                     virDomainObjPtr domobj = NULL;
                     virDomainDiskDefPtr diskdef = NULL;

                     if (!(domobj = virDomainFindByName(&driver->domains,
-                                                       entry->domains[i])))
+ entry->domains[i]))) {
+                        virDomainObjLock(vm);
                         goto cleanup;
+                    }

                     if (!(diskdef = virDomainDiskFindByPath(domobj->def,
                                                             disk->src))) {
                         virDomainObjUnlock(domobj);
+                        virDomainObjLock(vm);
                         goto cleanup;
                     }

@@ -3737,10 +3741,12 @@ int qemuProcessStart(virConnectPtr conn,
                                          "conflicts with other active "
                                          "domains"), disk->src);
                         virDomainObjUnlock(domobj);
+                        virDomainObjLock(vm);
                         goto cleanup;
                     }
                     virDomainObjUnlock(domobj);
                 }
+                virDomainObjLock(vm);
             }

/* Add to qemud_driver->sharedDisks list if the disk is shared */


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