Re: [libvirt] [PATCH v3 1/2] storage: allow metadata preallocation when creating qcow2 images

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->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);