10:11 a.m.
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