[libvirt] [PATCH v2 2/2] Add basic support for VDI images

[Martin Kletzander]

On 02/04/2013 11:06 PM, Eric Blake wrote:
> On 02/04/2013 02:49 PM, Martin Kletzander wrote:
>> QEMU is fully capable of handling VDI images and we just refuse to
>> work with them.  As qemu-img knows and supports this, there should be
>> no problem with this addition.
>>
>> This is of course, just basic functionality, without searching for any
>> backing files, etc.
>> ---
>>  src/util/virstoragefile.c | 15 +++++++++++----
>>  src/util/virstoragefile.h |  3 ++-
>>  2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> @@ -122,9 +122,10 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t);
>>  #define QED_F_BACKING_FILE 0x01
>>  #define QED_F_BACKING_FORMAT_NO_PROBE 0x04
>>
>> -/* VMDK needs at least this to find backing store,
>> +/* VMDK needs at least 20*512 B to find backing store,
>> + * ISO has 5 Byte magic on offset 32769,
>>   * other formats need less */
>> -#define STORAGE_MAX_HEAD (20*512)
>> +#define STORAGE_MAX_HEAD 32769+5
> 
> This hunk belongs in patch 1/2.
> 
>>
>>
>>  static struct FileTypeInfo const fileTypeInfo[] = {
>> @@ -164,7 +165,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
>>      },
>>      [VIR_STORAGE_FILE_ISO] = {
>>          32769, "CD001", ".iso",
>> -        0, -1, 0,
>> +        LV_LITTLE_ENDIAN, -2, 0,
>>          -1, 0, 0, -1, NULL
> 
> And this one, too.
> 
>>      },
>> +    /* TODO: add getBackingStore function */
>> +    [VIR_STORAGE_FILE_VDI] = {
>> +        64, "\x7f\x10\xda\xbe", ".vdi",
>> +        LV_LITTLE_ENDIAN, 68, 0x00010001,
>> +        68 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL},
>> +
> 
> Looks okay.
> 
>>      /* 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 },
>> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
>> index 618b028..83dc3f2 100644
>> --- a/src/util/virstoragefile.h
>> +++ b/src/util/virstoragefile.h
>> @@ -1,7 +1,7 @@
>>  /*
>>   * virstoragefile.h: file utility functions for FS storage backend
>>   *
>> - * Copyright (C) 2007-2009, 2012 Red Hat, Inc.
>> + * Copyright (C) 2007-2009, 2012, 2013 Red Hat, Inc.
> 
> I'd use 2012-2013, instead of comma notation.
> 

I haven't noticed that.  I guess 'copyright-update uses ranges only when
updating a range already.

> ACK with the reshuffle of iso hunks and copyright fix.
> 

Did all that and pushed both patches, thanks.

Martin