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,
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list