On 06/10/2013 02:10 PM, Ján Tomko wrote: [...] I guess we should use '< 0' here to unify the looks of it. Other than that it's way more readable than the previous version, so ACK with that fixed. Martin

Detect qcow2 images with version 3 in the image header as VIR_STORAGE_FILE_QCOW2. These images have a feature bitfield, with just one feature supported so far: lazy_refcounts. The header length changed too, moving the location of the backing format name. --- src/util/virstoragefile.c | 164 +++++++++++++++++++++++++++++++++++----------- src/util/virstoragefile.h | 12 ++++ 2 files changed, 139 insertions(+), 37 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a391738..f30324e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -59,6 +59,11 @@ VIR_ENUM_IMPL(virStorageFileFormat, "qcow", "qcow2", "qed", "vmdk", "vpc", "fat", "vhd", "vdi") +VIR_ENUM_IMPL(virStorageFileFeature, + VIR_STORAGE_FILE_FEATURE_LAST, + "lazy_refcounts", + ) + enum lv_endian { LV_LITTLE_ENDIAN = 1, /* 1234 */ LV_BIG_ENDIAN /* 4321 */ @@ -81,7 +86,7 @@ struct FileTypeInfo { * where we find version number, * -1 to always fail the version test, * -2 to always pass the version test */ - int versionNumber; /* Version number to validate */ + int versionNumbers[3]; /* Version numbers to validate, terminated by 0 */ int sizeOffset; /* Byte offset from start of file * where we find capacity info, * -1 to use st_size as capacity */ @@ -95,6 +100,8 @@ struct FileTypeInfo { * -1 if encryption is not used */ int (*getBackingStore)(char **res, int *format, const unsigned char *buf, size_t buf_size); + int (*getFeatures)(virBitmapPtr *features, int format, + unsigned char *buf, ssize_t len); }; static int cowGetBackingStore(char **, int *, @@ -103,6 +110,8 @@ static int qcow1GetBackingStore(char **, int *, const unsigned char *, size_t); static int qcow2GetBackingStore(char **, int *, const unsigned char *, size_t); +static int qcow2GetFeatures(virBitmapPtr *features, int format, + unsigned char *buf, ssize_t len); static int vmdk4GetBackingStore(char **, int *, const unsigned char *, size_t); static int @@ -122,6 +131,13 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t); #define QCOW2_HDR_EXTENSION_END 0 #define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA +#define QCOW2v3_HDR_FEATURES_INCOMPATIBLE (QCOW2_HDR_TOTAL_SIZE) +#define QCOW2v3_HDR_FEATURES_COMPATIBLE (QCOW2v3_HDR_FEATURES_INCOMPATIBLE+8) +#define QCOW2v3_HDR_FEATURES_AUTOCLEAR (QCOW2v3_HDR_FEATURES_COMPATIBLE+8) + +/* The location of the header size [4 bytes] */ +#define QCOW2v3_HDR_SIZE (QCOW2_HDR_TOTAL_SIZE+8+8+8+4) + #define QED_HDR_FEATURES_OFFSET (4+4+4+4) #define QED_HDR_IMAGE_SIZE (QED_HDR_FEATURES_OFFSET+8+8+8+8) #define QED_HDR_BACKING_FILE_OFFSET (QED_HDR_IMAGE_SIZE+8) @@ -137,16 +153,16 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t); static struct FileTypeInfo const fileTypeInfo[] = { [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, 0, 0, 0, 0, 0, NULL }, + -1, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_RAW] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, 0, 0, 0, 0, 0, NULL }, + -1, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_DIR] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, 0, 0, 0, 0, 0, NULL }, + -1, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_BOCHS] = { /*"Bochs Virtual HD Image", */ /* Untested */ 0, NULL, NULL, - LV_LITTLE_ENDIAN, 64, 0x20000, - 32+16+16+4+4+4+4+4, 8, 1, -1, NULL + LV_LITTLE_ENDIAN, 64, {0x20000, 0}, + 32+16+16+4+4+4+4+4, 8, 1, -1, NULL, NULL }, [VIR_STORAGE_FILE_CLOOP] = { /* #!/bin/sh @@ -154,67 +170,82 @@ static struct FileTypeInfo const fileTypeInfo[] = { modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1 */ /* Untested */ 0, NULL, NULL, - LV_LITTLE_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL + LV_LITTLE_ENDIAN, -1, {0}, + -1, 0, 0, -1, NULL, NULL }, [VIR_STORAGE_FILE_COW] = { 0, "OOOM", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+1024+4, 8, 1, -1, cowGetBackingStore + LV_BIG_ENDIAN, 4, {2, 0}, + 4+4+1024+4, 8, 1, -1, cowGetBackingStore, NULL }, [VIR_STORAGE_FILE_DMG] = { /* XXX QEMU says there's no magic for dmg, * /usr/share/misc/magic lists double magic (both offsets * would have to match) but then disables that check. */ 0, NULL, ".dmg", - 0, -1, 0, - -1, 0, 0, -1, NULL + 0, -1, {0}, + -1, 0, 0, -1, NULL, NULL }, [VIR_STORAGE_FILE_ISO] = { 32769, "CD001", ".iso", - LV_LITTLE_ENDIAN, -2, 0, - -1, 0, 0, -1, NULL + LV_LITTLE_ENDIAN, -2, {0}, + -1, 0, 0, -1, NULL, NULL }, [VIR_STORAGE_FILE_QCOW] = { 0, "QFI", NULL, - LV_BIG_ENDIAN, 4, 1, - QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW1_HDR_CRYPT, qcow1GetBackingStore, + LV_BIG_ENDIAN, 4, {1, 0}, + QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW1_HDR_CRYPT, qcow1GetBackingStore, NULL }, [VIR_STORAGE_FILE_QCOW2] = { 0, "QFI", NULL, - LV_BIG_ENDIAN, 4, 2, + LV_BIG_ENDIAN, 4, {2, 3}, QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW2_HDR_CRYPT, qcow2GetBackingStore, + qcow2GetFeatures }, [VIR_STORAGE_FILE_QED] = { /* http://wiki.qemu.org/Features/QED */ 0, "QED", NULL, - LV_LITTLE_ENDIAN, -2, -1, - QED_HDR_IMAGE_SIZE, 8, 1, -1, qedGetBackingStore, + LV_LITTLE_ENDIAN, -2, {0}, + QED_HDR_IMAGE_SIZE, 8, 1, -1, qedGetBackingStore, NULL }, [VIR_STORAGE_FILE_VMDK] = { 0, "KDMV", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 8, 512, -1, vmdk4GetBackingStore + LV_LITTLE_ENDIAN, 4, {1, 0}, + 4+4+4, 8, 512, -1, vmdk4GetBackingStore, NULL }, [VIR_STORAGE_FILE_VPC] = { 0, "conectix", NULL, - LV_BIG_ENDIAN, 12, 0x10000, - 8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, -1, NULL + LV_BIG_ENDIAN, 12, {0x10000, 0}, + 8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, -1, NULL, NULL }, /* TODO: add getBackingStore function */ [VIR_STORAGE_FILE_VDI] = { 64, "\x7f\x10\xda\xbe", ".vdi", - LV_LITTLE_ENDIAN, 68, 0x00010001, - 64 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL}, + LV_LITTLE_ENDIAN, 68, {0x00010001, 0}, + 64 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL, NULL}, /* Not direct file formats, but used for various drivers */ [VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, 0, 0, 0, 0, 0, NULL }, + -1, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_VHD] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, 0, 0, 0, 0, 0, NULL }, + -1, {0}, 0, 0, 0, 0, NULL, NULL }, }; verify(ARRAY_CARDINALITY(fileTypeInfo) == VIR_STORAGE_FILE_LAST); +/* qcow2 compatible features in the order they appear on-disk */ +enum qcow2CompatibleFeature { + QCOW2_COMPATIBLE_FEATURE_LAZY_REFCOUNTS = 0, + + QCOW2_COMPATIBLE_FEATURE_LAST +}; + +/* conversion to virStorageFeatures */ +static int qcow2CompatibleFeatureArray[] = { + VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS, +}; +verify(ARRAY_CARDINALITY(qcow2CompatibleFeatureArray) == + QCOW2_COMPATIBLE_FEATURE_LAST); + static int cowGetBackingStore(char **res, int *format, @@ -301,6 +332,8 @@ qcowXGetBackingStore(char **res, { unsigned long long offset; unsigned int size; + unsigned long long start; + int version; *res = NULL; if (format) @@ -351,11 +384,20 @@ qcowXGetBackingStore(char **res, * Thus the file region to search for extensions is * between the end of the header (QCOW2_HDR_TOTAL_SIZE) * and the start of the backingStoreName (offset) + * + * for qcow2 v3 images, the length of the header + * is stored at QCOW2v3_HDR_SIZE */ - if (isQCow2 && format && - qcow2GetBackingStoreFormat(format, buf, buf_size, QCOW2_HDR_TOTAL_SIZE, - offset) < 0) - return BACKING_STORE_INVALID; + if (isQCow2 && format) { + version = virReadBufInt32BE(buf + QCOWX_HDR_VERSION); + if (version == 2) + start = QCOW2_HDR_TOTAL_SIZE; + else + start = virReadBufInt32BE(buf + QCOW2v3_HDR_SIZE); + if (qcow2GetBackingStoreFormat(format, buf, buf_size, + start, offset) < 0) + return BACKING_STORE_INVALID; + } return BACKING_STORE_OK; } @@ -593,7 +635,7 @@ virStorageFileMatchesVersion(int format, unsigned char *buf, size_t buflen) { - int version; + int version, i; /* Validate version number info */ if (fileTypeInfo[format].versionOffset == -1) @@ -611,12 +653,14 @@ virStorageFileMatchesVersion(int format, else version = virReadBufInt32BE(buf + fileTypeInfo[format].versionOffset); - VIR_DEBUG("Compare detected version %d vs expected version %d", - version, fileTypeInfo[format].versionNumber); - if (version != fileTypeInfo[format].versionNumber) - return false; + for (i = 0; fileTypeInfo[format].versionNumbers[i] != 0; i++) { + VIR_DEBUG("Compare detected version %d vs expected version %d", + version, fileTypeInfo[format].versionNumbers[i]); + if (version == fileTypeInfo[format].versionNumbers[i]) + return true; + } - return true; + return false; } static bool @@ -669,6 +713,42 @@ cleanup: } +static int +qcow2GetFeatures(virBitmapPtr *features, + int format, + unsigned char *buf, + ssize_t len) +{ + int version = -1; + virBitmapPtr feat = NULL; + uint64_t bits; + int i; + + version = virReadBufInt32BE(buf + fileTypeInfo[format].versionOffset); + + if (version == 2) + return 0; + + if (len < QCOW2v3_HDR_SIZE) + return -1; + + if (!(feat = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) { + virReportOOMError(); + return -1; + } + + /* todo: check for incompatible or autoclear features? */ + bits = virReadBufInt64BE(buf + QCOW2v3_HDR_FEATURES_COMPATIBLE); + for (i = 0; i < QCOW2_COMPATIBLE_FEATURE_LAST; i++) { + if (bits & ((uint64_t) 1 << i)) + ignore_value(virBitmapSetBit(feat, qcow2CompatibleFeatureArray[i])); + } + + *features = feat; + return 0; +} + + /* Given a file descriptor FD open on PATH, and optionally opened from * a given DIRECTORY, return metadata about that file, assuming it has * the given FORMAT. */ @@ -803,6 +883,14 @@ virStorageFileGetMetadataInternal(const char *path, } } + if (fileTypeInfo[format].getFeatures != NULL && + fileTypeInfo[format].getFeatures(&meta->features, format, buf, len) < 0) + goto cleanup; + + if (format == VIR_STORAGE_FILE_QCOW2 && meta->features && + VIR_STRDUP(meta->compat, "1.1") < 0) + goto cleanup; + done: ret = meta; meta = NULL; @@ -1032,7 +1120,9 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta) virStorageFileFreeMetadata(meta->backingMeta); VIR_FREE(meta->backingStore); VIR_FREE(meta->backingStoreRaw); + VIR_FREE(meta->compat); VIR_FREE(meta->directory); + virBitmapFree(meta->features); VIR_FREE(meta); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c195c9a..a848fd0 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -24,6 +24,7 @@ #ifndef __VIR_STORAGE_FILE_H__ # define __VIR_STORAGE_FILE_H__ +# include "virbitmap.h" # include "virutil.h" enum virStorageFileFormat { @@ -51,6 +52,15 @@ enum virStorageFileFormat { VIR_ENUM_DECL(virStorageFileFormat); +enum virStorageFileFeature { + VIR_STORAGE_FILE_FEATURE_NONE = -1, + VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS, + + VIR_STORAGE_FILE_FEATURE_LAST +}; + +VIR_ENUM_DECL(virStorageFileFeature); + typedef struct _virStorageFileMetadata virStorageFileMetadata; typedef virStorageFileMetadata *virStorageFileMetadataPtr; struct _virStorageFileMetadata { @@ -62,6 +72,8 @@ struct _virStorageFileMetadata { virStorageFileMetadataPtr backingMeta; unsigned long long capacity; bool encrypted; + virBitmapPtr features; /* bits described by enum virStorageFileFeature */ + char *compat; }; # ifndef DEV_BSIZE -- 1.8.1.5

Add <features> and <compat> elements to volume target XML. <compat> is a string which for qcow2 represents the QEMU version it should be compatible with. Valid values are 0.10 and 1.1. 1.1 is implicit if the <features> element is present, otherwise qemu-img default is used. 0.10 can be specified to explicitly create older images after the qemu-img default changes. <features> contains optional features, so far <lazy_refcounts/> is available, which enables caching of reference counters, improving performance for snapshots. --- docs/formatstorage.html.in | 19 +++++++ docs/schemas/Makefile.am | 1 + docs/schemas/storagefilefeatures.rng | 24 +++++++++ docs/schemas/storagevol.rng | 7 +++ libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + src/conf/storage_conf.c | 74 +++++++++++++++++++++++++++ src/conf/storage_conf.h | 7 ++- src/libvirt_private.syms | 2 + src/storage/storage_backend_fs.c | 10 ++++ tests/storagevolxml2xmlin/vol-qcow2-1.1.xml | 32 ++++++++++++ tests/storagevolxml2xmlin/vol-qcow2-lazy.xml | 35 +++++++++++++ tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 33 ++++++++++++ tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 35 +++++++++++++ tests/storagevolxml2xmltest.c | 2 + 15 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 docs/schemas/storagefilefeatures.rng create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-1.1.xml create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-lazy.xml create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-1.1.xml create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-lazy.xml diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 1a45915..30a07f4 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -334,6 +334,10 @@ &lt;mode&gt;0744&lt;/mode&gt; &lt;label&gt;virt_image_t&lt;/label&gt; &lt;/permissions&gt; + &lt;compat&gt;1.1&lt;/compat&gt; + &lt;features&gt; + &lt;lazy_refcounts/&gt; + &lt;/features&gt; &lt;/target&gt;</pre> <dl> @@ -362,6 +366,21 @@ contains the MAC (eg SELinux) label string. <span class="since">Since 0.4.1</span> </dd> + <dt><code>compat</code></dt> + <dd>Specify compatibility level. So far, this is only used for + <code>type='qcow2'</code> volumes. Valid values are <code>0.10</code> + and <code>1.1</code> so far, specifying QEMU version the images should + be compatible with. If the <code>feature</code> element is present, + 1.1 is used. If omitted, qemu-img default is used. + <span class="since">Since 1.0.6</span> + </dd> + <dt><code>features</code></dt> + <dd>Format-specific features. Only used for <code>qcow2</code> now. + Valid sub-elements are: + <code>&lt;lazy_refcounts/&gt;</code> - allow delayed reference + counter updates. + <span class="since">Since 1.0.6</span> + </dd> </dl> <h3><a name="StorageVolBacking">Backing store elements</a></h3> diff --git a/docs/schemas/Makefile.am b/docs/schemas/Makefile.am index 8da2c67..47d1941 100644 --- a/docs/schemas/Makefile.am +++ b/docs/schemas/Makefile.am @@ -28,6 +28,7 @@ schema_DATA = \ nwfilter.rng \ secret.rng \ storageencryption.rng \ + storagefilefeatures.rng \ storagepool.rng \ storagevol.rng diff --git a/docs/schemas/storagefilefeatures.rng b/docs/schemas/storagefilefeatures.rng new file mode 100644 index 0000000..96a1829 --- /dev/null +++ b/docs/schemas/storagefilefeatures.rng @@ -0,0 +1,24 @@ +<?xml version="1.0"?> +<!-- A Relax NG schema for the libvirt volume features XML format --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" + datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + + <define name='compat'> + <element name='compat'> + <data type='string'> + <param name='pattern'>[^,]+</param> + </data> + </element> + </define> + <define name='fileFormatFeatures'> + <element name='features'> + <interleave> + <optional> + <element name='lazy_refcounts'> + <empty/> + </element> + </optional> + </interleave> + </element> + </define> +</grammar> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 4649d91..8bc5907 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -8,6 +8,7 @@ </start> <include href='storageencryption.rng'/> + <include href='storagefilefeatures.rng'/> <define name='vol'> @@ -113,6 +114,12 @@ <optional> <ref name='encryption'/> </optional> + <optional> + <ref name='compat'/> + </optional> + <optional> + <ref name='fileFormatFeatures'/> + </optional> </element> </define> diff --git a/libvirt.spec.in b/libvirt.spec.in index 8d43e6d..0bfda11 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2002,6 +2002,7 @@ fi %{_datadir}/libvirt/schemas/nwfilter.rng %{_datadir}/libvirt/schemas/secret.rng %{_datadir}/libvirt/schemas/storageencryption.rng +%{_datadir}/libvirt/schemas/storagefilefeatures.rng %{_datadir}/libvirt/schemas/storagepool.rng %{_datadir}/libvirt/schemas/storagevol.rng diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 9208329..aa39231 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -213,6 +213,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_datadir}/libvirt/schemas/nwfilter.rng %{mingw32_datadir}/libvirt/schemas/secret.rng %{mingw32_datadir}/libvirt/schemas/storageencryption.rng +%{mingw32_datadir}/libvirt/schemas/storagefilefeatures.rng %{mingw32_datadir}/libvirt/schemas/storagepool.rng %{mingw32_datadir}/libvirt/schemas/storagevol.rng %dir %{mingw32_datadir}/libvirt/api/ @@ -273,6 +274,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_datadir}/libvirt/schemas/nwfilter.rng %{mingw64_datadir}/libvirt/schemas/secret.rng %{mingw64_datadir}/libvirt/schemas/storageencryption.rng +%{mingw64_datadir}/libvirt/schemas/storagefilefeatures.rng %{mingw64_datadir}/libvirt/schemas/storagepool.rng %{mingw64_datadir}/libvirt/schemas/storagevol.rng %dir %{mingw64_datadir}/libvirt/api/ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index c58b728..35ad502 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -97,6 +97,8 @@ VIR_ENUM_IMPL(virStoragePoolSourceAdapterType, typedef const char *(*virStorageVolFormatToString)(int format); typedef int (*virStorageVolFormatFromString)(const char *format); +typedef const char *(*virStorageVolFeatureToString)(int feature); +typedef int (*virStorageVolFeatureFromString)(const char *feature); typedef const char *(*virStoragePoolFormatToString)(int format); typedef int (*virStoragePoolFormatFromString)(const char *format); @@ -107,6 +109,9 @@ struct _virStorageVolOptions { int defaultFormat; virStorageVolFormatToString formatToString; virStorageVolFormatFromString formatFromString; + virStorageVolFeatureToString featureToString; + virStorageVolFeatureFromString featureFromString; + int featureLast; }; /* Flags to indicate mandatory components in the pool source */ @@ -161,6 +166,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .defaultFormat = VIR_STORAGE_FILE_RAW, .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, + .featureFromString = virStorageFileFeatureTypeFromString, + .featureToString = virStorageFileFeatureTypeToString, + .featureLast = VIR_STORAGE_FILE_FEATURE_LAST, }, }, {.poolType = VIR_STORAGE_POOL_FS, @@ -174,6 +182,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .defaultFormat = VIR_STORAGE_FILE_RAW, .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, + .featureFromString = virStorageFileFeatureTypeFromString, + .featureToString = virStorageFileFeatureTypeToString, + .featureLast = VIR_STORAGE_FILE_FEATURE_LAST, }, }, {.poolType = VIR_STORAGE_POOL_NETFS, @@ -188,6 +199,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .defaultFormat = VIR_STORAGE_FILE_RAW, .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, + .featureFromString = virStorageFileFeatureTypeFromString, + .featureToString = virStorageFileFeatureTypeToString, + .featureLast = VIR_STORAGE_FILE_FEATURE_LAST, }, }, {.poolType = VIR_STORAGE_POOL_ISCSI, @@ -299,6 +313,8 @@ virStorageVolDefFree(virStorageVolDefPtr def) } VIR_FREE(def->source.extents); + VIR_FREE(def->target.compat); + virBitmapFree(def->target.features); VIR_FREE(def->target.path); VIR_FREE(def->target.perms.label); VIR_FREE(def->target.timestamps); @@ -1248,6 +1264,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *capacity = NULL; char *unit = NULL; xmlNodePtr node; + xmlNodePtr *nodes = NULL; + int i, n; options = virStorageVolOptionsForPoolType(pool->type); if (options == NULL) @@ -1335,12 +1353,46 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(format); } + ret->target.compat = virXPathString("string(./target/compat)", ctxt); + if (ret->target.compat && strchr(ret->target.compat, ',')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("forbidden characters in 'compat' attribute")); + goto cleanup; + } + + if (virXPathNode("./target/features", ctxt) && options->featureLast) { + if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0) + goto cleanup; + + if (!ret->target.compat && VIR_STRDUP(ret->target.compat, "1.1") < 0) + goto cleanup; + + if (!(ret->target.features = virBitmapNew(options->featureLast))) + goto no_memory; + + for (i = 0; i < n; i++) { + int f = options->featureFromString((const char*)nodes[i]->name); + + if (f < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unsupported feature %s"), + (const char*)nodes[i]->name); + goto cleanup; + } + ignore_value(virBitmapSetBit(ret->target.features, f)); + } + VIR_FREE(nodes); + } + if (virStorageDefParsePerms(ctxt, &ret->backingStore.perms, "./backingStore/permissions", DEFAULT_VOL_PERM_MODE) < 0) goto error; +no_memory: + virReportOOMError(); + cleanup: + VIR_FREE(nodes); VIR_FREE(allocation); VIR_FREE(capacity); VIR_FREE(unit); @@ -1476,6 +1528,28 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAdjustIndent(buf, -4); } + virBufferEscapeString(buf, " <compat>%s</compat>\n", def->compat); + + if (options->featureLast > 0 && def->features) { + int i; + bool b; + bool empty = virBitmapIsAllClear(def->features); + + if (empty) + virBufferAddLit(buf, " <features/>\n"); + else + virBufferAddLit(buf, " <features>\n"); + + for (i = 0; i < options->featureLast; i++) { + ignore_value(virBitmapGetBit(def->features, i, &b)); + if (b) + virBufferAsprintf(buf, " <%s/>\n", + options->featureToString(i)); + } + if (!empty) + virBufferAddLit(buf, " </features>\n"); + } + virBufferAsprintf(buf, " </%s>\n", type); return 0; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 8e739ff..3af59df 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -26,6 +26,7 @@ # include "internal.h" # include "storage_encryption_conf.h" +# include "virbitmap.h" # include "virthread.h" # include <libxml/tree.h> @@ -84,9 +85,11 @@ struct _virStorageVolTarget { virStoragePerms perms; virStorageTimestampsPtr timestamps; int type; /* only used by disk backend for partition type */ - - /* only used in vol->target, not in vol->backingstore. */ + /* The next three are currently only used in vol->target, + * not in vol->backingStore. */ virStorageEncryptionPtr encryption; + virBitmapPtr features; + char *compat; }; typedef struct _virStorageVolDef virStorageVolDef; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1ea7467..333a00a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1795,6 +1795,8 @@ virSocketAddrSetPort; # util/virstoragefile.h virStorageFileChainLookup; +virStorageFileFeatureTypeFromString; +virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; virStorageFileFreeMetadata; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b1efa50..3598d83 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -150,6 +150,16 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, */ } + virBitmapFree(target->features); + target->features = meta->features; + meta->features = NULL; + + if (meta->compat) { + VIR_FREE(target->compat); + target->compat = meta->compat; + meta->compat = NULL; + } + virStorageFileFreeMetadata(meta); return ret; diff --git a/tests/storagevolxml2xmlin/vol-qcow2-1.1.xml b/tests/storagevolxml2xmlin/vol-qcow2-1.1.xml new file mode 100644 index 0000000..e8df8b3 --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-qcow2-1.1.xml @@ -0,0 +1,32 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + <features/> + </target> + <backingStore> + <path>/var/lib/libvirt/images/BaseDemo.img</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2xmlin/vol-qcow2-lazy.xml b/tests/storagevolxml2xmlin/vol-qcow2-lazy.xml new file mode 100644 index 0000000..336342a --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-qcow2-lazy.xml @@ -0,0 +1,35 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + <compat>1.1</compat> + <features> + <lazy_refcounts/> + </features> + </target> + <backingStore> + <path>/var/lib/libvirt/images/BaseDemo.img</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml b/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml new file mode 100644 index 0000000..454ac11 --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml @@ -0,0 +1,33 @@ +<volume> + <name>OtherDemo.img</name> + <key>(null)</key> + <source> + </source> + <capacity unit='bytes'>5368709120</capacity> + <allocation unit='bytes'>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + <compat>1.1</compat> + <features/> + </target> + <backingStore> + <path>/var/lib/libvirt/images/BaseDemo.img</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml b/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml new file mode 100644 index 0000000..4e30ede --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml @@ -0,0 +1,35 @@ +<volume> + <name>OtherDemo.img</name> + <key>(null)</key> + <source> + </source> + <capacity unit='bytes'>5368709120</capacity> + <allocation unit='bytes'>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + <compat>1.1</compat> + <features> + <lazy_refcounts/> + </features> + </target> + <backingStore> + <path>/var/lib/libvirt/images/BaseDemo.img</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 07c79c1..e87b016 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -110,6 +110,8 @@ mymain(void) DO_TEST("pool-dir", "vol-file"); DO_TEST("pool-dir", "vol-file-backing"); DO_TEST("pool-dir", "vol-qcow2"); + DO_TEST("pool-dir", "vol-qcow2-1.1"); + DO_TEST("pool-dir", "vol-qcow2-lazy"); DO_TEST("pool-disk", "vol-partition"); DO_TEST("pool-logical", "vol-logical"); DO_TEST("pool-logical", "vol-logical-backing"); -- 1.8.1.5

Add -o compat= and -o lazy_refcounts options for qemu-img. --- src/storage/storage_backend.c | 36 +++++++++++++++++++++++-- tests/storagevolxml2argvdata/qcow2-1.1.argv | 1 + tests/storagevolxml2argvdata/qcow2-lazy.argv | 1 + tests/storagevolxml2argvdata/vol-qcow2-1.1.xml | 32 ++++++++++++++++++++++ tests/storagevolxml2argvdata/vol-qcow2-lazy.xml | 35 ++++++++++++++++++++++++ tests/storagevolxml2argvtest.c | 2 ++ 6 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/storagevolxml2argvdata/qcow2-1.1.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-lazy.argv create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-1.1.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-lazy.xml diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 0abc3f3..22cc051 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -655,9 +655,15 @@ static int virStorageBackendCreateQemuImgOpts(char **opts, const char *backingType, bool encryption, - bool preallocate) + bool preallocate, + int format, + const char *compat, + virBitmapPtr features) { virBuffer buf = VIR_BUFFER_INITIALIZER; + bool b; + int i; + if (backingType) virBufferAsprintf(&buf, "backing_fmt=%s,", backingType); if (encryption) @@ -665,6 +671,19 @@ virStorageBackendCreateQemuImgOpts(char **opts, if (preallocate) virBufferAddLit(&buf, "preallocation=metadata,"); + if (compat) + virBufferAsprintf(&buf, "compat=%s,", compat); + if (features && format == VIR_STORAGE_FILE_QCOW2) { + if (!compat) + virBufferAddLit(&buf, "compat=1.1,"); + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) + virBufferAsprintf(&buf, "%s,", + virStorageFileFeatureTypeToString(i)); + } + } + virBufferTrim(&buf, ",", -1); if (virBufferError(&buf)) { @@ -717,6 +736,16 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, _("metadata preallocation only available with qcow2")); return NULL; } + if (vol->target.compat && vol->target.format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("compatibility option only available with qcow2")); + return NULL; + } + if (vol->target.features && vol->target.format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("format features only available with qcow2")); + return NULL; + } if (inputvol) { if (!(inputPath = inputvol->target.path)) { @@ -838,7 +867,10 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { if (virStorageBackendCreateQemuImgOpts(&opts, backing ? backingType : NULL, - do_encryption, preallocate)) + do_encryption, preallocate, + vol->target.format, + vol->target.compat, + vol->target.features)) return NULL; if (opts) virCommandAddArgList(cmd, "-o", opts, NULL); diff --git a/tests/storagevolxml2argvdata/qcow2-1.1.argv b/tests/storagevolxml2argvdata/qcow2-1.1.argv new file mode 100644 index 0000000..797499f --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-1.1.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 -b /dev/null -o backing_fmt=raw,encryption=on,compat=1.1 /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-lazy.argv b/tests/storagevolxml2argvdata/qcow2-lazy.argv new file mode 100644 index 0000000..9160d47 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-lazy.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 -b /dev/null -o backing_fmt=raw,encryption=on,compat=1.1,lazy_refcounts /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/vol-qcow2-1.1.xml b/tests/storagevolxml2argvdata/vol-qcow2-1.1.xml new file mode 100644 index 0000000..696e1e0 --- /dev/null +++ b/tests/storagevolxml2argvdata/vol-qcow2-1.1.xml @@ -0,0 +1,32 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + <features/> + </target> + <backingStore> + <path>/dev/null</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2argvdata/vol-qcow2-lazy.xml b/tests/storagevolxml2argvdata/vol-qcow2-lazy.xml new file mode 100644 index 0000000..c1d7875 --- /dev/null +++ b/tests/storagevolxml2argvdata/vol-qcow2-lazy.xml @@ -0,0 +1,35 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + <compat>1.1</compat> + <features> + <lazy_refcounts/> + </features> + </target> + <backingStore> + <path>/dev/null</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 92ab2f2..25ff5a7 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -188,6 +188,8 @@ mymain(void) "qcow2-nobacking-none", 0, FMT_NONE); DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", "vol-file", "qcow2-nobacking-convert-none", 0, FMT_NONE); + DO_TEST(false, "pool-dir", "vol-qcow2-lazy", NULL, "qcow2-lazy", 0, FMT_OPTIONS); + DO_TEST(false, "pool-dir", "vol-qcow2-1.1", NULL, "qcow2-1.1", 0, FMT_OPTIONS); return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.1.5

Just to reduce the indentation levels. Remove the unneeded NULL check for disk->file, as virBufferEscapeString doesn't print anything with NULL arguments. --- src/conf/snapshot_conf.c | 53 ++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index c6b97d6..b500111 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -548,6 +548,33 @@ cleanup: return ret; } +static void +virDomainSnapshotDiskDefFormat(virBufferPtr buf, + virDomainSnapshotDiskDefPtr disk) +{ + if (!disk->name) + return; + + virBufferEscapeString(buf, " <disk name='%s'", disk->name); + if (disk->snapshot > 0) + virBufferAsprintf(buf, " snapshot='%s'", + virDomainSnapshotLocationTypeToString(disk->snapshot)); + if (!disk->file && disk->format == 0) { + virBufferAddLit(buf, "/>\n"); + return; + } + + virBufferAddLit(buf, ">\n"); + + virBufferAdjustIndent(buf, 6); + if (disk->format > 0) + virBufferEscapeString(buf, "<driver type='%s'/>\n", + virStorageFileFormatTypeToString(disk->format)); + virBufferEscapeString(buf, "<source file='%s'/>\n", disk->file); + virBufferAdjustIndent(buf, -6); + virBufferAddLit(buf, " </disk>\n"); +} + char *virDomainSnapshotDefFormat(const char *domain_uuid, virDomainSnapshotDefPtr def, unsigned int flags, @@ -583,30 +610,8 @@ char *virDomainSnapshotDefFormat(const char *domain_uuid, } if (def->ndisks) { virBufferAddLit(&buf, " <disks>\n"); - for (i = 0; i < def->ndisks; i++) { - virDomainSnapshotDiskDefPtr disk = &def->disks[i]; - - if (!disk->name) - continue; - - virBufferEscapeString(&buf, " <disk name='%s'", disk->name); - if (disk->snapshot) - virBufferAsprintf(&buf, " snapshot='%s'", - virDomainSnapshotLocationTypeToString(disk->snapshot)); - if (disk->file || disk->format > 0) { - virBufferAddLit(&buf, ">\n"); - if (disk->format > 0) - virBufferEscapeString(&buf, " <driver type='%s'/>\n", - virStorageFileFormatTypeToString( - disk->format)); - if (disk->file) - virBufferEscapeString(&buf, " <source file='%s'/>\n", - disk->file); - virBufferAddLit(&buf, " </disk>\n"); - } else { - virBufferAddLit(&buf, "/>\n"); - } - } + for (i = 0; i < def->ndisks; i++) + virDomainSnapshotDiskDefFormat(&buf, &def->disks[i]); virBufferAddLit(&buf, " </disks>\n"); } if (def->dom) { -- 1.8.1.5

For live external snapshots, the image creation is done by QEMU and we can't pass the needed options. --- docs/formatsnapshot.html.in | 5 ++++ src/qemu/qemu_driver.c | 72 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 76689cb..e160455 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -170,6 +170,11 @@ snapshots, the original file name becomes the read-only snapshot, and the new file name contains the read-write delta of all disk changes since the snapshot. + + An optional <code>compat</code> and <code>features</code> + elements may be specified to control format-specific + features. So far only for qcow2 and only for non-live + external snapshots. </dd> </dl> </dd> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 51952c9..09805da 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11008,6 +11008,44 @@ qemuDomainSnapshotCreateInactiveInternal(virQEMUDriverPtr driver, return qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-c", false); } +static int +qemuDomainSnapshotCreateQemuImgOpts(char **opts, + const char *backing_file, + const char *backing_fmt, + const char *compat, + int format, + virBitmapPtr features) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool b; + int i; + + virBufferEscapeString(&buf, "backing_file=%s,", backing_file); + virBufferEscapeString(&buf, "backing_fmt=%s,", backing_fmt); + virBufferEscapeString(&buf, "compat=%s,", compat); + if (features && format == VIR_STORAGE_FILE_QCOW2) { + if (!compat) + virBufferAddLit(&buf, "compat=1.1,"); + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) + virBufferAsprintf(&buf, "%s,", + virStorageFileFeatureTypeToString(i)); + } + } + + virBufferTrim(&buf, ",", -1); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return -1; + } + + *opts = virBufferContentAndReset(&buf); + return 0; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, @@ -11023,6 +11061,8 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, virBitmapPtr created = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int ret = -1; + char *opts = NULL; + const char *backing_fmt = NULL; if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) goto cleanup; @@ -11053,23 +11093,25 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, NULL))) goto cleanup; - if (defdisk->format > 0) { - /* adds cmd line arg: backing_file=/path/to/backing/file,backing_fmd=format */ - virCommandAddArgFormat(cmd, "backing_file=%s,backing_fmt=%s", - defdisk->src, - virStorageFileFormatTypeToString(defdisk->format)); - } else { - if (!cfg->allowDiskFormatProbing) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown image format of '%s' and " - "format probing is disabled"), - defdisk->src); - goto cleanup; - } - /* adds cmd line arg: backing_file=/path/to/backing/file */ - virCommandAddArgFormat(cmd, "backing_file=%s", defdisk->src); + if (defdisk->format > 0) { + backing_fmt = virStorageFileFormatTypeToString(defdisk->format); + } else if (!cfg->allowDiskFormatProbing) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown image format of '%s' and " + "format probing is disabled"), + defdisk->src); + goto cleanup; } + if (qemuDomainSnapshotCreateQemuImgOpts(&opts, defdisk->src, + backing_fmt, + snapdisk->compat, + snapdisk->format, + snapdisk->features) < 0) + goto cleanup; + + virCommandAddArg(cmd, opts); + VIR_FREE(opts); /* adds cmd line args: /path/to/target/file */ virCommandAddArg(cmd, snapdisk->file); -- 1.8.1.5