On 06/10/2013 02:10 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. --- 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 */
There could be a constant for this, which will be increased in case of need and will appear on some more readable place.
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) +
I must admit I'm not very fond of the naming (qcow2v3), but since this is an implementation detail and makes sense, let's go with it.
#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},
Good you added the trailing zero, in case some format goes to 3 version numbers, compiler will shout and we won't get to segfauls'n'stuff. Took me a while to appreciate this ;) But... [...]
+/* 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 */
Did you mean virStorageFileFeature?
+static int qcow2CompatibleFeatureArray[] = {
s/int/const int/ maybe ? [...]
@@ -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",
Changing the 'expected version' to something like 'one of the expected versions' could make users less confused when going through the logs, but not required.
+ version, fileTypeInfo[format].versionNumbers[i]); + if (version == fileTypeInfo[format].versionNumbers[i]) + return true; + }
... the constant mentioned earlier could be checked upon in here as well and that would make the format specification 1) safer 2) shorter (not much, though). [...]
@@ -669,6 +713,42 @@ cleanup: }
+static int +qcow2GetFeatures(virBitmapPtr *features, + int format, + unsigned char *buf, + ssize_t len)
indentation [...] ACK with: - the constant used instead of magic '3' - const int - indentation Martin