[libvirt] [PATCH 3/3] storage: Refactor location of metadata for storage drive access to files

Peter Krempa pkrempa at redhat.com
Tue Apr 8 14:12:23 UTC 2014


Now that we store all metadata about a storage image in a
virStorageSource struct lets use it also to store information needed by
the storage driver to access and do operations on the files.
---
 src/qemu/qemu_driver.c                | 25 +++++------
 src/storage/storage_backend.h         | 20 ++++++---
 src/storage/storage_backend_fs.c      | 12 ++---
 src/storage/storage_backend_gluster.c | 33 +++++++-------
 src/storage/storage_driver.c          | 82 ++++++++++++++++-------------------
 src/storage/storage_driver.h          | 27 +++---------
 src/util/virstoragefile.h             |  5 +++
 7 files changed, 96 insertions(+), 108 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d824e24..72834c2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12465,7 +12465,6 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
                                       bool active,
                                       bool reuse)
 {
-    virStorageFilePtr snapfile = NULL;
     int ret = -1;
     struct stat st;

@@ -12489,10 +12488,10 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
             return -1;
     }

-    if (!(snapfile = virStorageFileInit(&snapdisk->src)))
+    if (virStorageFileInit(&snapdisk->src) < 0)
         return -1;

-    if (virStorageFileStat(snapfile, &st) < 0) {
+    if (virStorageFileStat(&snapdisk->src, &st) < 0) {
         if (errno != ENOENT) {
             virReportSystemError(errno,
                                  _("unable to stat for disk %s: %s"),
@@ -12515,7 +12514,7 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
     ret = 0;

  cleanup:
-    virStorageFileFree(snapfile);
+    virStorageFileDeinit(&snapdisk->src);
     return ret;
 }

@@ -12738,7 +12737,6 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     int ret = -1;
     int fd = -1;
     bool need_unlink = false;
-    virStorageFilePtr snapfile = NULL;

     if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -12757,7 +12755,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     virStorageFileFreeMetadata(disk->backingChain);
     disk->backingChain = NULL;

-    if (!(snapfile = virStorageFileInit(&snap->src)))
+    if (virStorageFileInit(&snap->src) < 0)
         goto cleanup;

     if (qemuDomainSnapshotDiskGetSourceString(snap, &source) < 0)
@@ -12886,9 +12884,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     }

  cleanup:
-    if (need_unlink && virStorageFileUnlink(snapfile))
+    if (need_unlink && virStorageFileUnlink(&snap->src))
         VIR_WARN("unable to unlink just-created %s", source);
-    virStorageFileFree(snapfile);
+    virStorageFileDeinit(&snap->src);
     VIR_FREE(device);
     VIR_FREE(source);
     VIR_FREE(newsource);
@@ -12911,10 +12909,9 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
 {
     char *source = NULL;
     char *persistSource = NULL;
-    virStorageFilePtr diskfile = NULL;
     struct stat st;

-    diskfile = virStorageFileInit(&disk->src);
+    ignore_value(virStorageFileInit(&disk->src));

     if (VIR_STRDUP(source, origdisk->src.path) < 0 ||
         (persistDisk && VIR_STRDUP(persistSource, source) < 0))
@@ -12922,9 +12919,9 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,

     qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src.path,
                                       VIR_DISK_CHAIN_NO_ACCESS);
-    if (need_unlink && diskfile &&
-        virStorageFileStat(diskfile, &st) == 0 && S_ISREG(st.st_mode) &&
-        virStorageFileUnlink(diskfile) < 0)
+    if (need_unlink &&
+        virStorageFileStat(&disk->src, &st) == 0 && S_ISREG(st.st_mode) &&
+        virStorageFileUnlink(&disk->src) < 0)
         VIR_WARN("Unable to remove just-created %s", disk->src.path);

     /* Update vm in place to match changes.  */
@@ -12953,7 +12950,7 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
     }

  cleanup:
-    virStorageFileFree(diskfile);
+    virStorageFileDeinit(&disk->src);
     VIR_FREE(source);
     VIR_FREE(persistSource);
 }
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 4f95000..5997077 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -160,24 +160,34 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
                                   int imgformat);

 /* ------- virStorageFile backends ------------ */
+typedef struct _virStorageFileBackend virStorageFileBackend;
+typedef virStorageFileBackend *virStorageFileBackendPtr;
+
+struct _virStorageDriverData {
+    virStorageFileBackendPtr backend;
+    void *priv;
+};
+
 typedef int
-(*virStorageFileBackendInit)(virStorageFilePtr file);
+(*virStorageFileBackendInit)(virStorageSourcePtr src);

 typedef void
-(*virStorageFileBackendDeinit)(virStorageFilePtr file);
+(*virStorageFileBackendDeinit)(virStorageSourcePtr src);

 typedef int
-(*virStorageFileBackendCreate)(virStorageFilePtr file);
+(*virStorageFileBackendCreate)(virStorageSourcePtr src);

 typedef int
-(*virStorageFileBackendUnlink)(virStorageFilePtr file);
+(*virStorageFileBackendUnlink)(virStorageSourcePtr src);

 typedef int
-(*virStorageFileBackendStat)(virStorageFilePtr file,
+(*virStorageFileBackendStat)(virStorageSourcePtr src,
                              struct stat *st);

 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);

+
+
 struct _virStorageFileBackend {
     int type;
     int protocol;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4e4a7ae..bb20385 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1335,31 +1335,31 @@ virStorageBackend virStorageBackendNetFileSystem = {


 static int
-virStorageFileBackendFileUnlink(virStorageFilePtr file)
+virStorageFileBackendFileUnlink(virStorageSourcePtr src)
 {
     int ret;

-    ret = unlink(file->path);
+    ret = unlink(src->path);
     /* preserve errno */

     VIR_DEBUG("removing storage file %p(%s): ret=%d, errno=%d",
-              file, file->path, ret, errno);
+              src, src->path, ret, errno);

     return ret;
 }


 static int
-virStorageFileBackendFileStat(virStorageFilePtr file,
+virStorageFileBackendFileStat(virStorageSourcePtr src,
                               struct stat *st)
 {
     int ret;

-    ret = stat(file->path, st);
+    ret = stat(src->path, st);
     /* preserve errno */

     VIR_DEBUG("stat of storage file %p(%s): ret=%d, errno=%d",
-              file, file->path, ret, errno);
+              src, src->path, ret, errno);

     return ret;
 }
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index d131f13..2593c1c 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -546,41 +546,41 @@ struct _virStorageFileBackendGlusterPriv {


 static void
-virStorageFileBackendGlusterDeinit(virStorageFilePtr file)
+virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
 {
     VIR_DEBUG("deinitializing gluster storage file %p(%s/%s)",
-              file, file->hosts[0].name, file->path);
-    virStorageFileBackendGlusterPrivPtr priv = file->priv;
+              src, src->hosts[0].name, src->path);
+    virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;

     if (priv->vol)
         glfs_fini(priv->vol);
     VIR_FREE(priv->volname);

     VIR_FREE(priv);
-    file->priv = NULL;
+    src->drv->priv = NULL;
 }

 static int
-virStorageFileBackendGlusterInit(virStorageFilePtr file)
+virStorageFileBackendGlusterInit(virStorageSourcePtr src)
 {
     virStorageFileBackendGlusterPrivPtr priv = NULL;
-    virStorageNetHostDefPtr host = &(file->hosts[0]);
+    virStorageNetHostDefPtr host = &(src->hosts[0]);
     const char *hostname = host->name;
     int port = 0;

     VIR_DEBUG("initializing gluster storage file %p(%s/%s)",
-              file, hostname, file->path);
+              src, hostname, src->path);

     if (VIR_ALLOC(priv) < 0)
         return -1;

-    if (VIR_STRDUP(priv->volname, file->path) < 0)
+    if (VIR_STRDUP(priv->volname, src->path) < 0)
         goto error;

     if (!(priv->path = strchr(priv->volname, '/'))) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("invalid path of gluster volume: '%s'"),
-                       file->path);
+                       src->path);
         goto error;
     }

@@ -598,7 +598,6 @@ virStorageFileBackendGlusterInit(virStorageFilePtr file)
     if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX)
         hostname = host->socket;

-
     if (!(priv->vol = glfs_new(priv->volname))) {
         virReportOOMError();
         goto error;
@@ -620,7 +619,7 @@ virStorageFileBackendGlusterInit(virStorageFilePtr file)
         goto error;
     }

-    file->priv = priv;
+    src->drv->priv = priv;

     return 0;

@@ -635,32 +634,32 @@ virStorageFileBackendGlusterInit(virStorageFilePtr file)


 static int
-virStorageFileBackendGlusterUnlink(virStorageFilePtr file)
+virStorageFileBackendGlusterUnlink(virStorageSourcePtr src)
 {
-    virStorageFileBackendGlusterPrivPtr priv = file->priv;
+    virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
     int ret;

     ret = glfs_unlink(priv->vol, priv->path);
     /* preserve errno */

     VIR_DEBUG("removing storage file %p(%s/%s): ret=%d, errno=%d",
-              file, file->hosts[0].name, file->path, ret, errno);
+              src, src->hosts[0].name, src->path, ret, errno);
     return ret;
 }


 static int
-virStorageFileBackendGlusterStat(virStorageFilePtr file,
+virStorageFileBackendGlusterStat(virStorageSourcePtr src,
                                  struct stat *st)
 {
-    virStorageFileBackendGlusterPrivPtr priv = file->priv;
+    virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
     int ret;

     ret = glfs_stat(priv->vol, priv->path, st);
     /* preserve errno */

     VIR_DEBUG("stat of storage file %p(%s/%s): ret=%d, errno=%d",
-              file, file->hosts[0].name, file->path, ret, errno);
+              src, src->hosts[0].name, src->path, ret, errno);
     return ret;
 }

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index b26d791..de0e04c 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2731,82 +2731,74 @@ int storageRegister(void)


 /* ----------- file handlers cooperating with storage driver --------------- */
+static bool
+virStorageFileIsInitialized(virStorageSourcePtr src)
+{
+    return !!src->drv;
+}
+
 void
-virStorageFileFree(virStorageFilePtr file)
+virStorageFileDeinit(virStorageSourcePtr src)
 {
-    if (!file)
+    if (!virStorageFileIsInitialized(src))
         return;

-    if (file->backend &&
-        file->backend->backendDeinit)
-        file->backend->backendDeinit(file);
+    if (src->drv->backend &&
+        src->drv->backend->backendDeinit)
+        src->drv->backend->backendDeinit(src);

-    VIR_FREE(file->path);
-    virStorageNetHostDefFree(file->nhosts, file->hosts);
-    VIR_FREE(file);
+    VIR_FREE(src->drv);
 }


-virStorageFilePtr
+int
 virStorageFileInit(virStorageSourcePtr src)
 {
-    virStorageFilePtr file = NULL;
-
-    if (VIR_ALLOC(file) < 0)
-        return NULL;
-
-    file->type = virStorageSourceGetActualType(src);
-    file->protocol = src->protocol;
-    file->nhosts = src->nhosts;
-
-    if (VIR_STRDUP(file->path, src->path) < 0)
-        goto error;
-
-    if (!(file->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts)))
-        goto error;
+    int actualType = virStorageSourceGetActualType(src);
+    if (VIR_ALLOC(src->drv) < 0)
+        return -1;

-    if (!(file->backend = virStorageFileBackendForType(file->type,
-                                                       file->protocol)))
+    if (!(src->drv->backend = virStorageFileBackendForType(actualType,
+                                                           src->protocol)))
         goto error;

-    if (file->backend->backendInit &&
-        file->backend->backendInit(file) < 0)
+    if (src->drv->backend->backendInit &&
+        src->drv->backend->backendInit(src) < 0)
         goto error;

-    return file;
+    return 0;

  error:
-    VIR_FREE(file->path);
-    virStorageNetHostDefFree(file->nhosts, file->hosts);
-    VIR_FREE(file);
-    return NULL;
+    VIR_FREE(src->drv);
+    return -1;
 }


 /**
  * virStorageFileCreate: Creates an empty storage file via storage driver
  *
- * @file: file structure pointing to the file
+ * @src: file structure pointing to the file
  *
  * Returns 0 on success, -2 if the function isn't supported by the backend,
  * -1 on other failure. Errno is set in case of failure.
  */
 int
-virStorageFileCreate(virStorageFilePtr file)
+virStorageFileCreate(virStorageSourcePtr src)
 {
-    if (!file->backend->storageFileCreate) {
+    if (!virStorageFileIsInitialized(src) ||
+        !src->drv->backend->storageFileCreate) {
         errno = ENOSYS;
         return -2;
     }

-    return file->backend->storageFileCreate(file);
+    return src->drv->backend->storageFileCreate(src);
 }


 /**
  * virStorageFileUnlink: Unlink storage file via storage driver
  *
- * @file: file structure pointing to the file
+ * @src: file structure pointing to the file
  *
  * Unlinks the file described by the @file structure.
  *
@@ -2814,34 +2806,36 @@ virStorageFileCreate(virStorageFilePtr file)
  * -1 on other failure. Errno is set in case of failure.
  */
 int
-virStorageFileUnlink(virStorageFilePtr file)
+virStorageFileUnlink(virStorageSourcePtr src)
 {
-    if (!file->backend->storageFileUnlink) {
+    if (!virStorageFileIsInitialized(src) ||
+        !src->drv->backend->storageFileUnlink) {
         errno = ENOSYS;
         return -2;
     }

-    return file->backend->storageFileUnlink(file);
+    return src->drv->backend->storageFileUnlink(src);
 }


 /**
  * virStorageFileStat: returns stat struct of a file via storage driver
  *
- * @file: file structure pointing to the file
+ * @src: file structure pointing to the file
  * @stat: stat structure to return data
  *
  * Returns 0 on success, -2 if the function isn't supported by the backend,
  * -1 on other failure. Errno is set in case of failure.
 */
 int
-virStorageFileStat(virStorageFilePtr file,
+virStorageFileStat(virStorageSourcePtr src,
                    struct stat *st)
 {
-    if (!(file->backend->storageFileStat)) {
+    if (!virStorageFileIsInitialized(src) ||
+        !src->drv->backend->storageFileStat) {
         errno = ENOSYS;
         return -2;
     }

-    return file->backend->storageFileStat(file, st);
+    return src->drv->backend->storageFileStat(src, st);
 }
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index d5dda00..e1f5ffd 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -28,30 +28,13 @@
 # include "virstoragefile.h"
 # include <sys/stat.h>

-typedef struct _virStorageFileBackend virStorageFileBackend;
-typedef virStorageFileBackend *virStorageFileBackendPtr;
-
-typedef struct _virStorageFile virStorageFile;
-typedef virStorageFile *virStorageFilePtr;
-struct _virStorageFile {
-    virStorageFileBackendPtr backend;
-    void *priv;
-
-    char *path;
-    int type;
-    int protocol;
-
-    size_t nhosts;
-    virStorageNetHostDefPtr hosts;
-};
-
-virStorageFilePtr
+int
 virStorageFileInit(virStorageSourcePtr src);
-void virStorageFileFree(virStorageFilePtr file);
+void virStorageFileDeinit(virStorageSourcePtr src);

-int virStorageFileCreate(virStorageFilePtr file);
-int virStorageFileUnlink(virStorageFilePtr file);
-int virStorageFileStat(virStorageFilePtr file,
+int virStorageFileCreate(virStorageSourcePtr src);
+int virStorageFileUnlink(virStorageSourcePtr src);
+int virStorageFileStat(virStorageSourcePtr src,
                        struct stat *stat);

 int storageRegister(void);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 22198a8..7cb0f2b 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -208,6 +208,8 @@ enum virStorageSecretType {
     VIR_STORAGE_SECRET_TYPE_LAST
 };

+typedef struct _virStorageDriverData virStorageDriverData;
+typedef virStorageDriverData *virStorageDriverDataPtr;

 typedef struct _virStorageSource virStorageSource;
 typedef virStorageSource *virStorageSourcePtr;
@@ -243,6 +245,9 @@ struct _virStorageSource {
     unsigned long long capacity; /* in bytes, 0 if unknown */
     size_t nseclabels;
     virSecurityDeviceLabelDefPtr *seclabels;
+
+    /* metadata for storage driver access to remote and local volumes */
+    virStorageDriverDataPtr drv;
 };


-- 
1.9.1




More information about the libvir-list mailing list