Re: [libvirt] [PATCH]: Allow arbitrary paths to virStorageVolLookupByPath
Chris Lalancette <clalance <at> redhat.com>
2008-11-03 11:38:49 GMT
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
>> <at> <at> -357,7 +357,7 <at> <at>
>> char *
>> virStorageBackendStablePath(virConnectPtr conn,
>> virStoragePoolObjPtr pool,
>> - char *devpath)
>> + const char *devpath)
>> {
>> DIR *dh;
>> struct dirent *dent;
>> <at> <at> -366,7 +366,7 <at> <at>
>> 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
>> <at> <at> -410,7 +410,7 <at> <at>
>> /* 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
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
>> <at> <at> -357,7 +357,7 <at> <at>
>> char *
>> virStorageBackendStablePath(virConnectPtr conn,
>> virStoragePoolObjPtr pool,
>> - char *devpath)
>> + const char *devpath)
>> {
>> DIR *dh;
>> struct dirent *dent;
>> <at> <at> -366,7 +366,7 <at> <at>
>> 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
>> <at> <at> -410,7 +410,7 <at> <at>
>> /* 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