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