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

[Eric Blake]

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).

> 
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 5fbb6e7..c707200 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1012,10 +1012,12 @@ virStorageFileGetMetadataFromBuf(const char *path,
>      virStorageFileMetadataPtr ret = NULL;
>      char *canonPath;
> 
> -    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:

if (virStorageIsFile(path)) {
    if (!(canonPath = canonicalize_file_name(path))) {
        error;
    }
} else {
    if (VIR_STRDUP(canonPath, path) < 0) {
        error;
    }
}

In other words, the bug is that we are trying to canonicalize a non-file
name (which has a chance for false positives if the file system contains
an odd file name), rather than reserving the canonicalization to happen
ONLY when we know it is not a network protocol name.

-- 
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/0af22f59/attachment-0001.sig>