On Fri, Jan 27, 2012 at 07:29:56AM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx>
>
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list