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

Re: [libvirt] [PATCH]: Allow arbitrary paths to virStorageVolLookupByPath



Daniel P. Berrange wrote:
>> Personally, I think those are bad semantics for virStorageBackendStablePath;
>> assuming it succeeds, you should always be able to know that you have a copy,
>> regardless of whether the copy is the same as the original.  Should I change
>> virStorageBackendStablePath to those semantics, in which case your below code
>> would then be correct?
> 
> Yes, I think that's worth doing - will also avoid the cast in the input
> arg there

OK, updated patch attached; virStorageBackendStablePath now always returns a
copy of the given string, so it's always safe to unconditionally VIR_FREE it.  I
fixed up storage_backend_iscsi and storage_backend_disk to reflect this change.
 I also re-worked the code as you suggested, and added a bit more error checking.

Signed-off-by: Chris Lalancette <clalance redhat com>
Index: src/storage_backend.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.24
diff -u -r1.24 storage_backend.c
--- src/storage_backend.c	28 Oct 2008 17:48:06 -0000	1.24
+++ src/storage_backend.c	31 Oct 2008 11:56:33 -0000
@@ -357,7 +357,7 @@
 char *
 virStorageBackendStablePath(virConnectPtr conn,
                             virStoragePoolObjPtr pool,
-                            char *devpath)
+                            const char *devpath)
 {
     DIR *dh;
     struct dirent *dent;
@@ -366,7 +366,7 @@
     if (pool->def->target.path == NULL ||
         STREQ(pool->def->target.path, "/dev") ||
         STREQ(pool->def->target.path, "/dev/"))
-        return devpath;
+        return strdup(devpath);
 
     /* The pool is pointing somewhere like /dev/disk/by-path
      * or /dev/disk/by-id, so we need to check all symlinks in
@@ -410,7 +410,7 @@
     /* Couldn't find any matching stable link so give back
      * the original non-stable dev path
      */
-    return devpath;
+    return strdup(devpath);
 }
 
 
Index: src/storage_backend.h
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.9
diff -u -r1.9 storage_backend.h
--- src/storage_backend.h	23 Oct 2008 11:32:22 -0000	1.9
+++ src/storage_backend.h	31 Oct 2008 11:56:34 -0000
@@ -50,6 +50,7 @@
     VIR_STORAGE_BACKEND_POOL_SOURCE_DIR     = (1<<2),
     VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER = (1<<3),
     VIR_STORAGE_BACKEND_POOL_SOURCE_NAME    = (1<<4),
+    VIR_STORAGE_BACKEND_POOL_STABLE_PATH    = (1<<5),
 };
 
 enum partTableType {
@@ -138,7 +139,7 @@
 
 char *virStorageBackendStablePath(virConnectPtr conn,
                                   virStoragePoolObjPtr pool,
-                                  char *devpath);
+                                  const char *devpath);
 
 typedef int (*virStorageBackendListVolRegexFunc)(virConnectPtr conn,
                                                  virStoragePoolObjPtr pool,
Index: src/storage_backend_disk.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_disk.c,v
retrieving revision 1.16
diff -u -r1.16 storage_backend_disk.c
--- src/storage_backend_disk.c	23 Oct 2008 11:32:22 -0000	1.16
+++ src/storage_backend_disk.c	31 Oct 2008 11:56:34 -0000
@@ -109,8 +109,7 @@
                                                             devpath)) == NULL)
             return -1;
 
-        if (devpath != vol->target.path)
-            VIR_FREE(devpath);
+        VIR_FREE(devpath);
     }
 
     if (vol->key == NULL) {
@@ -447,7 +446,8 @@
     .deleteVol = virStorageBackendDiskDeleteVol,
 
     .poolOptions = {
-        .flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE),
+        .flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE|
+                  VIR_STORAGE_BACKEND_POOL_STABLE_PATH),
         .defaultFormat = VIR_STORAGE_POOL_DISK_UNKNOWN,
         .formatFromString = virStorageBackendPartTableTypeFromString,
         .formatToString = virStorageBackendPartTableTypeToString,
Index: src/storage_backend_iscsi.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_iscsi.c,v
retrieving revision 1.15
diff -u -r1.15 storage_backend_iscsi.c
--- src/storage_backend_iscsi.c	16 Oct 2008 15:06:04 -0000	1.15
+++ src/storage_backend_iscsi.c	31 Oct 2008 11:56:34 -0000
@@ -219,8 +219,7 @@
                                                         devpath)) == NULL)
         goto cleanup;
 
-    if (devpath != vol->target.path)
-        VIR_FREE(devpath);
+    VIR_FREE(devpath);
 
     if (virStorageBackendUpdateVolInfoFD(conn, vol, fd, 1) < 0)
         goto cleanup;
@@ -645,7 +644,8 @@
 
     .poolOptions = {
         .flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_HOST |
-                  VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE)
+                  VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE |
+                  VIR_STORAGE_BACKEND_POOL_STABLE_PATH)
     },
 
     .volType = VIR_STORAGE_VOL_BLOCK,
Index: src/storage_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_driver.c,v
retrieving revision 1.13
diff -u -r1.13 storage_driver.c
--- src/storage_driver.c	21 Oct 2008 17:15:53 -0000	1.13
+++ src/storage_driver.c	31 Oct 2008 11:56:34 -0000
@@ -966,8 +966,34 @@
 
     for (i = 0 ; i < driver->pools.count ; i++) {
         if (virStoragePoolObjIsActive(driver->pools.objs[i])) {
-            virStorageVolDefPtr vol =
-                virStorageVolDefFindByPath(driver->pools.objs[i], path);
+            virStorageVolDefPtr vol;
+            virStorageBackendPoolOptionsPtr options;
+
+            options = virStorageBackendPoolOptionsForType(driver->pools.objs[i]->def->type);
+            if (options == NULL)
+                continue;
+
+            if (options->flags & VIR_STORAGE_BACKEND_POOL_STABLE_PATH) {
+                const char *stable_path;
+
+                stable_path = virStorageBackendStablePath(conn,
+                                                          driver->pools.objs[i],
+                                                          path);
+                /*
+                 * virStorageBackendStablePath already does
+                 * virStorageReportError if it fails; we just need to keep
+                 * propagating the return code
+                 */
+                if (stable_path == NULL)
+                    return NULL;
+
+                vol = virStorageVolDefFindByPath(driver->pools.objs[i],
+                                                 stable_path);
+                VIR_FREE(stable_path);
+            }
+            else
+                vol = virStorageVolDefFindByPath(driver->pools.objs[i], path);
+
 
             if (vol)
                 return virGetStorageVol(conn,

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