[libvirt] [PATCH 03/18] util: storage: Don't force path canonicalization when loading metadata

[Eric Blake]

On 04/22/2014 07:34 AM, Eric Blake wrote:
> On 04/20/2014 04:13 PM, Peter Krempa wrote:
>> Avoid breaking gluster volumes that don't have local representation. Use
>> the provided name when canonicalization fails. Broken by commit 79f11b35
>>
>> Fixes:
>>  $ virsh pool-start glusterpool
>>  error: Failed to start pool glusterpool
>>  error: unable to resolve 'asdf': No such file or directory
>> ---
>>  src/util/virstoragefile.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> I'd feel a bit better if we had testsuite coverage for this (since my
> broken commit still managed to pass 'make check', it shows our testsuite
> has a hole).
> 

>> -    if (!(canonPath = canonicalize_file_name(path))) {
>> +    if (!(canonPath = canonicalize_file_name(path)) &&
>> +        VIR_STRDUP(canonPath, path) < 0) {
>>          virReportSystemError(errno, _("unable to resolve '%s'"), path);
>>          return NULL;
>>      }
> 
> I'm not sure if I like this.  We are blindly trying to resolve 'path' as
> if it were a local file name, and then if it failed, use 'path' as-is in
> case it was a remote name.  I think what we should really be doing is:

I'd still like the test improvements, but those can be separate patches
as long as they are in by the time gluster interaction is fully
integrated.  So for this patch, I can live with ACK if you squash this in:

diff --git i/src/util/virstoragefile.c w/src/util/virstoragefile.c
index c707200..6ea45a4 100644
--- i/src/util/virstoragefile.c
+++ w/src/util/virstoragefile.c
@@ -1012,9 +1012,12 @@ virStorageFileGetMetadataFromBuf(const char *path,
     virStorageFileMetadataPtr ret = NULL;
     char *canonPath;

-    if (!(canonPath = canonicalize_file_name(path)) &&
-        VIR_STRDUP(canonPath, path) < 0) {
-        virReportSystemError(errno, _("unable to resolve '%s'"), path);
+    if (virStorageIsFile(path)) {
+        if (!(canonPath = canonicalize_file_name(path))) {
+            virReportSystemError(errno, _("unable to resolve '%s'"), path);
+            return NULL;
+        }
+    } else if (VIR_STRDUP(canonPath, path) < 0) {
         return NULL;
     }

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140422/b395ed7d/attachment-0001.sig>