[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Add virStorageVolResize()



On Fri, Jan 27, 2012 at 07:29:56AM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
> 
> Add a new function to allow changing of capacity of storage volumes.

> diff --git a/src/libvirt.c b/src/libvirt.c
> index e9d638b..44865e8 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -12927,6 +12927,56 @@ error:
>      return NULL;
>  }
>  
> +/**
> + * virStorageVolResize:
> + * @vol: pointer to storage volume
> + * @capacity: new capacity
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Changes the capacity of the storage volume @vol to @capacity. The new
> + * capacity must not exceed the sum of current capacity of the volume and
> + * remainining free space of its parent pool. Also the new capacity must
> + * be greater than or equal to current allocation of the volume.
> + *
> + * Returns 0 on success, or -1 on error.
> + */
> +int
> +virStorageVolResize(virStorageVolPtr vol,
> +                    unsigned long long capacity,
> +                    unsigned int flags)
> +{
> +    virConnectPtr conn;
> +    VIR_DEBUG("vol=%p", vol);

Include all of the parameters in the debug line  "capacity=%llu flags=%x"

> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_STORAGE_VOL(vol)) {
> +        virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    conn = vol->conn;
> +
> +    if (conn->flags & VIR_CONNECT_RO) {
> +       virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> +       goto error;
> +    }
> +
> +    if (conn->storageDriver && conn->storageDriver->volResize) {
> +        int ret;
> +        ret = conn->storageDriver->volResize(vol, capacity, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    virDispatchError(vol->conn);
> +    return -1;
> +}

> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 1340b0c..67c113e 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -520,6 +520,7 @@ LIBVIRT_0.9.10 {
>      global:
>          virDomainShutdownFlags;
>          virStorageVolWipePattern;
> +	virStorageVolResize;
>  } LIBVIRT_0.9.9;

There's a whitespace buglet here - running 'make syntax-check' will
detect this sort of thing for you




>  virStorageBackendPtr virStorageBackendForType(int type);
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index d8dc29c..20f5534 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -1187,6 +1187,21 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn,
>      return 0;
>  }
>  
> +/**
> + * Resize a volume
> + */
> +static int
> +virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                                     virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> +                                     virStorageVolDefPtr vol,
> +                                     unsigned long long capacity,
> +                                     unsigned int flags ATTRIBUTE_UNUSED)
> +{
> +    return virStorageFileResize(vol->target.path,
> +                                vol->target.format,
> +                                capacity);
> +}

The virStorageFileResize method only works for raw files. So before
calling this, it is neccessary to check vol->target.format to ensure
it  == VIR_STORAGE_FILE_RAW. Raise an error for types == FILE_DIR
and for others we should be able to invoke 'qemu-img resize'.

> diff --git a/src/util/storage_file.c b/src/util/storage_file.c
> index ba9cfc5..f84feab 100644
> --- a/src/util/storage_file.c
> +++ b/src/util/storage_file.c
> @@ -931,6 +931,111 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta)
>      VIR_FREE(meta);
>  }
>  
> +static int
> +virStorageFileResizeForFD(const char *path,
> +                          int fd,
> +                          int format,
> +                          unsigned long long capacity)
> +{
> +    unsigned char *head = NULL;
> +    ssize_t len = STORAGE_MAX_HEAD;
> +    int ret = -1;
> +    struct stat sb;
> +
> +    if (fstat(fd, &sb) < 0) {
> +        virReportSystemError(errno,
> +                             _("cannot stat file '%s'"),
> +                             path);
> +        return -1;
> +    }
> +
> +    /* No header to probe for directories */
> +    if (S_ISDIR(sb.st_mode)) {
> +        return 0;
> +    }
> +
> +    if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
> +        virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
> +        return -1;
> +    }
> +
> +    if (VIR_ALLOC_N(head, len) < 0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    if ((len = read(fd, head, len)) < 0) {
> +        virReportSystemError(errno, _("cannot read header '%s'"), path);
> +        goto cleanup;
> +    }
> +
> +    if (format == VIR_STORAGE_FILE_AUTO)
> +        format = virStorageFileProbeFormatFromBuf(path, head, len);
> +
> +    if (format < 0 ||
> +        format >= VIR_STORAGE_FILE_LAST) {
> +        virReportSystemError(EINVAL, _("unknown storage file format %d"),
> +                             format);
> +        goto cleanup;
> +    }
> +
> +    if (fileTypeInfo[format].sizeOffset != -1) {
> +        unsigned long long *file_capacity;
> +
> +        if ((fileTypeInfo[format].sizeOffset + 8) > len)
> +            return -1;
> +
> +        file_capacity = (unsigned long long *)(head + fileTypeInfo[format].sizeOffset);
> +        if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN)
> +            *file_capacity = htole64(capacity);
> +        else
> +            *file_capacity = htobe64(capacity);
> +
> +        *file_capacity /= fileTypeInfo[format].sizeMultiplier;
> +    }
> +
> +    /* Move to start of file to write the header back with adjusted capacity */
> +    if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
> +        virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
> +        return -1;
> +    }

Hmm, actually I see now that my earlier comment was wrong - you are coping
with non-raw files here.  That said, although you were able to implement
this, I'm afraid I don't think we should be doing this. We can't assume
that just twiddling the header field with the size will suffice, because
some formats may need to actually allocate/enlarge extra metadata tables
elsewhere in their file format.  So we really need to call out to qemu-img

> +
> +    if ((len = write(fd, head, len)) < 0) {
> +        virReportSystemError(errno, _("cannot write header '%s'"), path);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(head);
> +    return ret;
> +}
> +
> +/**
> + * virStorageFileResize:
> + *
> + * Change the capacity of the storage file at 'path'.
> + */
> +int
> +virStorageFileResize(const char *path,
> +                     int format,
> +                     unsigned long long capacity)
> +{
> +    int fd, ret;
> +
> +    if ((fd = open(path, O_RDWR)) < 0) {
> +        virReportSystemError(errno, _("cannot open file '%s'"), path);
> +        return -1;
> +    }
> +
> +    ret = virStorageFileResizeForFD(path, fd, format, capacity);
> +
> +    VIR_FORCE_CLOSE(fd);
> +
> +    return ret;
> +}

In light of my comments above, I think we should simply ftruncate(fd, newsize)
here, and handle non-raw files with a separate function

Hmm, actually I wonder if we need to care about  sparse vs non-sparse file
resize (ie writing out zeros for the extra space instead of just truncate).
We could achieve this via a flag, or we could add the 'allocation' paramete
to the new API too.  I probably tend towards a flag

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]