[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:
>> 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);
> 
> Need to call virStorageReportError here on OOM.
> 
>>  
>>      /* 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);
> 
> And here.
> 
> Since virStorageBackendStablePath() api contract says that it is responsible
> for setting the errors upon failure.

Oops, of course.  I've fixed this up and committed the result; the final patch
is attached.

Thanks for the review,

-- 
Chris Lalancette
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	3 Nov 2008 11:32:15 -0000
@@ -357,16 +357,17 @@
 char *
 virStorageBackendStablePath(virConnectPtr conn,
                             virStoragePoolObjPtr pool,
-                            char *devpath)
+                            const char *devpath)
 {
     DIR *dh;
     struct dirent *dent;
+    char *stablepath;
 
     /* Short circuit if pool has no target, or if its /dev */
     if (pool->def->target.path == NULL ||
         STREQ(pool->def->target.path, "/dev") ||
         STREQ(pool->def->target.path, "/dev/"))
-        return devpath;
+        goto ret_strdup;
 
     /* The pool is pointing somewhere like /dev/disk/by-path
      * or /dev/disk/by-id, so we need to check all symlinks in
@@ -382,7 +383,6 @@
     }
 
     while ((dent = readdir(dh)) != NULL) {
-        char *stablepath;
         if (dent->d_name[0] == '.')
             continue;
 
@@ -407,10 +407,17 @@
 
     closedir(dh);
 
+ ret_strdup:
     /* Couldn't find any matching stable link so give back
      * the original non-stable dev path
      */
-    return devpath;
+
+    stablepath = strdup(devpath);
+
+    if (stablepath == NULL)
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("dup path"));
+
+    return stablepath;
 }
 
 
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	3 Nov 2008 11:32:15 -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	3 Nov 2008 11:32:15 -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	3 Nov 2008 11:32:16 -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	3 Nov 2008 11:32:16 -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]