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