Re: [PATCH 6/8] qemu: Translate the pool disk source earlier

Osier Yang <jyang@xxxxxxxxxx> · Mon, 08 Apr 2013 19:08:18 +0800

On 06/04/13 08:34, John Ferlan wrote:
On 04/04/2013 03:38 PM, Osier Yang wrote:
To support "shareable" for volume type disk, we have to translate
the source before trying to add the shared disk entry. To archieve
s/archeive/achieve/

the goal, this moves the helper qemuTranslateDiskSourcePool into
src/qemu/qemu_conf.c, and introduce a internal only member (voltype)
s/a internal/an internal/

for struct _virDomainDiskSourcePoolDef, to record the underlying
volume type, for use when building the drive string.
s/type,/type/

Later patch will support "shareable" volume type disk.
---
  src/conf/domain_conf.h  |  1 +
  src/qemu/qemu_command.c | 56 +------------------------------------------------
  src/qemu/qemu_command.h |  1 -
  src/qemu/qemu_conf.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++
  src/qemu/qemu_conf.h    |  3 +++
  src/qemu/qemu_driver.c  | 16 ++++++++++----
  src/qemu/qemu_process.c |  7 +++++++
  7 files changed, 76 insertions(+), 60 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 988636e..13d6d89 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -611,6 +611,7 @@ typedef struct _virDomainDiskSourcePoolDef virDomainDiskSourcePoolDef;
  struct _virDomainDiskSourcePoolDef {
      char *pool; /* pool name */
      char *volume; /* volume name */
+    int voltype; /* enum virStorageVolType, internal only */
  };
  typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
  
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cdfe801..c29796d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2681,56 +2681,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op
      return 0;
  }
  
-static int
-qemuTranslateDiskSourcePool(virConnectPtr conn,
-                            virDomainDiskDefPtr def,
-                            int *voltype)
-{
-    virStoragePoolPtr pool = NULL;
-    virStorageVolPtr vol = NULL;
-    virStorageVolInfo info;
-    int ret = -1;
-
-    if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
-        return 0;
-
-    if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
-        return -1;
-
-    if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume)))
-        goto cleanup;
-
-    if (virStorageVolGetInfo(vol, &info) < 0)
-        goto cleanup;
-
-    if (def->startupPolicy &&
-        info.type != VIR_STORAGE_VOL_FILE) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("'startupPolicy' is only valid for 'file' type volume"));
-        goto cleanup;
-    }
-
-    switch (info.type) {
-    case VIR_STORAGE_VOL_FILE:
-    case VIR_STORAGE_VOL_BLOCK:
-    case VIR_STORAGE_VOL_DIR:
-        if (!(def->src = virStorageVolGetPath(vol)))
-            goto cleanup;
-        break;
-    case VIR_STORAGE_VOL_NETWORK:
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("Using network volume as disk source is not supported"));
-        goto cleanup;
-    }
-
-    *voltype = info.type;
-    ret = 0;
-cleanup:
-    virStoragePoolFree(pool);
-    virStorageVolFree(vol);
-    return ret;
-}
-
  char *
  qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
                    virDomainDiskDefPtr disk,
@@ -2743,7 +2693,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
          virDomainDiskGeometryTransTypeToString(disk->geometry.trans);
      int idx = virDiskNameToIndex(disk->dst);
      int busid = -1, unitid = -1;
-    int voltype = -1;
  
      if (idx < 0) {
          virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2751,9 +2700,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
          goto error;
      }
  
-    if (qemuTranslateDiskSourcePool(conn, disk, &voltype) < 0)
-        goto error;
-
      switch (disk->bus) {
      case VIR_DOMAIN_DISK_BUS_SCSI:
          if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
@@ -2889,7 +2835,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
                  break;
              }
          } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
-            switch (voltype) {
+            switch (disk->srcpool->voltype) {
              case VIR_STORAGE_VOL_DIR:
                  if (!disk->readonly) {
                      virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 17687f4..20e3066 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -237,5 +237,4 @@ qemuParseKeywords(const char *str,
                    char ***retvalues,
                    int allowEmptyValue);
  
-
  #endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c2e2e10..8ae690f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1240,3 +1240,55 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
  {
      return virAtomicIntInc(&driver->nextvmid);
  }
+
+int
+qemuTranslateDiskSourcePool(virConnectPtr conn,
+                            virDomainDiskDefPtr def)
+{
+    virStoragePoolPtr pool = NULL;
+    virStorageVolPtr vol = NULL;
+    virStorageVolInfo info;
+    int ret = -1;
+
+    if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
+        return 0;
+
+    if (!def->srcpool)
+        return 0;
+
+    if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
+        return -1;
+
+    if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume)))
+        goto cleanup;
+
+    if (virStorageVolGetInfo(vol, &info) < 0)
+        goto cleanup;
+
+    if (def->startupPolicy &&
+        info.type != VIR_STORAGE_VOL_FILE) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("'startupPolicy' is only valid for 'file' type volume"));
+        goto cleanup;
+    }
+
+    switch (info.type) {
+    case VIR_STORAGE_VOL_FILE:
+    case VIR_STORAGE_VOL_BLOCK:
+    case VIR_STORAGE_VOL_DIR:
+        if (!(def->src = virStorageVolGetPath(vol)))
+            goto cleanup;
+        break;
+    case VIR_STORAGE_VOL_NETWORK:
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Using network volume as disk source is not supported"));
+        goto cleanup;
+    }
+
+    def->srcpool->voltype = info.type;
+    ret = 0;
+cleanup:
+    virStoragePoolFree(pool);
+    virStorageVolFree(vol);
+    return ret;
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c5ddaad..9f58454 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -303,4 +303,7 @@ void qemuSharedDiskEntryFree(void *payload, const void *name)
  int qemuDriverAllocateID(virQEMUDriverPtr driver);
  virDomainXMLConfPtr virQEMUDriverCreateXMLConf(void);
  
+int qemuTranslateDiskSourcePool(virConnectPtr conn,
+                                virDomainDiskDefPtr def);
+
  #endif /* __QEMUD_CONF_H */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 552a81b..4e8c666 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5748,6 +5748,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
          goto end;
      }
  
+    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
+        goto end;
+
      if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
          goto end;
  
@@ -6016,7 +6019,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
  }
  
  static int
-qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
+qemuDomainChangeDiskMediaLive(virConnectPtr conn,
+                              virDomainObjPtr vm,
                                virDomainDeviceDefPtr dev,
                                virQEMUDriverPtr driver,
                                bool force)
@@ -6029,6 +6033,9 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
      virCapsPtr caps = NULL;
      int ret = -1;
  
+    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
+        goto end;
+
      if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
          goto end;
  
@@ -6107,7 +6114,8 @@ end:
  }
  
  static int
-qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
+qemuDomainUpdateDeviceLive(virConnectPtr conn,
+                           virDomainObjPtr vm,
                             virDomainDeviceDefPtr dev,
                             virDomainPtr dom,
                             bool force)
@@ -6117,7 +6125,7 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
  
      switch (dev->type) {
      case VIR_DOMAIN_DEVICE_DISK:
-        ret = qemuDomainChangeDiskMediaLive(vm, dev, driver, force);
+        ret = qemuDomainChangeDiskMediaLive(conn, vm, dev, driver, force);
          break;
      case VIR_DOMAIN_DEVICE_GRAPHICS:
          ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
@@ -6529,7 +6537,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
              ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom);
              break;
          case QEMU_DEVICE_UPDATE:
-            ret = qemuDomainUpdateDeviceLive(vm, dev_copy, dom, force);
+            ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force);
              break;
          default:
              virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8c4bfb7..3ac0c70 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3008,6 +3008,8 @@ qemuProcessReconnect(void *opaque)
       * qemu_driver->sharedDisks.
       */
      for (i = 0; i < obj->def->ndisks; i++) {
+        if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0)
+            goto error;
          if (qemuAddSharedDisk(driver, obj->def->disks[i],
                                obj->def->name) < 0)
              goto error;
@@ -3556,6 +3558,11 @@ int qemuProcessStart(virConnectPtr conn,
              goto cleanup;
      }
  
+    for (i = 0; i < vm->def->ndisks; i++) {
+        if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0)
+            goto cleanup;
+    }
+
      VIR_DEBUG("Building emulator command line");
      if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
                                       priv->monJSON != 0, priv->qemuCaps,


ACK; however, I see paths to qemuBuildDriveStr() that don't go through
qemuBuildCommandLine(), so just double check that you aren't missing a
place to get that disk->src set for one of these pool/vol's.  The point
being that qemuBuildDriveStr() "had" code that did something before
(albeit in this set of patches). Since you're moving it - just be sure
there's no path that could enter qemuBuildDriveStr() that would need it.


Yeah, I expect there are still places missed, but as I replied for 0/8.
Let's find them out step by step.

Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list







