[libvirt] [PATCH 0/3] Fix startup of gluster pools
Currently a gluster pool fails to start due to an attempt to canonicalize a path residing on gluster storage. This series rearranges things to avoid that. Peter Krempa (3): storage: Return backing format from virStorageFileGetMetadataFromFD storage: fs: Drop-in replace use of virStorageFileGetMetadataFromBuf utils: storage: Canonicalize paths only for local filesystems src/qemu/qemu_driver.c | 2 +- src/storage/storage_backend_fs.c | 20 +++++++------------- src/util/virstoragefile.c | 27 +++++++++++++++------------ src/util/virstoragefile.h | 3 ++- 4 files changed, 25 insertions(+), 27 deletions(-) -- 1.9.3
Add argument to return backing file format of a file probed by virStorageFileGetMetadataFromFD so that it can be used in place of virStorageFileGetMetadataFromBuf. --- src/qemu/qemu_driver.c | 2 +- src/util/virstoragefile.c | 6 ++++-- src/util/virstoragefile.h | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2b852eb..2680399 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10364,7 +10364,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, } } - if (!(meta = virStorageFileGetMetadataFromFD(path, fd, format))) + if (!(meta = virStorageFileGetMetadataFromFD(path, fd, format, NULL))) goto cleanup; /* Get info for normal formats */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index b90cdc9..4329395 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1091,14 +1091,16 @@ virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta, virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, int fd, - int format) + int format, + int *backingFormat) + { virStorageSourcePtr ret = NULL; if (!(ret = virStorageFileMetadataNew(path, format))) goto cleanup; - if (virStorageFileGetMetadataFromFDInternal(ret, fd, NULL) < 0) { + if (virStorageFileGetMetadataFromFDInternal(ret, fd, backingFormat) < 0) { virStorageSourceFree(ret); ret = NULL; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 082ff5b..158806b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -271,7 +271,8 @@ int virStorageFileGetMetadata(virStorageSourcePtr src, ATTRIBUTE_NONNULL(1); virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, int fd, - int format); + int format, + int *backingFormat); virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, char *buf, size_t len, -- 1.9.3
On 05/28/2014 08:15 AM, Peter Krempa wrote:
Add argument to return backing file format of a file probed by virStorageFileGetMetadataFromFD so that it can be used in place of virStorageFileGetMetadataFromBuf. --- src/qemu/qemu_driver.c | 2 +- src/util/virstoragefile.c | 6 ++++-- src/util/virstoragefile.h | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Use virStorageFileGetMetadataFromFD instead in virStorageBackendProbeTarget as it now returns all required data and the storage file is already open in a filedescriptor. Also fix improper error code being returned when virFileReadHeaderFD would fail as virStorageBackendUpdateVolTargetInfoFD would set the return code to 0. --- src/storage/storage_backend_fs.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 33551e7..003c6df 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -71,8 +71,6 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, int ret = -1; virStorageSourcePtr meta = NULL; struct stat sb; - char *header = NULL; - ssize_t len = VIR_STORAGE_MAX_HEADER; *backingStore = NULL; *backingStoreFormat = VIR_STORAGE_FILE_AUTO; @@ -89,22 +87,19 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto error; } + ret = -1; + if (S_ISDIR(sb.st_mode)) { target->format = VIR_STORAGE_FILE_DIR; } else { - if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) { - virReportSystemError(errno, _("cannot read header '%s'"), - target->path); + if (!(meta = virStorageFileGetMetadataFromFD(target->path, + fd, + VIR_STORAGE_FILE_AUTO, + backingStoreFormat))) goto error; - } - if (!(meta = virStorageFileGetMetadataFromBuf(target->path, - header, len, - backingStore, - backingStoreFormat))) { - ret = -1; + if (VIR_STRDUP(*backingStore, meta->backingStoreRaw) < 0) goto error; - } } VIR_FORCE_CLOSE(fd); @@ -170,7 +165,6 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, cleanup: virStorageSourceFree(meta); - VIR_FREE(header); return ret; } -- 1.9.3
On 05/28/2014 08:16 AM, Peter Krempa wrote:
Use virStorageFileGetMetadataFromFD instead in virStorageBackendProbeTarget as it now returns all required data and the storage file is already open in a filedescriptor.
Also fix improper error code being returned when virFileReadHeaderFD would fail as virStorageBackendUpdateVolTargetInfoFD would set the return code to 0. --- src/storage/storage_backend_fs.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Now that virStorageFileGetMetadataFromBuf is used only for remote filesystems, don't canonicalize the path in it. --- src/util/virstoragefile.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 4329395..2feda67 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -952,15 +952,8 @@ virStorageFileMetadataNew(const char *path, if (VIR_STRDUP(ret->relPath, path) < 0) goto error; - if (virStorageIsFile(path)) { - if (!(ret->path = canonicalize_file_name(path))) { - virReportSystemError(errno, _("unable to resolve '%s'"), path); - goto error; - } - } else { - if (VIR_STRDUP(ret->path, path) < 0) - goto error; - } + if (VIR_STRDUP(ret->path, path) < 0) + goto error; return ret; @@ -1096,16 +1089,24 @@ virStorageFileGetMetadataFromFD(const char *path, { virStorageSourcePtr ret = NULL; + char *canonPath = NULL; + + if (!(canonPath = canonicalize_file_name(path))) { + virReportSystemError(errno, _("unable to resolve '%s'"), path); + goto cleanup; + } - if (!(ret = virStorageFileMetadataNew(path, format))) + if (!(ret = virStorageFileMetadataNew(canonPath, format))) goto cleanup; + if (virStorageFileGetMetadataFromFDInternal(ret, fd, backingFormat) < 0) { virStorageSourceFree(ret); ret = NULL; } cleanup: + VIR_FREE(canonPath); return ret; } -- 1.9.3
On 05/28/2014 08:16 AM, Peter Krempa wrote:
Now that virStorageFileGetMetadataFromBuf is used only for remote filesystems, don't canonicalize the path in it. --- src/util/virstoragefile.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
ACK; counts as a bug fix, so safe to push series for 1.2.5. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/28/14 18:48, Eric Blake wrote:
On 05/28/2014 08:16 AM, Peter Krempa wrote:
Now that virStorageFileGetMetadataFromBuf is used only for remote filesystems, don't canonicalize the path in it. --- src/util/virstoragefile.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
ACK; counts as a bug fix, so safe to push series for 1.2.5.
Series pushed; Thanks Peter
participants (2)
-
Eric Blake -
Peter Krempa