On 11/12/13 05:19, Eric Blake wrote:
> Putting together pieces from previous patches, it is now possible
> for 'virsh dumpxml --pool gluster volname' to report metadata
Did you mean 'virsh vol-dumpxml --pool ... '?
> about a qcow2 file stored on gluster. The backing file is still
> treated as raw; to fix that, more patches are needed to make the
> storage backing chain analysis recursive rather than halting at
> a network protocol name, but that work will not need any further
> calls into libgfapi so much as just reusing this code, and that
> should be the only code outside of the storage driver that needs
> any help from libgfapi. Any additional use of libgfapi within
> libvirt should only be needed for implementing storage pool APIs
> such as volume creation or resizing, where backing chain analysis
> should be unaffected.
>
> * src/storage/storage_backend_gluster.c
> (virStorageBackendGlusterOpen): Make relative name handling easier.
> (virStorageBackendGlusterReadHeader): New helper function.
> (virStorageBackendGlusterRefreshVol): Probe non-raw files.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/storage/storage_backend_gluster.c | 87 +++++++++++++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
> diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
> index bc90de9..69e8e61 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
...
> @@ -124,6 +130,37 @@ error:
> }
>
>
> +static int
This function is declared as an int, but ...
> +virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
> + const char *name,
> + int maxlen,
> + char **buf)
> +{
> + char *s;
> + size_t nread = 0;
.. returns size_t. This could overflow normally, but is guarded by
maxlen. Okay.
> +
> + if (VIR_ALLOC_N(*buf, maxlen) < 0)
> + return -1;
> +
> + s = *buf;
> + while (maxlen) {
> + ssize_t r = glfs_read(fd, s, maxlen, 0);
> + if (r < 0 && errno == EINTR)
> + continue;
> + if (r < 0) {
> + VIR_FREE(*buf);
> + virReportSystemError(errno, _("unable to read '%s'"), name);
> + return r;
> + }
> + if (r == 0)
> + return nread;
> + buf += r;
> + maxlen -= r;
> + nread += r;
> + }
> + return nread;
> +}
> +
> /* Populate *volptr for the given name and stat information, or leave
> * it NULL if the entry should be skipped (such as "."). Return 0 on
> * success, -1 on failure. */
...
> @@ -162,11 +203,57 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
> }
> state->uri->path = tmp;
>
> + if (!(fd = glfs_open(state->vol, name, O_RDONLY| O_NONBLOCK | O_NOCTTY))) {
> + if ((errno == ENOENT || errno == ELOOP) &&
> + S_ISLNK(st->st_mode)) {
> + VIR_WARN("ignoring dangling symlink '%s'", name);
> + ret = 0;
> + } else {
> + virReportSystemError(errno, _("cannot open volume '%s'"), name);
> + }
> + goto cleanup;
> + }
Now that you actually open the volume file and probe it, you need to
kill the comment added in previous patch:
+ if (VIR_ALLOC(vol) < 0 ||
+ VIR_STRDUP(vol->name, name) < 0 ||
+ virAsprintf(&vol->key, "%s%s/%s",
+ *pool == '/' ? "" : "/", pool, vol->name) < 0)
+ goto cleanup;
+
+ /* FIXME - must open files to determine if they are non-raw */
+ vol->type = VIR_STORAGE_VOL_NETWORK;
+ vol->target.format = VIR_STORAGE_FILE_RAW;
+ vol->capacity = vol->allocation = st->st_size;
+ tmp = state->uri->path;
+ state->uri->path = vol->key;
+ if (!(vol->target.path = virURIFormat(state->uri))) {
+ state->uri->path = tmp;
+ goto cleanup;
> +
> + if ((len = virStorageBackendGlusterReadHeader(fd, name, len, &header)) < 0)
> + goto cleanup;
> +
> + if ((vol->target.format = virStorageFileProbeFormatFromBuf(name,
> + header,
> + len)) < 0 ||
> + !(meta = virStorageFileGetMetadataFromBuf(name, header, len,
> + vol->target.format)))
> + goto cleanup;
Also as you are erroring out in case of failure, you can remove the
lines setting some of the defaults.
> +
> + if (meta->backingStore) {
> + vol->backingStore.path = meta->backingStore;
> + meta->backingStore = NULL;
> + vol->backingStore.format = meta->backingStoreFormat;
> + if (vol->backingStore.format < 0)
> + vol->backingStore.format = VIR_STORAGE_FILE_RAW;
> + }
> + if (meta->capacity)
> + vol->capacity = meta->capacity;
> + if (meta->encrypted) {
> + if (VIR_ALLOC(vol->target.encryption) < 0)
> + goto cleanup;
> + if (vol->target.format == VIR_STORAGE_FILE_QCOW ||
> + vol->target.format == VIR_STORAGE_FILE_QCOW2)
> + vol->target.encryption->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW;
> + }
> + vol->target.features = meta->features;
> + meta->features = NULL;
> + vol->target.compat = meta->compat;
> + meta->compat = NULL;
> +
> *volptr = vol;
> vol = NULL;
> ret = 0;
> cleanup:
> + virStorageFileFreeMetadata(meta);
> virStorageVolDefFree(vol);
> + if (fd)
> + glfs_close(fd);
> + VIR_FREE(header);
> return ret;
> }
>
ACK with the comment nit fixed.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list