[libvirt] [PATCH v5 0/5] Add qcow2 extensions support

Since QEMU 1.1 qcow2 supports a features bitfield, allowing to implement additional features without breaking compatibility with older QEMU versions. The version header on-disk is bumped to 3, however the format name is still 'qcow2', not 'qcow3' (which is what I used in v2). v5: Use [0-9]+.[0-9]+ for compat value and enforce it in XML. Fix broken rebase (error vs. cleanup labels). Check if the specified feature is available with the current compat level. v4: rebased https://www.redhat.com/archives/libvir-list/2013-June/msg00427.html v3: https://www.redhat.com/archives/libvir-list/2013-May/msg01179.html v2: https://www.redhat.com/archives/libvir-list/2013-February/msg00212.html qcow2 vs. qcow3 discussion: https://www.redhat.com/archives/libvir-list/2013-March/msg00094.html Ján Tomko (5): util: add support for qcow2v3 image detection conf: add features to volume target XML storage: add support for creating qcow2 images with extensions conf: add features to snapshot disk XML qemu: add qcow2 extension support to inactive external snapshots docs/formatsnapshot.html.in | 5 + docs/formatstorage.html.in | 20 +++ docs/schemas/Makefile.am | 1 + docs/schemas/domainsnapshot.rng | 7 + docs/schemas/storagefilefeatures.rng | 24 ++++ docs/schemas/storagevol.rng | 7 + libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + src/conf/snapshot_conf.c | 84 +++++++++++ src/conf/snapshot_conf.h | 2 + src/conf/storage_conf.c | 78 +++++++++++ src/conf/storage_conf.h | 7 +- src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 88 ++++++++++-- src/storage/storage_backend.c | 62 ++++++++- src/storage/storage_backend_fs.c | 10 ++ src/util/virstoragefile.c | 169 ++++++++++++++++++----- src/util/virstoragefile.h | 12 ++ tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 5 + tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 4 + 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 + 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 + 30 files changed, 737 insertions(+), 61 deletions(-) create mode 100644 docs/schemas/storagefilefeatures.rng 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 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 -- 1.8.1.5

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 | 169 ++++++++++++++++++++++++++++++++++++---------- src/util/virstoragefile.h | 12 ++++ 2 files changed, 144 insertions(+), 37 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a391738..27aa4fe 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 */ @@ -70,6 +75,8 @@ enum { BACKING_STORE_ERROR, }; +#define FILE_TYPE_VERSIONS_LAST 2 + /* Either 'magic' or 'extension' *must* be provided */ struct FileTypeInfo { int magicOffset; /* Byte offset of the magic */ @@ -81,7 +88,8 @@ 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[FILE_TYPE_VERSIONS_LAST]; + /* Version numbers to validate. Zeroes are ignored. */ int sizeOffset; /* Byte offset from start of file * where we find capacity info, * -1 to use st_size as capacity */ @@ -95,6 +103,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 +113,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 +134,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 +156,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}, + 32+16+16+4+4+4+4+4, 8, 1, -1, NULL, NULL }, [VIR_STORAGE_FILE_CLOOP] = { /* #!/bin/sh @@ -154,67 +173,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}, + 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}, + 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}, + 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}, + 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}, + 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 virStorageFileFeature */ +static const 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 +335,8 @@ qcowXGetBackingStore(char **res, { unsigned long long offset; unsigned int size; + unsigned long long start; + int version; *res = NULL; if (format) @@ -351,11 +387,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 +638,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 +656,16 @@ 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; + i < FILE_TYPE_VERSIONS_LAST && fileTypeInfo[format].versionNumbers[i]; + i++) { + VIR_DEBUG("Compare detected version %d vs one of the expected versions %d", + version, fileTypeInfo[format].versionNumbers[i]); + if (version == fileTypeInfo[format].versionNumbers[i]) + return true; + } - return true; + return false; } static bool @@ -669,6 +718,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 +888,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 +1125,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

On 06/19/2013 05:24 PM, Ján Tomko wrote:
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. ---
Everything was fixed, so my ACK still stands, Martin

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 | 20 +++++++ 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 | 78 +++++++++++++++++++++++++++ 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, 287 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..c1c1b15 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -334,6 +334,10 @@ <mode>0744</mode> <label>virt_image_t</label> </permissions> + <compat>1.1</compat> + <features> + <lazy_refcounts/> + </features> </target></pre> <dl> @@ -362,6 +366,22 @@ 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.7</span> + </dd> + <dt><code>features</code></dt> + <dd>Format-specific features. Only used for <code>qcow2</code> now. + Valid sub-elements are: + <ul> + <li><code><lazy_refcounts/></code> - allow delayed reference + counter updates. <span class="since">Since 1.0.7</span></li> + </ul> + </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..424b4e2 --- /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'>[0-9]+\.[0-9]+</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 e357a3d..a0a390e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2006,6 +2006,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..288e265 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,8 @@ struct _virStorageVolOptions { int defaultFormat; virStorageVolFormatToString formatToString; virStorageVolFormatFromString formatFromString; + virStorageVolFeatureToString featureToString; + virStorageVolFeatureFromString featureFromString; }; /* Flags to indicate mandatory components in the pool source */ @@ -161,6 +165,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .defaultFormat = VIR_STORAGE_FILE_RAW, .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, + .featureFromString = virStorageFileFeatureTypeFromString, + .featureToString = virStorageFileFeatureTypeToString, }, }, {.poolType = VIR_STORAGE_POOL_FS, @@ -174,6 +180,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .defaultFormat = VIR_STORAGE_FILE_RAW, .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, + .featureFromString = virStorageFileFeatureTypeFromString, + .featureToString = virStorageFileFeatureTypeToString, }, }, {.poolType = VIR_STORAGE_POOL_NETFS, @@ -188,6 +196,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .defaultFormat = VIR_STORAGE_FILE_RAW, .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, + .featureFromString = virStorageFileFeatureTypeFromString, + .featureToString = virStorageFileFeatureTypeToString, }, }, {.poolType = VIR_STORAGE_POOL_ISCSI, @@ -299,6 +309,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 +1260,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,17 +1349,59 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(format); } + ret->target.compat = virXPathString("string(./target/compat)", ctxt); + if (ret->target.compat) { + char **version = virStringSplit(ret->target.compat, ".", 2); + unsigned int result; + + if (!version || !version[1] || + virStrToLong_ui(version[0], NULL, 10, &result) < 0 || + virStrToLong_ui(version[1], NULL, 10, &result) < 0) { + virStringFreeList(version); + virReportError(VIR_ERR_XML_ERROR, "%s", + _("forbidden characters in 'compat' attribute")); + goto error; + } + virStringFreeList(version); + } + + if (options->featureFromString && virXPathNode("./target/features", ctxt)) { + if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0) + goto error; + + if (!ret->target.compat && VIR_STRDUP(ret->target.compat, "1.1") < 0) + goto error; + + if (!(ret->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) + 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 error; + } + 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; cleanup: + VIR_FREE(nodes); VIR_FREE(allocation); VIR_FREE(capacity); VIR_FREE(unit); return ret; +no_memory: + virReportOOMError(); error: virStorageVolDefFree(ret); ret = NULL; @@ -1476,6 +1532,28 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAdjustIndent(buf, -4); } + virBufferEscapeString(buf, " <compat>%s</compat>\n", def->compat); + + if (options->featureToString && 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 < VIR_STORAGE_FILE_FEATURE_LAST; 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 b449293..7ac6fdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1799,6 +1799,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

On 06/19/2013 05:24 PM, Ján Tomko wrote:
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. ---
ACK, Martin

Add -o compat= and -o lazy_refcounts options for qemu-img. --- src/storage/storage_backend.c | 62 ++++++++++++++++++++++--- 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, 126 insertions(+), 7 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 deea850..b2c9608 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -633,9 +633,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) @@ -643,16 +649,45 @@ virStorageBackendCreateQemuImgOpts(char **opts, if (preallocate) virBufferAddLit(&buf, "preallocation=metadata,"); + if (compat) + virBufferAsprintf(&buf, "compat=%s,", compat); + if (features && format == VIR_STORAGE_FILE_QCOW2) { + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) { + switch (i) { + case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS: + if (STREQ(compat, "0.10")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Feature %s not supported with compat" + " level %s"), + virStorageFileFeatureTypeToString(i), + compat); + goto error; + } + break; + case VIR_STORAGE_FILE_FEATURE_LAST: + ; + } + virBufferAsprintf(&buf, "%s,", + virStorageFileFeatureTypeToString(i)); + } + } + } + virBufferTrim(&buf, ",", -1); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } + if (virBufferError(&buf)) + goto no_memory; *opts = virBufferContentAndReset(&buf); return 0; + +no_memory: + virReportOOMError(); +error: + virBufferFreeAndReset(&buf); + return -1; } virCommandPtr @@ -695,6 +730,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)) { @@ -816,7 +861,10 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { if (virStorageBackendCreateQemuImgOpts(&opts, backing ? backingType : NULL, - do_encryption, preallocate) < 0) + do_encryption, preallocate, + vol->target.format, + vol->target.compat, + vol->target.features) < 0) 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

On 06/19/2013 05:24 PM, Ján Tomko wrote:
Add -o compat= and -o lazy_refcounts options for qemu-img. --- src/storage/storage_backend.c | 62 ++++++++++++++++++++++--- 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, 126 insertions(+), 7 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 deea850..b2c9608 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -633,9 +633,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) @@ -643,16 +649,45 @@ virStorageBackendCreateQemuImgOpts(char **opts, if (preallocate) virBufferAddLit(&buf, "preallocation=metadata,");
+ if (compat) + virBufferAsprintf(&buf, "compat=%s,", compat); + if (features && format == VIR_STORAGE_FILE_QCOW2) { + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) { + switch (i) {
Either have that 'i' as virStorageFeature (or whatever the name of the enum is) or cast it here so there is a check for all possible values without default).
+ case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS: + if (STREQ(compat, "0.10")) {
/STREQ/STREQ_NULLABLE/ [...]
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);
Wrap those lines and add a test with compat=0.10,lazy_refcounts=on. ACK with nits fixed. Martin

On 06/20/2013 02:00 PM, Martin Kletzander wrote:
On 06/19/2013 05:24 PM, Ján Tomko wrote:
Add -o compat= and -o lazy_refcounts options for qemu-img. --- src/storage/storage_backend.c | 62 ++++++++++++++++++++++--- 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, 126 insertions(+), 7 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 deea850..b2c9608 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -633,9 +633,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) @@ -643,16 +649,45 @@ virStorageBackendCreateQemuImgOpts(char **opts, if (preallocate) virBufferAddLit(&buf, "preallocation=metadata,");
+ if (compat) + virBufferAsprintf(&buf, "compat=%s,", compat); + if (features && format == VIR_STORAGE_FILE_QCOW2) { + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) { + switch (i) {
Either have that 'i' as virStorageFeature (or whatever the name of the enum is) or cast it here so there is a check for all possible values without default).
+ case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS: + if (STREQ(compat, "0.10")) {
/STREQ/STREQ_NULLABLE/
[...]
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);
Wrap those lines and add a test with compat=0.10,lazy_refcounts=on.
ACK with nits fixed.
Fixed and pushed along with patches 1 and 2. Thank you! Jan

On 06/19/2013 11:24 AM, Ján Tomko wrote:
Add -o compat= and -o lazy_refcounts options for qemu-img. --- src/storage/storage_backend.c | 62 ++++++++++++++++++++++--- 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, 126 insertions(+), 7 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 deea850..b2c9608 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -633,9 +633,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) @@ -643,16 +649,45 @@ virStorageBackendCreateQemuImgOpts(char **opts, if (preallocate) virBufferAddLit(&buf, "preallocation=metadata,");
+ if (compat) + virBufferAsprintf(&buf, "compat=%s,", compat); + if (features && format == VIR_STORAGE_FILE_QCOW2) { + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) { + switch (i) { + case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS: + if (STREQ(compat, "0.10")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Feature %s not supported with compat" + " level %s"), + virStorageFileFeatureTypeToString(i), + compat); + goto error; + } + break;
With the patches for virNetDevSetupControlFull() my Coverity now runs; however, it's not very happy here... The pushed source code has: ... + case VIR_STORAGE_FILE_FEATURE_NONE: + case VIR_STORAGE_FILE_FEATURE_LAST: + ; But VIR_STORAGE_FILE_FEATURE_NONE = -1 and VIR_STORAGE_FILE_FEATURE_LAST is used as the loop ender and thus Coverity deems the code unreachable eliciting a "DEADCODE" error since 'i' could never be one or the other. I suppose the loop could change to: for (i = VIR_STORAGE_FILE_FEATURE_NONE; i <= VIR_STORAGE_FILE_FEATURE_LAST; i++) but that'd cause virBitmapGetBit() failure since 'i' cannot be negative in the call to it. John
+ case VIR_STORAGE_FILE_FEATURE_LAST: + ; + } + virBufferAsprintf(&buf, "%s,", + virStorageFileFeatureTypeToString(i)); + } + } + } + virBufferTrim(&buf, ",", -1); ...

On 06/21/2013 08:40 PM, John Ferlan wrote:
On 06/19/2013 11:24 AM, Ján Tomko wrote:
+ if (features && format == VIR_STORAGE_FILE_QCOW2) { + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) { + switch (i) { + case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS: + if (STREQ(compat, "0.10")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Feature %s not supported with compat" + " level %s"), + virStorageFileFeatureTypeToString(i), + compat); + goto error; + } + break;
With the patches for virNetDevSetupControlFull() my Coverity now runs; however, it's not very happy here...
The pushed source code has: ... + case VIR_STORAGE_FILE_FEATURE_NONE: + case VIR_STORAGE_FILE_FEATURE_LAST: + ;
But VIR_STORAGE_FILE_FEATURE_NONE = -1 and VIR_STORAGE_FILE_FEATURE_LAST is used as the loop ender and thus Coverity deems the code unreachable eliciting a "DEADCODE" error since 'i' could never be one or the other.
I suppose the loop could change to:
for (i = VIR_STORAGE_FILE_FEATURE_NONE; i <= VIR_STORAGE_FILE_FEATURE_LAST; i++)
but that'd cause virBitmapGetBit() failure since 'i' cannot be negative in the call to it.
John
I've pushed a fix for that: commit 7a99eb912f8c99c23dc3e015a35224fc2c925459 Author: Ján Tomko <jtomko@redhat.com> AuthorDate: 2013-06-24 08:35:59 +0200 Commit: Ján Tomko <jtomko@redhat.com> CommitDate: 2013-06-24 08:44:46 +0200 Get rid of useless VIR_STORAGE_FILE_FEATURE_NONE It's not used anywhere except for the switch in virStorageBackendCreateQemuImgOpts, where leaving it in causes a dead code coverity warning and omitting it breaks compilation because of unhandled enum value. Introduced by 6298f74. git describe: v1.0.6-119-g7a99eb9 Jan

Add <features> and <compat> elements to snapshot disk 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. --- docs/schemas/domainsnapshot.rng | 7 ++ src/conf/snapshot_conf.c | 84 ++++++++++++++++++++++++ src/conf/snapshot_conf.h | 2 + tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 5 ++ tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 4 ++ 5 files changed, 102 insertions(+) diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 7b46df1..63c8511 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -6,6 +6,7 @@ </start> <include href='domaincommon.rng'/> + <include href='storagefilefeatures.rng'/> <define name='domainsnapshot'> <element name='domainsnapshot'> @@ -144,6 +145,12 @@ <empty/> </element> </optional> + <optional> + <ref name='compat'/> + </optional> + <optional> + <ref name='fileFormatFeatures'/> + </optional> </interleave> </group> </choice> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index b500111..3596413 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -82,6 +82,9 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) { VIR_FREE(disk->name); VIR_FREE(disk->file); + VIR_FREE(disk->compat); + virBitmapFree(disk->features); + disk->features = NULL; } void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) @@ -103,6 +106,38 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) } static int +virDomainSnapshotDiskFeaturesDefParse(virDomainSnapshotDiskDefPtr def, + xmlNodePtr node) +{ + xmlNodePtr cur; + int f, n = 0; + + if (!(def->features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) { + virReportOOMError(); + return -1; + } + + for (cur = node->children; cur; cur = cur->next) { + if (cur->type != XML_ELEMENT_NODE) + continue; + + f = virStorageFileFeatureTypeFromString((const char*) cur->name); + + if (f < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unsupported feature %s"), + (const char*) cur->name); + return -1; + } + ignore_value(virBitmapSetBit(def->features, f)); + n++; + + } + if (n > 0 && !def->compat && VIR_STRDUP(def->compat, "1.1") < 0) + return -1; + return 0; +} + +static int virDomainSnapshotDiskDefParseXML(xmlNodePtr node, virDomainSnapshotDiskDefPtr def) { @@ -146,6 +181,27 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, goto cleanup; } VIR_FREE(driver); + } else if (xmlStrEqual(cur->name, BAD_CAST "compat")) { + if (VIR_STRDUP(def->compat, (const char*)cur->content) < 0) + goto cleanup; + if (def->compat) { + char **version = virStringSplit(def->compat, ".", 2); + unsigned int result; + + if (!version || !version[1] || + virStrToLong_ui(version[0], NULL, 10, &result) < 0 || + virStrToLong_ui(version[1], NULL, 10, &result) < 0) { + virStringFreeList(version); + virReportError(VIR_ERR_XML_ERROR, "%s", + _("forbidden characters in 'compat'" + " attribute")); + goto cleanup; + } + virStringFreeList(version); + } + } else if (xmlStrEqual(cur->name, BAD_CAST "features")) { + if (virDomainSnapshotDiskFeaturesDefParse(def, cur) < 0) + goto cleanup; } } cur = cur->next; @@ -549,6 +605,29 @@ cleanup: } static void +virDomainSnapshotDiskFeaturesDefFormat(virBufferPtr buf, + virBitmapPtr features) +{ + int i; + + if (virBitmapIsAllClear(features)) { + virBufferAddLit(buf, "<features/>\n"); + return; + } + + virBufferAddLit(buf, "<features>\n"); + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + bool b; + + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) + virBufferAsprintf(buf, " <%s/>\n", + virStorageFileFeatureTypeToString(i)); + } + virBufferAddLit(buf, "</features>\n"); +} + +static void virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainSnapshotDiskDefPtr disk) { @@ -571,6 +650,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, "<driver type='%s'/>\n", virStorageFileFormatTypeToString(disk->format)); virBufferEscapeString(buf, "<source file='%s'/>\n", disk->file); + virBufferEscapeString(buf, "<compat>%s</compat>\n", disk->compat); + + if (disk->features) + virDomainSnapshotDiskFeaturesDefFormat(buf, disk->features); + virBufferAdjustIndent(buf, -6); virBufferAddLit(buf, " </disk>\n"); } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 6d625a7..2be5319 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -53,6 +53,8 @@ struct _virDomainSnapshotDiskDef { int snapshot; /* enum virDomainSnapshotLocation */ char *file; /* new source file when snapshot is external */ int format; /* enum virStorageFileFormat */ + char *compat; + virBitmapPtr features; }; /* Stores the complete snapshot metadata */ diff --git a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml index ee6b46a..79895d7 100644 --- a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml @@ -11,6 +11,11 @@ </disk> <disk name='hde' snapshot='external'> <source file='/path/to/new'/> + <driver type='qcow2'/> + <compat>1.1</compat> + <features> + <lazy_refcounts/> + </features> </disk> </disks> </domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index 5f42bf5..4c3f8ad 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -18,6 +18,10 @@ <disk name='hde' snapshot='external'> <driver type='qcow2'/> <source file='/path/to/new'/> + <compat>1.1</compat> + <features> + <lazy_refcounts/> + </features> </disk> <disk name='hdf' snapshot='external'> <driver type='qcow2'/> -- 1.8.1.5

On 06/19/2013 05:24 PM, Ján Tomko wrote:
Add <features> and <compat> elements to snapshot disk 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. ---
Some code duplication from the previous patches done here, but that's fine due to the nature of the snapshot design, I guess. What could be changed is the fact that (as per discussion with you), there should be check that these new things are not requested in an XML for snapshot which doesn't support them or we can't control them (internal or active). I can't also decide whether we want this support in the snapshot XML. It's written nowhere that those features will disappear if not added in the XML, but we can't control that for active or internal snapshots. The same applies when such features are requested and the snapshot is active or internal. The patch looks good, conditional ACK if the check is added and if Eric or Peter will say they want this to be supported in snapshots. If not, then we don't have to bother with such dual feature support and can remove this and the next patch from the series. Eric, Peter? What's your opinion on this? Thanks, Martin

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 | 88 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 78 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 9b738e0..58e0f51 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11011,6 +11011,60 @@ 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) { + for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { + ignore_value(virBitmapGetBit(features, i, &b)); + if (b) { + switch (i) { + case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS: + if (STREQ(compat, "0.10")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Feature %s not supported with compat" + " level %s"), + virStorageFileFeatureTypeToString(i), + compat); + goto error; + } + break; + case VIR_STORAGE_FILE_FEATURE_LAST: + ; + } + virBufferAsprintf(&buf, "%s,", + virStorageFileFeatureTypeToString(i)); + } + } + } + + virBufferTrim(&buf, ",", -1); + + if (virBufferError(&buf)) + goto no_memory; + + *opts = virBufferContentAndReset(&buf); + return 0; + +no_memory: + virReportOOMError(); +error: + virBufferFreeAndReset(&buf); + return -1; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, @@ -11026,6 +11080,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; @@ -11056,23 +11112,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

Il 19/06/2013 17:24, Ján Tomko ha scritto:
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 | 88 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 78 insertions(+), 15 deletions(-)
You can use 'mode': 'existing' to do this for live external snapshots. Basically run the inactive snapshot code, and then use QMP to switch to the new file. Paolo
participants (4)
-
John Ferlan
-
Ján Tomko
-
Martin Kletzander
-
Paolo Bonzini