[libvirt] [PATCH v2 0/3] storage: allow metadata preallocation when creating qcow2 images

Add support for preallocating metadata 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. Ján Tomko (3): storage: add a flag for metadata preallocation to VolCreate storage: allow metadata preallocation for qcow2 images virsh: allow metadata preallocation when creating volumes include/libvirt/libvirt.h.in | 4 +++ src/libvirt.c | 4 +- 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 | 11 ++++++-- 8 files changed, 82 insertions(+), 33 deletions(-) -- 1.7.8.6

--- include/libvirt/libvirt.h.in | 4 ++++ src/libvirt.c | 4 ++-- src/storage/storage_backend.h | 3 ++- src/storage/storage_backend_fs.c | 16 ++++++++++------ src/storage/storage_driver.c | 6 +++--- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bf584a0..d41be3f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2900,6 +2900,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 bdb1dc6..6f08d19 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -13299,7 +13299,7 @@ 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 @@ -13350,7 +13350,7 @@ 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 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 3322677..2a95174 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1049,7 +1049,8 @@ static int _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags) { virStorageBackendBuildVolFrom create_func; int tool_type; @@ -1082,7 +1083,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; } @@ -1095,8 +1096,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); } /* @@ -1109,9 +1113,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 faca2a2..98037ee 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1365,7 +1365,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); @@ -1442,7 +1442,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); @@ -1490,7 +1490,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 11/12/12 17:08, Ján Tomko wrote:
--- include/libvirt/libvirt.h.in | 4 ++++ src/libvirt.c | 4 ++-- src/storage/storage_backend.h | 3 ++- src/storage/storage_backend_fs.c | 16 ++++++++++------ src/storage/storage_driver.c | 6 +++--- 5 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bf584a0..d41be3f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2900,6 +2900,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 bdb1dc6..6f08d19 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -13299,7 +13299,7 @@ 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 @@ -13350,7 +13350,7 @@ 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
Here at the documentation for these APIs you should write up what the new flag actually does when used.
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 3322677..2a95174 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1049,7 +1049,8 @@ static int _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags) { virStorageBackendBuildVolFrom create_func; int tool_type; @@ -1082,7 +1083,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; } @@ -1095,8 +1096,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);
You shouldn't enable support of flags before the actual code that implements them goes in.
+ + return _virStorageBackendFileSystemVolBuild(conn, pool, vol, NULL, flags); }
/* @@ -1109,9 +1113,9 @@ virStorageBackendFileSystemVolBuildFrom(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags) { - virCheckFlags(0, -1); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1);
Same here.
- 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 faca2a2..98037ee 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1365,7 +1365,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, virStorageVolDefPtr voldef = NULL; virStorageVolPtr ret = NULL, volobj = NULL;
- virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
and here.
storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); @@ -1442,7 +1442,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); @@ -1490,7 +1490,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, virStorageVolPtr ret = NULL, volobj = NULL; int buildret;
- virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
and here ...
storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);

Metadata preallocation is supported both for creating new images or converting existing ones. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=684793 --- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++++------------ 1 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 41a19a1..bb20f88 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -670,8 +670,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 ? @@ -699,11 +702,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. @@ -798,14 +813,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); @@ -842,12 +858,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"); } } -- 1.7.8.6

On 11/12/12 17:08, Ján Tomko wrote:
Metadata preallocation is supported both for creating new images or converting existing ones.
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=684793 --- src/storage/storage_backend.c | 46 ++++++++++++++++++++++++++++------------ 1 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 41a19a1..bb20f88 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -670,8 +670,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;
This will work in most of the cases, but as preallocate is a bool, please use !! to typecast it to bool: preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA);
const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ?
Otherwise looks good. Peter

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 | 11 ++++++++--- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index f219de9..223487e 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")}, {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")}, {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")}, {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; 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")}, {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; 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 0984e6e..f8fa6aa 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2364,13 +2364,14 @@ 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) B<Example> @@ -2379,7 +2380,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. @@ -2387,10 +2388,12 @@ 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) =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 @@ -2407,9 +2410,10 @@ 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) =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>. @@ -2417,6 +2421,7 @@ 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) =item B<vol-delete> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> -- 1.7.8.6

On 11/12/12 17:08, 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 | 11 ++++++++--- 2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index f219de9..223487e 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")}, {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"))
Missing space after if. Breaks syntax-check.
+ 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")}, {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"))
here too...
+ 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")}, {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;
You are not initializing flags if --prealloc-metadata is not specified
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL))) goto cleanup;
+ if(vshCommandOptBool(cmd, "prealloc-metadata"))
(missing space)
+ 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);
and calling libvirt API with possible garbage.
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")}, {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;
Uninitialized flags.
if (!(origvol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) goto cleanup;
+ if(vshCommandOptBool(cmd, "prealloc-metadata"))
missing space.
+ 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 0984e6e..f8fa6aa 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2364,13 +2364,14 @@ 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)
I haven't read the other patches so I don't know what this actually does. And the documentation here doesn't help much. From the point of view of the user (who hasn't read the code) it's the same as me. Could you please add more docs what this option actually does?
B<Example>
@@ -2379,7 +2380,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. @@ -2387,10 +2388,12 @@ 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)
=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 @@ -2407,9 +2410,10 @@ 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)
=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>. @@ -2417,6 +2421,7 @@ 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)
=item B<vol-delete> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path>
Peter
participants (2)
-
Ján Tomko
-
Peter Krempa