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

Re: [libvirt] [PATCH 4/9] Implement translateDiskSourcePool



On 2013年02月01日 01:42, John Ferlan wrote:
On 01/30/2013 01:11 PM, Osier Yang wrote:
It iterates over all the domain disks, and translate the source of
all the disks of 'volume' type from storage pool/volume to the real
underlying source.

Disks of type 'file', 'block', and 'dir' are supported now. Network
type is not supported yet, it will be another patch.

src/storage/storage_driver.c:
   * New helper storagePoolObjFindByDiskSource to find the specified
     pool by name.
   * Implement translateDiskSourcePool as storageTranslateDomainDiskSourcePool
---
  src/storage/storage_driver.c |   90 ++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index e98c18c..9f60f2c 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -47,6 +47,8 @@
  #include "virlog.h"
  #include "virfile.h"
  #include "fdstream.h"
+#include "domain_conf.h"
+#include "domain_storage.h"
  #include "configmake.h"

  #define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -2367,6 +2369,88 @@ storageListAllPools(virConnectPtr conn,
      return ret;
  }

+static virStoragePoolObjPtr
+storagePoolObjFindByDiskSource(virConnectPtr conn,
+                               const char *name)
+{
+    virStorageDriverStatePtr driver = conn->storagePrivateData;
+    virStoragePoolObjPtr pool = NULL;
+
+    storageDriverLock(driver);
+    pool = virStoragePoolObjFindByName(&driver->pools, name);
+    storageDriverUnlock(driver);
+
+    if (!pool) {
+        virReportError(VIR_ERR_NO_STORAGE_POOL,
+                       _("no storage pool with matching name '%s'"),
+                       name);
+        goto error;
+    }
+
+    if (!virStoragePoolObjIsActive(pool)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("The specified pool '%s' is not active"),
+                       name);
+        goto error;
+    }
+
+    return pool;
+error:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
+    return NULL;
+}
+
+static int
+storageTranslateDomainDiskSourcePool(virConnectPtr conn,
+                                     virDomainDefPtr def)
+{
+    virStoragePoolObjPtr pool = NULL;
+    virStorageVolDefPtr vol = NULL;
+    int i;
+    int ret = -1;
+
+    for (i = 0; i<  def->ndisks; i++) {
+        virDomainDiskDefPtr disk = def->disks[i];
+
+        if (disk->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
+            continue;
+
+        if (!(pool = storagePoolObjFindByDiskSource(conn, disk->srcpool->pool)))
+            goto cleanup;
+
+        if (!(vol = virStorageVolDefFindByName(pool, disk->srcpool->volume))) {
+            virReportError(VIR_ERR_NO_STORAGE_VOL,
+                           _("no storage vol of specified pool with "
+                             "matching name '%s'"),
+                           disk->srcpool->volume);
s/vol/volume

Since you have the pool name, wouldn't it be better to state that volume
"%s" was not found in pool "%s"?

Agreed.


Although I now see you just hoisted the message from elsewhere.

+            goto cleanup;
+        }
+
+        switch (vol->type) {
+        case VIR_STORAGE_VOL_FILE:
+        case VIR_STORAGE_VOL_BLOCK:
+        case VIR_STORAGE_VOL_DIR:
+            disk->src = strdup(vol->target.path);

And if !disk->src here what assumptions get broken later?

Hum, this inspires me notice vol->target.path can be NULL (it's not
mandatory in vol XML). And it should error out as long as the the disk
is not CD-ROM or Floppy.

But I don't think it's neccesary to check the return value of strdup.

Will post v2.


+            break;
+        case VIR_STORAGE_VOL_NETWORK:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("Using network volume as disk source is not supported"));
+            goto cleanup;
+        }
+
+        virStoragePoolObjUnlock(pool);
+        pool = NULL;
+    }
+
+    ret = 0;
+
+cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
+    return ret;
+}
+
  static virStorageDriver storageDriver = {
      .name = "storage",
      .open = storageOpen, /* 0.4.0 */
@@ -2423,8 +2507,14 @@ static virStateDriver stateDriver = {
      .reload = storageDriverReload,
  };

+static virDomainStorageDriver domainStorageDriver = {
+    .translateDiskSourcePool = storageTranslateDomainDiskSourcePool,
+};
+
+
  int storageRegister(void) {
      virRegisterStorageDriver(&storageDriver);
      virRegisterStateDriver(&stateDriver);
+    virRegisterDomainStorageDriver(&domainStorageDriver);
      return 0;
  }


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


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