[libvirt] [libvirt-glib] Add virStorageVolResize()
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> Add a new function to allow changing of capacity of storage volumes. --- include/libvirt/libvirt.h.in | 5 ++ src/driver.h | 5 ++ src/libvirt.c | 50 ++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +++- src/remote_protocol-structs | 6 ++ src/storage/storage_backend.h | 6 ++ src/storage/storage_backend_fs.c | 18 +++++++ src/storage/storage_driver.c | 83 ++++++++++++++++++++++++++++++ src/util/storage_file.c | 105 ++++++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 4 ++ tools/virsh.c | 53 +++++++++++++++++++ 13 files changed, 345 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..b169592 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2386,6 +2386,11 @@ char * virStorageVolGetXMLDesc (virStorageVolPtr pool, char * virStorageVolGetPath (virStorageVolPtr vol); +int virStorageVolResize (virStorageVolPtr vol, + unsigned long long capacity, + unsigned int flags); + + /** * virKeycodeSet: * diff --git a/src/driver.h b/src/driver.h index df2aa60..c850926 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1261,6 +1261,10 @@ typedef int unsigned long long offset, unsigned long long length, unsigned int flags); +typedef int + (*virDrvStorageVolResize) (virStorageVolPtr vol, + unsigned long long capacity, + unsigned int flags); typedef int (*virDrvStoragePoolIsActive)(virStoragePoolPtr pool); @@ -1323,6 +1327,7 @@ struct _virStorageDriver { virDrvStorageVolGetInfo volGetInfo; virDrvStorageVolGetXMLDesc volGetXMLDesc; virDrvStorageVolGetPath volGetPath; + virDrvStorageVolResize volResize; virDrvStoragePoolIsActive poolIsActive; virDrvStoragePoolIsPersistent poolIsPersistent; }; 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); + + 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; +} /** * virNodeNumOfDevices: 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; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f79f53e..2bb4cbf 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4837,6 +4837,7 @@ static virStorageDriver storage_driver = { .volGetInfo = remoteStorageVolGetInfo, /* 0.4.1 */ .volGetXMLDesc = remoteStorageVolGetXMLDesc, /* 0.4.1 */ .volGetPath = remoteStorageVolGetPath, /* 0.4.1 */ + .volResize = remoteStorageVolResize, /* 0.9.10 */ .poolIsActive = remoteStoragePoolIsActive, /* 0.7.3 */ .poolIsPersistent = remoteStoragePoolIsPersistent, /* 0.7.3 */ }; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 0f354bb..29f98fc 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1676,6 +1676,12 @@ struct remote_storage_vol_get_path_ret { remote_nonnull_string name; }; +struct remote_storage_vol_resize_args { + remote_nonnull_storage_vol vol; + unsigned hyper capacity; + unsigned int flags; +}; + /* Node driver calls: */ struct remote_node_num_of_devices_args { @@ -2667,7 +2673,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258, /* autogen autogen */ - REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259 /* autogen autogen */ + REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259, /* autogen autogen */ + REMOTE_PROC_STORAGE_VOL_RESIZE = 300 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index de85862..9a60fc2 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1260,6 +1260,11 @@ struct remote_storage_vol_get_path_args { struct remote_storage_vol_get_path_ret { remote_nonnull_string name; }; +struct remote_storage_vol_resize_args { + remote_nonnull_storage_vol vol; + uint64_t capacity; + u_int flags; +}; struct remote_node_num_of_devices_args { remote_string cap; u_int flags; @@ -2101,4 +2106,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258, REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259, + REMOTE_PROC_STORAGE_VOL_RESIZE = 300, }; diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 75ed676..a37bf7c 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -44,6 +44,11 @@ typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjP typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags); +typedef int (*virStorageBackendVolumeResize)(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned long long capacity, + unsigned int flags); /* File creation/cloning functions used for cloning between backends */ int virStorageBackendCreateRaw(virConnectPtr conn, @@ -78,6 +83,7 @@ struct _virStorageBackend { virStorageBackendCreateVol createVol; virStorageBackendRefreshVol refreshVol; virStorageBackendDeleteVol deleteVol; + virStorageBackendVolumeResize resizeVol; }; 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); +} + virStorageBackend virStorageBackendDirectory = { .type = VIR_STORAGE_POOL_DIR, @@ -1199,6 +1214,7 @@ virStorageBackend virStorageBackendDirectory = { .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, + .resizeVol = virStorageBackendFileSystemVolResize, }; #if WITH_STORAGE_FS @@ -1216,6 +1232,7 @@ virStorageBackend virStorageBackendFileSystem = { .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, + .resizeVol = virStorageBackendFileSystemVolResize, }; virStorageBackend virStorageBackendNetFileSystem = { .type = VIR_STORAGE_POOL_NETFS, @@ -1232,5 +1249,6 @@ virStorageBackend virStorageBackendNetFileSystem = { .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, + .resizeVol = virStorageBackendFileSystemVolResize, }; #endif /* WITH_STORAGE_FS */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index a332ada..d3eff93 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1695,7 +1695,89 @@ out: return ret; } +static int +storageVolumeResize(virStorageVolPtr obj, + unsigned long long capacity, + unsigned int flags) +{ + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStorageBackendPtr backend; + virStoragePoolObjPtr pool = NULL; + virStorageVolDefPtr vol = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); + storageDriverUnlock(driver); + if (!pool) { + virStorageReportError(VIR_ERR_NO_STORAGE_POOL, + "%s", _("no storage pool with matching uuid")); + goto out; + } + + if (!virStoragePoolObjIsActive(pool)) { + virStorageReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("storage pool is not active")); + goto out; + } + + if ((backend = virStorageBackendForType(pool->def->type)) == NULL) + goto out; + + vol = virStorageVolDefFindByName(pool, obj->name); + + if (vol == NULL) { + virStorageReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching name '%s'"), + obj->name); + goto out; + } + + if (vol->building) { + virStorageReportError(VIR_ERR_OPERATION_INVALID, + _("volume '%s' is still being allocated."), + vol->name); + goto out; + } + + if (capacity < vol->allocation) { + virStorageReportError(VIR_ERR_INVALID_ARG, + "%s", _("can not shrink capacity below " + "existing allocation")); + + goto out; + } + + if (capacity > vol->allocation + pool->def->available) { + virStorageReportError(VIR_ERR_INVALID_ARG, + "%s", _("Not enough space left on storage pool")); + + goto out; + } + + if (!backend->resizeVol) { + virStorageReportError(VIR_ERR_NO_SUPPORT, + "%s", _("storage pool does not support changing " + "of volume capacity")); + + goto out; + } + + if (backend->resizeVol(obj->conn, pool, vol, capacity, flags) < 0) + goto out; + + vol->capacity = capacity; + ret = 0; + +out: + if (pool) + virStoragePoolObjUnlock(pool); + + return ret; +} /* If the volume we're wiping is already a sparse file, we simply * truncate and extend it to its original size, filling it with @@ -2243,6 +2325,7 @@ static virStorageDriver storageDriver = { .volGetInfo = storageVolumeGetInfo, /* 0.4.0 */ .volGetXMLDesc = storageVolumeGetXMLDesc, /* 0.4.0 */ .volGetPath = storageVolumeGetPath, /* 0.4.0 */ + .volResize = storageVolumeResize, /* 0.9.10 */ .poolIsActive = storagePoolIsActive, /* 0.7.3 */ .poolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ 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; + } + + 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; +} + #ifdef __linux__ # ifndef NFS_SUPER_MAGIC diff --git a/src/util/storage_file.h b/src/util/storage_file.h index b8920d0..f6c6048 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -72,6 +72,10 @@ int virStorageFileGetMetadataFromFD(const char *path, void virStorageFileFreeMetadata(virStorageFileMetadata *meta); +int virStorageFileResize(const char *path, + int format, + unsigned long long capacity); + enum { VIR_STORAGE_FILE_SHFS_NFS = (1 << 0), VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1), diff --git a/tools/virsh.c b/tools/virsh.c index 74655c2..20d4bd0 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11279,6 +11279,58 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "vol-resize" command + */ +static const vshCmdInfo info_vol_resize[] = { + {"help", N_("resize a vol")}, + {"desc", N_("Resizes a storage vol.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_resize[] = { + {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, + {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new capacity for the vol with optional k,M,G,T suffix")}, + {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdVolResize(vshControl *ctl, const vshCmd *cmd) +{ + virStorageVolPtr vol; + const char *capacityStr = NULL; + unsigned long long capacity = 0; + bool ret = true; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) + return false; + + if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) + goto cleanup; + if (cmdVolSize(capacityStr, &capacity) < 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup; + } + + if (virStorageVolResize(vol, capacity, 0) == 0) { + vshPrint(ctl, "Size of volume '%s' successfully changed to %s\n", + virStorageVolGetName(vol), capacityStr); + ret = true; + } else { + vshError(ctl, "Failed to change size of volume '%s' to %s\n", + virStorageVolGetName(vol), capacityStr); + ret = false; + } + +cleanup: + virStorageVolFree(vol); + return ret; +} + /* * "vol-dumpxml" command @@ -16141,6 +16193,7 @@ static const vshCmdDef storageVolCmds[] = { {"vol-pool", cmdVolPool, opts_vol_pool, info_vol_pool, 0}, {"vol-upload", cmdVolUpload, opts_vol_upload, info_vol_upload, 0}, {"vol-wipe", cmdVolWipe, opts_vol_wipe, info_vol_wipe, 0}, + {"vol-resize", cmdVolResize, opts_vol_resize, info_vol_resize, 0}, {NULL, NULL, NULL, NULL, 0} }; -- 1.7.7.5
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 :|
On 01/26/2012 10:29 PM, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Add a new function to allow changing of capacity of storage volumes. --- include/libvirt/libvirt.h.in | 5 ++ src/driver.h | 5 ++ src/libvirt.c | 50 ++++++++++++++++++ src/libvirt_public.syms | 1 +
This part forms the public API,
src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +++- src/remote_protocol-structs | 6 ++
This forwards the public API over the wire,
src/storage/storage_backend.h | 6 ++ src/storage/storage_backend_fs.c | 18 +++++++ src/storage/storage_driver.c | 83 ++++++++++++++++++++++++++++++ src/util/storage_file.c | 105 ++++++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 4 ++
this implements things,
tools/virsh.c | 53 +++++++++++++++++++
and this exposes the implementation to the user (which can also be done at the same time as the public api, if it is simple enough). And it's missing tools/virsh.pod for documentation. I think, in the interest of hitting 0.9.10 freeze deadlines, that we want to commit to the API, but that your implementation still has some issues to be worked out. So, what I'm going to do is split this into three patches, then just review _and commit_ the first two, while leaving the implementation for a v2 patch, once you've fixed things to address Dan's comments (such as, if you're going to resize a qcow2 image, you have to do it with qemu-img and not by raw poking in the header of the file; as well as implementing a sparse vs. pre-allocation flag). The rest of this mail is a review of part 1:
13 files changed, 345 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..b169592 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2386,6 +2386,11 @@ char * virStorageVolGetXMLDesc (virStorageVolPtr pool,
char * virStorageVolGetPath (virStorageVolPtr vol);
+int virStorageVolResize (virStorageVolPtr vol, + unsigned long long capacity, + unsigned int flags);
I think this makes sense. We normally want to resize capacity; which may or may not also increase allocation. We may also want to resize allocation without affecting capacity (basically, turn a thin disk into a full one), but when you throw things like qcow2 in the mix (allocation may be less than capacity when things are sparse, or may be greater than capacity due to the size of the headers managing the disk), I don't think you can ever directly target the allocation parameter. And the presence of flags will let us add flags if we ever change our minds and need to add even more features (such as async operation, where we then send an event when things complete).
+++ b/src/libvirt.c @@ -12927,6 +12927,56 @@ error: return NULL; }
+/** + * virStorageVolResize: + * @vol: pointer to storage volume + * @capacity: new capacity
In what units? virDomainBlockResize used kbytes as the unit, but I think that's not as nice as doing things in raw bytes.
+ * @flags: extra flags; not used yet, so callers should always pass 0
Let's go ahead and define some useful flags now, so that we can give further details on the semantics. You don't have to implement them all right away, but I'd rather have things called out in advance. For now, I'm going to document things as having all expansions be all NUL bytes, although we can always add a flag in the future to allow the resize to use unspecified bytes if that can make an operation faster.
+ * + * 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
s/remainining/remaining/
+ * be greater than or equal to current allocation of the volume.
I think this is too strong - downsizing _might_ be reasonable, even though it is generally a form of data loss and thus must not be default, so let's control it by a flag. Allowing a delta might be nice, and that says we should pass a signed value (shouldn't be an issue - if you can show me anyone with a single storage volume with 2^63 bytes, let alone 2^64, I'd be impressed with their setup!). Hmm, I wonder if we also want to support a percentage growth mode (I want my disk to be made 20% bigger than it currently is, no matter its current size); but that seems difficult to do without a floating point parameter; so at this point, I'm inclined to say that the user must call virStorageVolGetInfo, do the percentage growth themselves, then call the API.
+ * + * 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); + + 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; + }
Even though I'm proposing making capacity signed, we can do some sanity checking on it to avoid negative values without a request to shrink.
+++ b/src/libvirt_public.syms @@ -520,6 +520,7 @@ LIBVIRT_0.9.10 { global: virDomainShutdownFlags; virStorageVolWipePattern; + virStorageVolResize;
In addition to Dan's comment about TAB, I also like to sort these lines.
} LIBVIRT_0.9.9;
# .... define new API here using predicted next version number ....
Skipping to the only other file I'd like in patch 1:
+++ b/tools/virsh.c @@ -11279,6 +11279,58 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) return ret; }
+/* + * "vol-resize" command + */ +static const vshCmdInfo info_vol_resize[] = { + {"help", N_("resize a vol")}, + {"desc", N_("Resizes a storage vol.")},
I'll go ahead and add flag support.
+ {NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_resize[] = { + {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, + {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new capacity for the vol with optional k,M,G,T suffix")},
Long line. Disk manufacturers would have us use multiples of 1000 rather than 1024 in parsing numbers; but at least you are consistent with the rest of virsh.
+ {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdVolResize(vshControl *ctl, const vshCmd *cmd) +{ + virStorageVolPtr vol; + const char *capacityStr = NULL; + unsigned long long capacity = 0; + bool ret = true;
This should be false, so that...
+ + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) + return false; + + if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) + goto cleanup; + if (cmdVolSize(capacityStr, &capacity) < 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup;
this error message gives the right return value.
+ vshError(ctl, "Failed to change size of volume '%s' to %s\n",
Missing translation marks. Here's the delta I'm proposing; I'll repost this as a formal patch when I finish reviewing part 2. diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index b169592..d26cbbd 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -2386,8 +2386,14 @@ char * virStorageVolGetXMLDesc (virStorageVolPtr pool, char * virStorageVolGetPath (virStorageVolPtr vol); +typedef enum { + VIR_STORAGE_VOL_RESIZE_ALLOCATE = 1 << 0, /* force allocation of new size */ + VIR_STORAGE_VOL_RESIZE_DELTA = 1 << 1, /* size is relative to current */ + VIR_STORAGE_VOL_RESIZE_SHRINK = 1 << 2, /* allow decrease in capacity */ +} virStorageVolResizeFlags; + int virStorageVolResize (virStorageVolPtr vol, - unsigned long long capacity, + long long capacity, unsigned int flags); diff --git i/src/driver.h w/src/driver.h index c850926..485b578 100644 --- i/src/driver.h +++ w/src/driver.h @@ -1263,7 +1263,7 @@ typedef int unsigned int flags); typedef int (*virDrvStorageVolResize) (virStorageVolPtr vol, - unsigned long long capacity, + long long capacity, unsigned int flags); typedef int diff --git i/src/libvirt.c w/src/libvirt.c index 44865e8..540d74a 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -12930,23 +12930,39 @@ error: /** * 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. + * @capacity: new capacity, in bytes + * @flags: bitwise-OR of virStorageVolResizeFlags + * + * Changes the capacity of the storage volume @vol to @capacity. The + * operation will fail if the new capacity requires allocation that would + * exceed the remaining free space in the parent pool. The contents of + * the new capacity will appear as all zero bytes. + * + * Normally, the operation will attempt to affect capacity with a minimum + * impact on allocation (that is, the default operation favors a sparse + * resize). If @flags contains VIR_STORAGE_VOL_RESIZE_ALLOCATE, then the + * operation will ensure that allocation is sufficient for the new + * capacity; this may make the operation take noticeably longer. + * + * Normally, the operation treats @capacity as the new size in bytes; + * but if @flags contains VIR_STORAGE_RESIZE_DELTA, then @capacity + * represents the size difference to add to the current size. It is + * up to the storage pool implementation whether unaligned requests are + * rounded up to the next valid boundary, or rejected. + * + * Normally, this operation should only be used to enlarge capacity; + * but if @flags contains VIR_STORAGE_RESIZE_SHRINK, it is possible to + * attempt a reduction in capacity even though it might cause data loss. * * Returns 0 on success, or -1 on error. */ int virStorageVolResize(virStorageVolPtr vol, - unsigned long long capacity, + long long capacity, unsigned int flags) { virConnectPtr conn; - VIR_DEBUG("vol=%p", vol); + VIR_DEBUG("vol=%p capacity=%lld flags=%x", vol, capacity, flags); virResetLastError(); @@ -12963,6 +12979,16 @@ virStorageVolResize(virStorageVolPtr vol, goto error; } + /* Negative capacity is valid only with both delta and shrink; + * zero capacity is valid with either delta or shrink. */ + if ((capacity < 0 && !(flags & VIR_STORAGE_VOL_RESIZE_DELTA) && + !(flags & VIR_STORAGE_VOL_RESIZE_SHRINK)) || + (capacity == 0 && !((flags & VIR_STORAGE_VOL_RESIZE_DELTA) || + (flags & VIR_STORAGE_VOL_RESIZE_SHRINK)))) { + virLibStorageVolError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (conn->storageDriver && conn->storageDriver->volResize) { int ret; ret = conn->storageDriver->volResize(vol, capacity, flags); diff --git i/src/libvirt_public.syms w/src/libvirt_public.syms index 67c113e..8bdb24b 100644 --- i/src/libvirt_public.syms +++ w/src/libvirt_public.syms @@ -519,8 +519,8 @@ LIBVIRT_0.9.9 { LIBVIRT_0.9.10 { global: virDomainShutdownFlags; + virStorageVolResize; virStorageVolWipePattern; - virStorageVolResize; } LIBVIRT_0.9.9; # .... define new API here using predicted next version number .... diff --git i/tools/virsh.c w/tools/virsh.c index 7f3a55c..634c0f8 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -11284,14 +11284,20 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_vol_resize[] = { {"help", N_("resize a vol")}, - {"desc", N_("Resizes a storage vol.")}, + {"desc", N_("Resizes a storage volume.")}, {NULL, NULL} }; static const vshCmdOptDef opts_vol_resize[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, - {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new capacity for the vol with optional k,M,G,T suffix")}, + {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("new capacity for the vol with optional k,M,G,T suffix")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {"allocate", VSH_OT_BOOL, 0, + N_("allocate the new capacity, rather than leaving it sparse")}, + {"delta", VSH_OT_BOOL, 0, + N_("use capacity as a delta to current size, rather than the new size")}, + {"shrink", VSH_OT_BOOL, 0, N_("allow the resize to shrink the volume")}, {NULL, 0, 0, NULL} }; @@ -11301,7 +11307,18 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) virStorageVolPtr vol; const char *capacityStr = NULL; unsigned long long capacity = 0; - bool ret = true; + unsigned int flags = 0; + bool ret = false; + bool delta = false; + + if (vshCommandOptBool(cmd, "allocate")) + flags |= VIR_STORAGE_VOL_RESIZE_ALLOCATE; + if (vshCommandOptBool(cmd, "delta")) { + delta = true; + flags |= VIR_STORAGE_VOL_RESIZE_DELTA; + } + if (vshCommandOptBool(cmd, "shrink")) + flags |= VIR_STORAGE_VOL_RESIZE_SHRINK; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -11311,17 +11328,29 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) goto cleanup; - if (cmdVolSize(capacityStr, &capacity) < 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; + if (delta && *capacityStr == '-') { + if (cmdVolSize(capacityStr + 1, &capacity) < 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup; + } + capacity = -capacity; + } else { + if (cmdVolSize(capacityStr, &capacity) < 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup; + } } - if (virStorageVolResize(vol, capacity, 0) == 0) { - vshPrint(ctl, "Size of volume '%s' successfully changed to %s\n", + if (virStorageVolResize(vol, capacity, flags) == 0) { + vshPrint(ctl, + delta ? _("Size of volume '%s' successfully changed by %s\n") + : _("Size of volume '%s' successfully changed to %s\n"), virStorageVolGetName(vol), capacityStr); ret = true; } else { - vshError(ctl, "Failed to change size of volume '%s' to %s\n", + vshError(ctl, + delta ? _("Failed to change size of volume '%s' by %s\n") + : _("Failed to change size of volume '%s' to %s\n"), virStorageVolGetName(vol), capacityStr); ret = false; } diff --git i/tools/virsh.pod w/tools/virsh.pod index 8599f66..6622caf 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -2012,6 +2012,17 @@ I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume is in. I<vol-name-or-path> is the name or path of the volume to return the volume key for. +=item B<vol-resize> [I<--pool> I<pool-or-uuid>] I<vol-name-or-path> +I<pool-or-uuid> I<capacity> [I<--allocate>] [I<--delta>] [I<--shrink>] + +Resize the capacity of the given volume, in bytes. +I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume +is in. I<vol-name-or-key-or-path> is the name or key or path of the volume +to resize. The new capacity might be sparse unless I<--allocate> is +specified. Normally, I<capacity> is the new size, but if I<--delta> +is present, then it is added to the existing size. Attempts to shrink +the volume will fail unless I<--shrink> is present. + =back =head1 SECRET COMMMANDS -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Sat, Jan 28, 2012 at 1:17 AM, Eric Blake <eblake@redhat.com> wrote:
On 01/26/2012 10:29 PM, Zeeshan Ali (Khattak) wrote:
+/** + * virStorageVolResize: + * @vol: pointer to storage volume + * @capacity: new capacity
In what units? virDomainBlockResize used kbytes as the unit,
Shouldn't that be implicit unless specified otherwise in the docs? I am fine with or without explicit docs.
+ * be greater than or equal to current allocation of the volume.
I think this is too strong - downsizing _might_ be reasonable, even though it is generally a form of data loss and thus must not be default, so let's control it by a flag.
That might work for raw files but qemu-img refuse to downsize the image.
Here's the delta I'm proposing; I'll repost this as a formal patch when I finish reviewing part 2.
So I'm OK with everyone other than I addressed above. Most important thing is that you test your changes. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Sat, Jan 28, 2012 at 02:49:46AM +0200, Zeeshan Ali (Khattak) wrote:
On Sat, Jan 28, 2012 at 1:17 AM, Eric Blake <eblake@redhat.com> wrote:
On 01/26/2012 10:29 PM, Zeeshan Ali (Khattak) wrote:
+/** + * virStorageVolResize: + * @vol: pointer to storage volume + * @capacity: new capacity
In what units? virDomainBlockResize used kbytes as the unit,
Shouldn't that be implicit unless specified otherwise in the docs? I am fine with or without explicit docs.
Given that virStorageVolInfo works in bytes, then I'd say we should use bytes here too. It is a shame we used kbytes for virDomainBlockResize. 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 :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Zeeshan Ali (Khattak)