[libvirt] [PATCH v3 0/2] storage: allow metadata preallocation when creating qcow2 images
Diff to v1: * A flag for virStorageVolCreateXML and virStorageVolCreateXMLFrom is used instead of guessing from the allocation element. * The flag is exposed and documented in virsh. Diff to v2: * merged first two patches to enable and implement the flag at the same time * more documentation * fixed spaces to pass syntax-check * !! for unsigned -> bool conversion * initialize flags in cmdVolCreateFrom and cmdVolClone Ján Tomko (2): storage: allow metadata preallocation when creating qcow2 images virsh: allow metadata preallocation when creating volumes include/libvirt/libvirt.h.in | 4 +++ src/libvirt.c | 16 ++++++++++-- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++----------- src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 16 ++++++++----- src/storage/storage_driver.c | 6 ++-- tools/virsh-volume.c | 25 +++++++++++++++++--- tools/virsh.pod | 23 ++++++++++++++++-- 8 files changed, 105 insertions(+), 34 deletions(-) -- 1.7.8.6
Add VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA flag to virStorageVolCreateXML and virStorageVolCreateXMLFrom. This flag requests metadata preallocation when creating/cloning qcow2 images, resulting in creating a sparse file with qcow2 metadata. It has only slightly larger disk usage compared to new image with no allocation, but offers higher performance. --- include/libvirt/libvirt.h.in | 4 +++ src/libvirt.c | 16 ++++++++++-- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++----------- src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 16 ++++++++----- src/storage/storage_driver.c | 6 ++-- 6 files changed, 64 insertions(+), 27 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a19f37a..17804ca 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2906,6 +2906,10 @@ virStorageVolPtr virStorageVolLookupByPath (virConnectPtr conn, const char* virStorageVolGetName (virStorageVolPtr vol); const char* virStorageVolGetKey (virStorageVolPtr vol); +typedef enum { + VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0, +} virStorageVolCreateFlags; + virStorageVolPtr virStorageVolCreateXML (virStoragePoolPtr pool, const char *xmldesc, unsigned int flags); diff --git a/src/libvirt.c b/src/libvirt.c index 4b7baab..6a7a817 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -13382,11 +13382,16 @@ virStorageVolGetKey(virStorageVolPtr vol) * virStorageVolCreateXML: * @pool: pointer to storage pool * @xmlDesc: description of volume to create - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virStorageVolCreateFlags * * Create a storage volume within a pool based * on an XML description. Not all pools support - * creation of volumes + * creation of volumes. + * + * Since 1.0.1 VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA + * in flags can be used to get higher performance with + * qcow2 image files which don't support full preallocation, + * by creating a sparse image file with metadata. * * Returns the storage volume, or NULL on error */ @@ -13433,13 +13438,18 @@ error: * @pool: pointer to parent pool for the new volume * @xmlDesc: description of volume to create * @clonevol: storage volume to use as input - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virStorageVolCreateFlags * * Create a storage volume in the parent pool, using the * 'clonevol' volume as input. Information for the new * volume (name, perms) are passed via a typical volume * XML description. * + * Since 1.0.1 VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA + * in flags can be used to get higher performance with + * qcow2 image files which don't support full preallocation, + * by creating a sparse image file with metadata. + * * Returns the storage volume, or NULL on error */ virStorageVolPtr diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 83c4755..083028d 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -668,8 +668,11 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); unsigned long long int size_arg; + bool preallocate = false; - virCheckFlags(0, -1); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); + + preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA); const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ? @@ -697,11 +700,23 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, inputvol->target.format); return -1; } + if (preallocate && vol->target.format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata preallocation only available with qcow2")); + return -1; + } if (vol->backingStore.path) { int accessRetCode = -1; char *absolutePath = NULL; + if (preallocate) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata preallocation conflicts with backing" + " store")); + return -1; + } + /* XXX: Not strictly required: qemu-img has an option a different * backing store, not really sure what use it serves though, and it * may cause issues with lvm. Untested essentially. @@ -796,14 +811,15 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, inputPath, vol->target.path, NULL); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { - virCommandAddArg(cmd, "-e"); - } + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS && + (do_encryption || preallocate)) { + virCommandAddArg(cmd, "-o"); + virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "", + (do_encryption && preallocate) ? "," : "", + preallocate ? "preallocation=metadata" : ""); + } else if (do_encryption) { + virCommandAddArg(cmd, "-e"); } - } else if (vol->backingStore.path) { virCommandAddArgList(cmd, "create", "-f", type, "-b", vol->backingStore.path, NULL); @@ -840,12 +856,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->target.path, NULL); virCommandAddArgFormat(cmd, "%lluK", size_arg); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { - virCommandAddArg(cmd, "-e"); - } + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS && + (do_encryption || preallocate)) { + virCommandAddArg(cmd, "-o"); + virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "", + (do_encryption && preallocate) ? "," : "", + preallocate ? "preallocation=metadata" : ""); + } else if (do_encryption) { + virCommandAddArg(cmd, "-e"); } } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 29cad9d..c991015 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -37,7 +37,8 @@ typedef int (*virStorageBackendStopPool)(virConnectPtr conn, virStoragePoolObjPt typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, virStoragePoolObjPtr pool, unsigned int flags); typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, - virStoragePoolObjPtr pool, virStorageVolDefPtr vol); + virStoragePoolObjPtr pool, virStorageVolDefPtr vol, + unsigned int flags); typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 4e6ebbf..7a54ead 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1045,7 +1045,8 @@ static int _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags) { virStorageBackendBuildVolFrom create_func; int tool_type; @@ -1078,7 +1079,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, return -1; } - if (create_func(conn, pool, vol, inputvol, 0) < 0) + if (create_func(conn, pool, vol, inputvol, flags) < 0) return -1; return 0; } @@ -1091,8 +1092,11 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, static int virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) { - return _virStorageBackendFileSystemVolBuild(conn, pool, vol, NULL); + virStorageVolDefPtr vol, + unsigned int flags) { + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); + + return _virStorageBackendFileSystemVolBuild(conn, pool, vol, NULL, flags); } /* @@ -1105,9 +1109,9 @@ virStorageBackendFileSystemVolBuildFrom(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags) { - virCheckFlags(0, -1); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); - return _virStorageBackendFileSystemVolBuild(conn, pool, vol, inputvol); + return _virStorageBackendFileSystemVolBuild(conn, pool, vol, inputvol, flags); } /** diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 3cb2312..c567fff 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1348,7 +1348,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, virStorageVolDefPtr voldef = NULL; virStorageVolPtr ret = NULL, volobj = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); @@ -1425,7 +1425,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, voldef->building = 1; virStoragePoolObjUnlock(pool); - buildret = backend->buildVol(obj->conn, pool, buildvoldef); + buildret = backend->buildVol(obj->conn, pool, buildvoldef, flags); storageDriverLock(driver); virStoragePoolObjLock(pool); @@ -1473,7 +1473,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, virStorageVolPtr ret = NULL, volobj = NULL; int buildret; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); -- 1.7.8.6
On 05.12.2012 11:48, Ján Tomko wrote:
Add VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA flag to virStorageVolCreateXML and virStorageVolCreateXMLFrom. This flag requests metadata preallocation when creating/cloning qcow2 images, resulting in creating a sparse file with qcow2 metadata. It has only slightly larger disk usage compared to new image with no allocation, but offers higher performance. --- include/libvirt/libvirt.h.in | 4 +++ src/libvirt.c | 16 ++++++++++-- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++----------- src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 16 ++++++++----- src/storage/storage_driver.c | 6 ++-- 6 files changed, 64 insertions(+), 27 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a19f37a..17804ca 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2906,6 +2906,10 @@ virStorageVolPtr virStorageVolLookupByPath (virConnectPtr conn, const char* virStorageVolGetName (virStorageVolPtr vol); const char* virStorageVolGetKey (virStorageVolPtr vol);
+typedef enum { + VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0, +} virStorageVolCreateFlags; + virStorageVolPtr virStorageVolCreateXML (virStoragePoolPtr pool, const char *xmldesc, unsigned int flags); diff --git a/src/libvirt.c b/src/libvirt.c index 4b7baab..6a7a817 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -13382,11 +13382,16 @@ virStorageVolGetKey(virStorageVolPtr vol) * virStorageVolCreateXML: * @pool: pointer to storage pool * @xmlDesc: description of volume to create - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virStorageVolCreateFlags * * Create a storage volume within a pool based * on an XML description. Not all pools support - * creation of volumes + * creation of volumes. + * + * Since 1.0.1 VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA + * in flags can be used to get higher performance with + * qcow2 image files which don't support full preallocation, + * by creating a sparse image file with metadata.
The 'Since 1.0.1' is not needed as users can simply 'git blame include/libvirt/libvirt.h.in'. Moreover, we are not using it anywhere among code. I'd reword this as 'The VIR_STORAGE_..._METADATA flag can be used to ...'.
* * Returns the storage volume, or NULL on error */ @@ -13433,13 +13438,18 @@ error: * @pool: pointer to parent pool for the new volume * @xmlDesc: description of volume to create * @clonevol: storage volume to use as input - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virStorageVolCreateFlags * * Create a storage volume in the parent pool, using the * 'clonevol' volume as input. Information for the new * volume (name, perms) are passed via a typical volume * XML description. * + * Since 1.0.1 VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA + * in flags can be used to get higher performance with + * qcow2 image files which don't support full preallocation, + * by creating a sparse image file with metadata. + *
Likewise.
* Returns the storage volume, or NULL on error */ virStorageVolPtr diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 83c4755..083028d 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -668,8 +668,11 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); unsigned long long int size_arg; + bool preallocate = false;
- virCheckFlags(0, -1); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); + + preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA);
const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ? @@ -697,11 +700,23 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, inputvol->target.format); return -1; } + if (preallocate && vol->target.format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata preallocation only available with qcow2")); + return -1; + }
if (vol->backingStore.path) { int accessRetCode = -1; char *absolutePath = NULL;
+ if (preallocate) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata preallocation conflicts with backing" + " store")); + return -1; + } + /* XXX: Not strictly required: qemu-img has an option a different * backing store, not really sure what use it serves though, and it * may cause issues with lvm. Untested essentially. @@ -796,14 +811,15 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, inputPath, vol->target.path, NULL);
- if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { - virCommandAddArg(cmd, "-e"); - } + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS && + (do_encryption || preallocate)) { + virCommandAddArg(cmd, "-o"); + virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "", + (do_encryption && preallocate) ? "," : "", + preallocate ? "preallocation=metadata" : ""); + } else if (do_encryption) { + virCommandAddArg(cmd, "-e"); } - } else if (vol->backingStore.path) { virCommandAddArgList(cmd, "create", "-f", type, "-b", vol->backingStore.path, NULL); @@ -840,12 +856,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->target.path, NULL); virCommandAddArgFormat(cmd, "%lluK", size_arg);
- if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { - virCommandAddArg(cmd, "-e"); - } + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS && + (do_encryption || preallocate)) { + virCommandAddArg(cmd, "-o"); + virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "", + (do_encryption && preallocate) ? "," : "", + preallocate ? "preallocation=metadata" : ""); + } else if (do_encryption) { + virCommandAddArg(cmd, "-e"); } }
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 29cad9d..c991015 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -37,7 +37,8 @@ typedef int (*virStorageBackendStopPool)(virConnectPtr conn, virStoragePoolObjPt typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, virStoragePoolObjPtr pool, unsigned int flags);
typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, - virStoragePoolObjPtr pool, virStorageVolDefPtr vol); + virStoragePoolObjPtr pool, virStorageVolDefPtr vol, + unsigned int flags); typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 4e6ebbf..7a54ead 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1045,7 +1045,8 @@ static int _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags) { virStorageBackendBuildVolFrom create_func; int tool_type; @@ -1078,7 +1079,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, return -1; }
- if (create_func(conn, pool, vol, inputvol, 0) < 0) + if (create_func(conn, pool, vol, inputvol, flags) < 0) return -1; return 0; } @@ -1091,8 +1092,11 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, static int virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) { - return _virStorageBackendFileSystemVolBuild(conn, pool, vol, NULL); + virStorageVolDefPtr vol, + unsigned int flags) { + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); + + return _virStorageBackendFileSystemVolBuild(conn, pool, vol, NULL, flags); }
/* @@ -1105,9 +1109,9 @@ virStorageBackendFileSystemVolBuildFrom(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags) { - virCheckFlags(0, -1); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1);
- return _virStorageBackendFileSystemVolBuild(conn, pool, vol, inputvol); + return _virStorageBackendFileSystemVolBuild(conn, pool, vol, inputvol, flags); }
/** diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 3cb2312..c567fff 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1348,7 +1348,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, virStorageVolDefPtr voldef = NULL; virStorageVolPtr ret = NULL, volobj = NULL;
- virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); @@ -1425,7 +1425,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, voldef->building = 1; virStoragePoolObjUnlock(pool);
- buildret = backend->buildVol(obj->conn, pool, buildvoldef); + buildret = backend->1nl(obj->conn, pool, buildvoldef, flags);
storageDriverLock(driver); virStoragePoolObjLock(pool); @@ -1473,7 +1473,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, virStorageVolPtr ret = NULL, volobj = NULL; int buildret;
- virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
Otherwise looking good. ACK. Michal
On 12/05/2012 09:29 AM, Michal Privoznik wrote:
+ * + * Since 1.0.1 VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA + * in flags can be used to get higher performance with + * qcow2 image files which don't support full preallocation, + * by creating a sparse image file with metadata.
The 'Since 1.0.1' is not needed as users can simply 'git blame include/libvirt/libvirt.h.in'. Moreover, we are not using it anywhere among code. I'd reword this as 'The VIR_STORAGE_..._METADATA flag can be used to ...'.
Someday, it would be really nice to expand http://libvirt.org/hvsupport.html to say which flags are supported since which version (that is, knowing that virDomainStartFlags() is available is interesting, but it is even more interesting to know _which_ drivers support which flags, and when support for those flags was added). But if we ever do come up with a way to list per-flag support for APIs, I think automating it to the hvsupport.html page is sufficient; we don't also need to call it out in the libvirt.c documentation. So I'm okay with the complaint of not needing the 'since 1.0.1' here. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add --prealloc-metadata flag to these commands: vol-clone vol-create vol-create-as vol-create-from --- tools/virsh-volume.c | 25 +++++++++++++++++++++---- tools/virsh.pod | 23 ++++++++++++++++++++--- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index f219de9..1ba0188 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -124,6 +124,7 @@ static const vshCmdOptDef opts_vol_create_as[] = { N_("the backing volume if taking a snapshot")}, {"backing-vol-format", VSH_OT_STRING, 0, N_("format of backing volume if taking a snapshot")}, + {"prealloc-metadata", VSH_OT_BOOL, 0, N_("preallocate metadata (for qcow2 instead of full allocation)")}, {NULL, 0, 0, NULL} }; @@ -146,7 +147,10 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) const char *snapshotStrVol = NULL, *snapshotStrFormat = NULL; unsigned long long capacity, allocation = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; + unsigned long flags = 0; + if (vshCommandOptBool(cmd, "prealloc-metadata")) + flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME))) return false; @@ -256,7 +260,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) goto cleanup; } xml = virBufferContentAndReset(&buf); - vol = virStorageVolCreateXML(pool, xml, 0); + vol = virStorageVolCreateXML(pool, xml, flags); VIR_FREE(xml); virStoragePoolFree(pool); @@ -287,6 +291,7 @@ static const vshCmdInfo info_vol_create[] = { static const vshCmdOptDef opts_vol_create[] = { {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pool name")}, {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file containing an XML vol description")}, + {"prealloc-metadata", VSH_OT_BOOL, 0, N_("preallocate metadata (for qcow2 instead of full allocation)")}, {NULL, 0, 0, NULL} }; @@ -297,8 +302,11 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) virStorageVolPtr vol; const char *from = NULL; bool ret = true; + unsigned int flags = 0; char *buffer; + if (vshCommandOptBool(cmd, "prealloc-metadata")) + flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME))) return false; @@ -314,7 +322,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) return false; } - vol = virStorageVolCreateXML(pool, buffer, 0); + vol = virStorageVolCreateXML(pool, buffer, flags); VIR_FREE(buffer); virStoragePoolFree(pool); @@ -343,6 +351,7 @@ static const vshCmdOptDef opts_vol_create_from[] = { {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file containing an XML vol description")}, {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("input vol name or key")}, {"inputpool", VSH_OT_STRING, 0, N_("pool name or uuid of the input volume's pool")}, + {"prealloc-metadata", VSH_OT_BOOL, 0, N_("preallocate metadata (for qcow2 instead of full allocation)")}, {NULL, 0, 0, NULL} }; @@ -354,10 +363,13 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; bool ret = false; char *buffer = NULL; + unsigned int flags = 0; if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL))) goto cleanup; + if (vshCommandOptBool(cmd, "prealloc-metadata")) + flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; if (vshCommandOptString(cmd, "file", &from) <= 0) { goto cleanup; } @@ -370,7 +382,7 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - newvol = virStorageVolCreateXMLFrom(pool, buffer, inputvol, 0); + newvol = virStorageVolCreateXMLFrom(pool, buffer, inputvol, flags); if (newvol != NULL) { vshPrint(ctl, _("Vol %s created from input vol %s\n"), @@ -434,6 +446,7 @@ static const vshCmdOptDef opts_vol_clone[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("orig vol name or key")}, {"newname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("clone name")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {"prealloc-metadata", VSH_OT_BOOL, 0, N_("preallocate metadata (for qcow2 instead of full allocation)")}, {NULL, 0, 0, NULL} }; @@ -446,10 +459,14 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd) char *origxml = NULL; xmlChar *newxml = NULL; bool ret = false; + unsigned int flags = 0; if (!(origvol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) goto cleanup; + if (vshCommandOptBool(cmd, "prealloc-metadata")) + flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + origpool = virStoragePoolLookupByVolume(origvol); if (!origpool) { vshError(ctl, "%s", _("failed to get parent pool")); @@ -469,7 +486,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - newvol = virStorageVolCreateXMLFrom(origpool, (char *) newxml, origvol, 0); + newvol = virStorageVolCreateXMLFrom(origpool, (char *) newxml, origvol, flags); if (newvol != NULL) { vshPrint(ctl, _("Vol %s cloned from %s\n"), diff --git a/tools/virsh.pod b/tools/virsh.pod index 7dde3df..b0e7064 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2405,13 +2405,17 @@ Returns the UUID of the named I<pool>. =over 4 -=item B<vol-create> I<pool-or-uuid> I<FILE> +=item B<vol-create> I<pool-or-uuid> I<FILE> [I<--prealloc-metadata>] Create a volume from an XML <file>. I<pool-or-uuid> is the name or UUID of the storage pool to create the volume in. I<FILE> is the XML <file> with the volume definition. An easy way to create the XML <file> is to use the B<vol-dumpxml> command to obtain the definition of a pre-existing volume. +[I<--prealloc-metadata>] preallocate metadata (for qcow2 images which don't +support full allocation). This option creates a sparse image file with metadata, +resulting in higher performance compared to images with no preallocation and +only slightly higher initial disk space usage. B<Example> @@ -2420,7 +2424,7 @@ B<Example> virsh vol-create differentstoragepool newvolume.xml =item B<vol-create-from> I<pool-or-uuid> I<FILE> [I<--inputpool> -I<pool-or-uuid>] I<vol-name-or-key-or-path> +I<pool-or-uuid>] I<vol-name-or-key-or-path> [I<--prealloc-metadata>] Create a volume, using another volume as input. I<pool-or-uuid> is the name or UUID of the storage pool to create the volume in. @@ -2428,10 +2432,15 @@ I<FILE> is the XML <file> with the volume definition. I<--inputpool> I<pool-or-uuid> is the name or uuid of the storage pool the source volume is in. I<vol-name-or-key-or-path> is the name or key or path of the source volume. +[I<--prealloc-metadata>] preallocate metadata (for qcow2 images which don't +support full allocation). This option creates a sparse image file with metadata, +resulting in higher performance compared to images with no preallocation and +only slightly higher initial disk space usage. =item B<vol-create-as> I<pool-or-uuid> I<name> I<capacity> [I<--allocation> I<size>] [I<--format> I<string>] [I<--backing-vol> I<vol-name-or-key-or-path>] [I<--backing-vol-format> I<string>] +[I<--prealloc-metadata>] Create a volume from a set of arguments. I<pool-or-uuid> is the name or UUID of the storage pool to create the volume @@ -2448,9 +2457,13 @@ volume to be used if taking a snapshot of an existing volume. I<--backing-vol-format> I<string> is the format of the snapshot backing volume; raw, bochs, qcow, qcow2, qed, vmdk, host_device. These are, however, meant for file based storage pools. +[I<--prealloc-metadata>] preallocate metadata (for qcow2 images which don't +support full allocation). This option creates a sparse image file with metadata, +resulting in higher performance compared to images with no preallocation and +only slightly higher initial disk space usage. =item B<vol-clone> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> -I<name> +I<name> [I<--prealloc-metadata>] Clone an existing volume. Less powerful, but easier to type, version of B<vol-create-from>. @@ -2458,6 +2471,10 @@ I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool to create the volume in. I<vol-name-or-key-or-path> is the name or key or path of the source volume. I<name> is the name of the new volume. +[I<--prealloc-metadata>] preallocate metadata (for qcow2 images which don't +support full allocation). This option creates a sparse image file with metadata, +resulting in higher performance compared to images with no preallocation and +only slightly higher initial disk space usage. =item B<vol-delete> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> -- 1.7.8.6
On 05.12.2012 11:48, Ján Tomko wrote:
Add --prealloc-metadata flag to these commands: vol-clone vol-create vol-create-as vol-create-from --- tools/virsh-volume.c | 25 +++++++++++++++++++++---- tools/virsh.pod | 23 ++++++++++++++++++++--- 2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index f219de9..1ba0188 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -124,6 +124,7 @@ static const vshCmdOptDef opts_vol_create_as[] = { N_("the backing volume if taking a snapshot")}, {"backing-vol-format", VSH_OT_STRING, 0, N_("format of backing volume if taking a snapshot")}, + {"prealloc-metadata", VSH_OT_BOOL, 0, N_("preallocate metadata (for qcow2 instead of full allocation)")}, {NULL, 0, 0, NULL} };
@@ -146,7 +147,10 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) const char *snapshotStrVol = NULL, *snapshotStrFormat = NULL; unsigned long long capacity, allocation = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; + unsigned long flags = 0;
+ if (vshCommandOptBool(cmd, "prealloc-metadata")) + flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
While this works for now, I'd rather do it as: flags |= VIR_STORAGE_..._METADATA; since this maybe easily overlooked, and reduces a future patch size if somebody invents a new flag and add its conditional setting in the front of yours. Otherwise looking good. ACK with this squashed in: diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 1ba0188..145953b 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -150,7 +150,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) unsigned long flags = 0; if (vshCommandOptBool(cmd, "prealloc-metadata")) - flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME))) return false; @@ -306,7 +306,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) char *buffer; if (vshCommandOptBool(cmd, "prealloc-metadata")) - flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME))) return false; @@ -369,7 +369,7 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (vshCommandOptBool(cmd, "prealloc-metadata")) - flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; if (vshCommandOptString(cmd, "file", &from) <= 0) { goto cleanup; } @@ -465,7 +465,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (vshCommandOptBool(cmd, "prealloc-metadata")) - flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; origpool = virStoragePoolLookupByVolume(origvol); if (!origpool) { Michal
On 05.12.2012 11:48, Ján Tomko wrote:
Diff to v1: * A flag for virStorageVolCreateXML and virStorageVolCreateXMLFrom is used instead of guessing from the allocation element. * The flag is exposed and documented in virsh.
Diff to v2: * merged first two patches to enable and implement the flag at the same time * more documentation * fixed spaces to pass syntax-check * !! for unsigned -> bool conversion * initialize flags in cmdVolCreateFrom and cmdVolClone
Ján Tomko (2): storage: allow metadata preallocation when creating qcow2 images virsh: allow metadata preallocation when creating volumes
include/libvirt/libvirt.h.in | 4 +++ src/libvirt.c | 16 ++++++++++-- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++----------- src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 16 ++++++++----- src/storage/storage_driver.c | 6 ++-- tools/virsh-volume.c | 25 +++++++++++++++++--- tools/virsh.pod | 23 ++++++++++++++++-- 8 files changed, 105 insertions(+), 34 deletions(-)
I've ACKed the both patches. However, I am giving time for anybody who wants to chime in and tell his opinion on 'Since 1.0.1' comment in the first patch. If nobody replies in near future (say by tomorrow) I'll push the patches. Possible objection can be pushed as several patch then. Michal
On 05.12.2012 17:32, Michal Privoznik wrote:
On 05.12.2012 11:48, Ján Tomko wrote:
Diff to v1: * A flag for virStorageVolCreateXML and virStorageVolCreateXMLFrom is used instead of guessing from the allocation element. * The flag is exposed and documented in virsh.
Diff to v2: * merged first two patches to enable and implement the flag at the same time * more documentation * fixed spaces to pass syntax-check * !! for unsigned -> bool conversion * initialize flags in cmdVolCreateFrom and cmdVolClone
Ján Tomko (2): storage: allow metadata preallocation when creating qcow2 images virsh: allow metadata preallocation when creating volumes
include/libvirt/libvirt.h.in | 4 +++ src/libvirt.c | 16 ++++++++++-- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++----------- src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 16 ++++++++----- src/storage/storage_driver.c | 6 ++-- tools/virsh-volume.c | 25 +++++++++++++++++--- tools/virsh.pod | 23 ++++++++++++++++-- 8 files changed, 105 insertions(+), 34 deletions(-)
I've ACKed the both patches. However, I am giving time for anybody who wants to chime in and tell his opinion on 'Since 1.0.1' comment in the first patch. If nobody replies in near future (say by tomorrow) I'll push the patches. Possible objection can be pushed as several patch then.
Michal
Pushed now. Michal
participants (3)
-
Eric Blake -
Ján Tomko -
Michal Privoznik