[Qemu-devel] [PATCH 0/7] block-vpc: Improve support for VHD images

[Qemu-devel] [PATCH 0/7] block-vpc: Improve support for VHD images

[Kevin Wolf]

This patch series improves the support for Virtual PC images. Until
now, support for this image format is read-only and broken for large
images. With these patches applied, you can create and write to VHD
Dynamic Disks and some bugs in the read support are fixed. They have
been tested with qemu-img convert from and to raw images and a
successful openSUSE installation on a fresh VHD image.

The most important use case is probably qemu-img which can be used to convert
from and to VHD now.

Kevin Wolf (7):
  block-vpc: Split up struct vpc_subheader
  block-vpc: Adapt header structures to official documentation
  block-vpc: Fix disk size
  block-vpc: Fix support for images > 4 GB
  block-vpc: Use the qemu block layer
  block-vpc: Write support
  block-vpc: Create images

 block-vpc.c |  512 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 435 insertions(+), 77 deletions(-)

[Qemu-devel] [PATCH 1/7] block-vpc: Split up struct vpc_subheader

[Kevin Wolf]

struct vpc_subheader currently is a union of two completely different
data structures (the Hard Disk Footer and the Dynamic Disk Header).
That doesn't make too much sense, so split them up.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |   85 ++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index f76c451..0b4666d 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -31,36 +31,31 @@
 //#define CACHE
 
 // always big-endian
-struct vpc_subheader {
-    char magic[8]; // "conectix" / "cxsparse"
-    union {
-	struct {
-	    uint32_t unk1[2];
-	    uint32_t unk2; // always zero?
-	    uint32_t subheader_offset;
-	    uint32_t unk3; // some size?
-	    char creator[4]; // "vpc "
-	    uint16_t major;
-	    uint16_t minor;
-	    char guest[4]; // "Wi2k"
-	    uint32_t unk4[7];
-	    uint8_t vnet_id[16]; // virtual network id, purpose unknown
-	    // next 16 longs are used, but dunno the purpose
-	    // next 6 longs unknown, following 7 long maybe a serial
-	    char padding[HEADER_SIZE - 84];
-	} main;
-	struct {
-	    uint32_t unk1[2]; // all bits set
-	    uint32_t unk2; // always zero?
-	    uint32_t pagetable_offset;
-	    uint32_t unk3;
-	    uint32_t pagetable_entries; // 32bit/entry
-	    uint32_t pageentry_size; // 512*8*512
-	    uint32_t nb_sectors;
-	    char padding[HEADER_SIZE - 40];
-	} sparse;
-	char padding[HEADER_SIZE - 8];
-    } type;
+struct vhd_footer {
+    char creator[8]; // "conectix
+    uint32_t unk1[2];
+    uint32_t unk2; // always zero?
+    uint32_t subheader_offset;
+    uint32_t unk3; // some size?
+    char creator_app[4]; // "vpc "
+    uint16_t major;
+    uint16_t minor;
+    char guest[4]; // "Wi2k"
+    uint32_t unk4[7];
+    uint8_t vnet_id[16]; // virtual network id, purpose unknown
+    // next 16 longs are used, but dunno the purpose
+    // next 6 longs unknown, following 7 long maybe a serial
+};
+
+struct vhd_dyndisk_header {
+    char magic[8]; // "cxsparse"
+    uint32_t unk1[2]; // all bits set
+    uint32_t unk2; // always zero?
+    uint32_t pagetable_offset;
+    uint32_t unk3;
+    uint32_t pagetable_entries; // 32bit/entry
+    uint32_t pageentry_size; // 512*8*512
+    uint32_t nb_sectors;
 };
 
 typedef struct BDRVVPCState {
@@ -90,7 +85,9 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVVPCState *s = bs->opaque;
     int fd, i;
-    struct vpc_subheader header;
+    struct vhd_footer* footer;
+    struct vhd_dyndisk_header* dyndisk_header;
+    uint8_t buf[HEADER_SIZE];
 
     fd = open(filename, O_RDONLY | O_BINARY);
     if (fd < 0)
@@ -100,25 +97,29 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
 
     s->fd = fd;
 
-    if (read(fd, &header, HEADER_SIZE) != HEADER_SIZE)
+    if (read(fd, buf, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
 
-    if (strncmp(header.magic, "conectix", 8))
+    footer = (struct vhd_footer*) buf;
+    if (strncmp(footer->creator, "conectix", 8))
         goto fail;
-    lseek(s->fd, be32_to_cpu(header.type.main.subheader_offset), SEEK_SET);
 
-    if (read(fd, &header, HEADER_SIZE) != HEADER_SIZE)
+    lseek(s->fd, be32_to_cpu(footer->subheader_offset), SEEK_SET);
+    if (read(fd, buf, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
 
-    if (strncmp(header.magic, "cxsparse", 8))
-	goto fail;
+    footer = NULL;
+    dyndisk_header = (struct vhd_dyndisk_header*) buf;
+
+    if (strncmp(dyndisk_header->magic, "cxsparse", 8))
+        goto fail;
 
-    bs->total_sectors = ((uint64_t)be32_to_cpu(header.type.sparse.pagetable_entries) *
-			be32_to_cpu(header.type.sparse.pageentry_size)) / 512;
+    bs->total_sectors = ((uint64_t)be32_to_cpu(dyndisk_header->pagetable_entries) *
+			be32_to_cpu(dyndisk_header->pageentry_size)) / 512;
 
-    lseek(s->fd, be32_to_cpu(header.type.sparse.pagetable_offset), SEEK_SET);
+    lseek(s->fd, be32_to_cpu(dyndisk_header->pagetable_offset), SEEK_SET);
 
-    s->pagetable_entries = be32_to_cpu(header.type.sparse.pagetable_entries);
+    s->pagetable_entries = be32_to_cpu(dyndisk_header->pagetable_entries);
     s->pagetable = qemu_malloc(s->pagetable_entries * 4);
     if (!s->pagetable)
 	goto fail;
@@ -128,7 +129,7 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     for (i = 0; i < s->pagetable_entries; i++)
 	be32_to_cpus(&s->pagetable[i]);
 
-    s->pageentry_size = be32_to_cpu(header.type.sparse.pageentry_size);
+    s->pageentry_size = be32_to_cpu(dyndisk_header->pageentry_size);
 #ifdef CACHE
     s->pageentry_u8 = qemu_malloc(512);
     if (!s->pageentry_u8)
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 1/7] block-vpc: Split up struct vpc_subheader

[Anthony Liguori]

Kevin Wolf wrote:
> struct vpc_subheader currently is a union of two completely different
> data structures (the Hard Disk Footer and the Dynamic Disk Header).
> That doesn't make too much sense, so split them up.
>
> Signed-off-by: Kevin Wolf <kwolf@suse.de>
>   

Applied all.  Thanks.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 2/7] block-vpc: Adapt header structures to official documentation

[Kevin Wolf]

The current definition of the VirtualPC headers is incomplete and partly
even wrong. This patch changes the header structs according to the
official VHD specification.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |  118 +++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 82 insertions(+), 36 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index 0b4666d..3f25785 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -30,41 +30,87 @@
 
 //#define CACHE
 
+enum vhd_type {
+    VHD_FIXED           = 2,
+    VHD_DYNAMIC         = 3,
+    VHD_DIFFERENCING    = 4,
+};
+
 // always big-endian
 struct vhd_footer {
-    char creator[8]; // "conectix
-    uint32_t unk1[2];
-    uint32_t unk2; // always zero?
-    uint32_t subheader_offset;
-    uint32_t unk3; // some size?
-    char creator_app[4]; // "vpc "
-    uint16_t major;
-    uint16_t minor;
-    char guest[4]; // "Wi2k"
-    uint32_t unk4[7];
-    uint8_t vnet_id[16]; // virtual network id, purpose unknown
-    // next 16 longs are used, but dunno the purpose
-    // next 6 longs unknown, following 7 long maybe a serial
+    char        creator[8]; // "conectix"
+    uint32_t    features;
+    uint32_t    version;
+
+    // Offset of next header structure, 0xFFFFFFFF if none
+    uint64_t    data_offset;
+
+    // Seconds since Jan 1, 2000 0:00:00 (UTC)
+    uint32_t    timestamp;
+
+    char        creator_app[4]; // "vpc "
+    uint16_t    major;
+    uint16_t    minor;
+    char        creator_os[4]; // "Wi2k"
+
+    uint64_t    orig_size;
+    uint64_t    size;
+
+    uint16_t    cyls;
+    uint8_t     heads;
+    uint8_t     secs_per_cyl;
+
+    uint32_t    type;
+
+    // Checksum of the Hard Disk Footer ("one's complement of the sum of all
+    // the bytes in the footer without the checksum field")
+    uint32_t    checksum;
+
+    // UUID used to identify a parent hard disk (backing file)
+    uint8_t     uuid[16];
+
+    uint8_t     in_saved_state;
 };
 
 struct vhd_dyndisk_header {
-    char magic[8]; // "cxsparse"
-    uint32_t unk1[2]; // all bits set
-    uint32_t unk2; // always zero?
-    uint32_t pagetable_offset;
-    uint32_t unk3;
-    uint32_t pagetable_entries; // 32bit/entry
-    uint32_t pageentry_size; // 512*8*512
-    uint32_t nb_sectors;
+    char        magic[8]; // "cxsparse"
+
+    // Offset of next header structure, 0xFFFFFFFF if none
+    uint64_t    data_offset;
+
+    // Offset of the Block Allocation Table (BAT)
+    uint64_t    table_offset;
+
+    uint32_t    version;
+    uint32_t    max_table_entries; // 32bit/entry
+
+    // 2 MB by default, must be a power of two
+    uint32_t    block_size;
+
+    uint32_t    checksum;
+    uint8_t     parent_uuid[16];
+    uint32_t    parent_timestamp;
+    uint32_t    reserved;
+
+    // Backing file name (in UTF-16)
+    uint8_t     parent_name[512];
+
+    struct {
+        uint32_t    platform;
+        uint32_t    data_space;
+        uint32_t    data_length;
+        uint32_t    reserved;
+        uint64_t    data_offset;
+    } parent_locator[8];
 };
 
 typedef struct BDRVVPCState {
     int fd;
 
-    int pagetable_entries;
+    int max_table_entries;
     uint32_t *pagetable;
 
-    uint32_t pageentry_size;
+    uint32_t block_size;
 #ifdef CACHE
     uint8_t *pageentry_u8;
     uint32_t *pageentry_u32;
@@ -104,7 +150,7 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     if (strncmp(footer->creator, "conectix", 8))
         goto fail;
 
-    lseek(s->fd, be32_to_cpu(footer->subheader_offset), SEEK_SET);
+    lseek(s->fd, be64_to_cpu(footer->data_offset), SEEK_SET);
     if (read(fd, buf, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
 
@@ -114,22 +160,22 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     if (strncmp(dyndisk_header->magic, "cxsparse", 8))
         goto fail;
 
-    bs->total_sectors = ((uint64_t)be32_to_cpu(dyndisk_header->pagetable_entries) *
-			be32_to_cpu(dyndisk_header->pageentry_size)) / 512;
+    bs->total_sectors = ((uint64_t)be32_to_cpu(dyndisk_header->max_table_entries) *
+			be32_to_cpu(dyndisk_header->block_size)) / 512;
 
-    lseek(s->fd, be32_to_cpu(dyndisk_header->pagetable_offset), SEEK_SET);
+    lseek(s->fd, be64_to_cpu(dyndisk_header->table_offset), SEEK_SET);
 
-    s->pagetable_entries = be32_to_cpu(dyndisk_header->pagetable_entries);
-    s->pagetable = qemu_malloc(s->pagetable_entries * 4);
+    s->max_table_entries = be32_to_cpu(dyndisk_header->max_table_entries);
+    s->pagetable = qemu_malloc(s->max_table_entries * 4);
     if (!s->pagetable)
 	goto fail;
-    if (read(s->fd, s->pagetable, s->pagetable_entries * 4) !=
-	s->pagetable_entries * 4)
+    if (read(s->fd, s->pagetable, s->max_table_entries * 4) !=
+	s->max_table_entries * 4)
 	goto fail;
-    for (i = 0; i < s->pagetable_entries; i++)
+    for (i = 0; i < s->max_table_entries; i++)
 	be32_to_cpus(&s->pagetable[i]);
 
-    s->pageentry_size = be32_to_cpu(dyndisk_header->pageentry_size);
+    s->block_size = be32_to_cpu(dyndisk_header->block_size);
 #ifdef CACHE
     s->pageentry_u8 = qemu_malloc(512);
     if (!s->pageentry_u8)
@@ -152,10 +198,10 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
     uint64_t bitmap_offset, block_offset;
     uint32_t pagetable_index, pageentry_index;
 
-    pagetable_index = offset / s->pageentry_size;
-    pageentry_index = (offset % s->pageentry_size) / 512;
+    pagetable_index = offset / s->block_size;
+    pageentry_index = (offset % s->block_size) / 512;
 
-    if (pagetable_index > s->pagetable_entries || s->pagetable[pagetable_index] == 0xffffffff)
+    if (pagetable_index > s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
 	return -1; // not allocated
 
     bitmap_offset = 512 * s->pagetable[pagetable_index];
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/7] block-vpc: Fix disk size

[Kevin Wolf]

VirtualPC bases the virtual disk size on the geometry rather than on
the size stored in the header.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index 3f25785..3c92bb3 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -150,6 +150,12 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     if (strncmp(footer->creator, "conectix", 8))
         goto fail;
 
+    // The visible size of a image in Virtual PC depends on the geometry
+    // rather than on the size stored in the footer (the size in the footer
+    // is too large usually)
+    bs->total_sectors = (int64_t)
+        be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
+
     lseek(s->fd, be64_to_cpu(footer->data_offset), SEEK_SET);
     if (read(fd, buf, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
@@ -160,9 +166,6 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     if (strncmp(dyndisk_header->magic, "cxsparse", 8))
         goto fail;
 
-    bs->total_sectors = ((uint64_t)be32_to_cpu(dyndisk_header->max_table_entries) *
-			be32_to_cpu(dyndisk_header->block_size)) / 512;
-
     lseek(s->fd, be64_to_cpu(dyndisk_header->table_offset), SEEK_SET);
 
     s->max_table_entries = be32_to_cpu(dyndisk_header->max_table_entries);
-- 
1.6.0.2

[Qemu-devel] [PATCH 4/7] block-vpc: Fix support for images > 4 GB

[Kevin Wolf]

This patch fixes the truncation of sector offsets to 32 bits.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index 3c92bb3..51644e6 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -207,7 +207,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
     if (pagetable_index > s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
 	return -1; // not allocated
 
-    bitmap_offset = 512 * s->pagetable[pagetable_index];
+    bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
     block_offset = bitmap_offset + 512 + (512 * pageentry_index);
 
 //    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 4/7] block-vpc: Fix support for images > 4 GB

[Kevin Wolf]

Kevin Wolf schrieb:
> This patch fixes the truncation of sector offsets to 32 bits.
> 
> Signed-off-by: Kevin Wolf <kwolf@suse.de>
> ---
>  block-vpc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/block-vpc.c b/block-vpc.c
> index 3c92bb3..51644e6 100644
> --- a/block-vpc.c
> +++ b/block-vpc.c
> @@ -207,7 +207,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
>      if (pagetable_index > s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
>  	return -1; // not allocated
>  
> -    bitmap_offset = 512 * s->pagetable[pagetable_index];
> +    bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
>      block_offset = bitmap_offset + 512 + (512 * pageentry_index);
>  
>  //    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",

Anthony, I think you missed this fix. I guess that's because the mail
was delivered to the list only some hours after I sent the series.
Anyway, it's still needed.

Kevin

Re: [Qemu-devel] [PATCH 4/7] block-vpc: Fix support for images > 4 GB

[Anthony Liguori]

Kevin Wolf wrote:
> Kevin Wolf schrieb:
>   
>> This patch fixes the truncation of sector offsets to 32 bits.
>>
>> Signed-off-by: Kevin Wolf <kwolf@suse.de>
>> ---
>>  block-vpc.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/block-vpc.c b/block-vpc.c
>> index 3c92bb3..51644e6 100644
>> --- a/block-vpc.c
>> +++ b/block-vpc.c
>> @@ -207,7 +207,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
>>      if (pagetable_index > s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
>>  	return -1; // not allocated
>>  
>> -    bitmap_offset = 512 * s->pagetable[pagetable_index];
>> +    bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
>>      block_offset = bitmap_offset + 512 + (512 * pageentry_index);
>>  
>>  //    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
>>     
>
> Anthony, I think you missed this fix. I guess that's because the mail
> was delivered to the list only some hours after I sent the series.
> Anyway, it's still needed.
>   

Indeed, thanks for catching that.

Regards,

Anthony Liguori

> Kevin
>   

Re: [Qemu-devel] [PATCH 4/7] block-vpc: Fix support for images > 4 GB

[Anthony Liguori]

Kevin Wolf wrote:
> This patch fixes the truncation of sector offsets to 32 bits.
>
> Signed-off-by: Kevin Wolf <kwolf@suse.de>
> ---
>  block-vpc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/block-vpc.c b/block-vpc.c
> index 3c92bb3..51644e6 100644
> --- a/block-vpc.c
> +++ b/block-vpc.c
> @@ -207,7 +207,7 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
>      if (pagetable_index > s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
>  	return -1; // not allocated
>  
> -    bitmap_offset = 512 * s->pagetable[pagetable_index];
> +    bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
>      block_offset = bitmap_offset + 512 + (512 * pageentry_index);
>  
>  //    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
>   

Applied.  Thanks.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 5/7] block-vpc: Use the qemu block layer

[Kevin Wolf]

Instead of accessing the file directly, use the qemu block layer.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |   59 ++++++++++++++++++++++++++++++++---------------------------
 1 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index 51644e6..c1d61de 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -105,7 +105,7 @@ struct vhd_dyndisk_header {
 };
 
 typedef struct BDRVVPCState {
-    int fd;
+    BlockDriverState *hd;
 
     int max_table_entries;
     uint32_t *pagetable;
@@ -130,20 +130,18 @@ static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
 static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVVPCState *s = bs->opaque;
-    int fd, i;
+    int ret, i;
     struct vhd_footer* footer;
     struct vhd_dyndisk_header* dyndisk_header;
     uint8_t buf[HEADER_SIZE];
 
-    fd = open(filename, O_RDONLY | O_BINARY);
-    if (fd < 0)
-        return -1;
-
     bs->read_only = 1; // no write support yet
 
-    s->fd = fd;
+    ret = bdrv_file_open(&s->hd, filename, flags);
+    if (ret < 0)
+        return ret;
 
-    if (read(fd, buf, HEADER_SIZE) != HEADER_SIZE)
+    if (bdrv_pread(s->hd, 0, buf, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
 
     footer = (struct vhd_footer*) buf;
@@ -156,8 +154,8 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     bs->total_sectors = (int64_t)
         be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
 
-    lseek(s->fd, be64_to_cpu(footer->data_offset), SEEK_SET);
-    if (read(fd, buf, HEADER_SIZE) != HEADER_SIZE)
+    if (bdrv_pread(s->hd, be64_to_cpu(footer->data_offset), buf, HEADER_SIZE)
+            != HEADER_SIZE)
         goto fail;
 
     footer = NULL;
@@ -166,15 +164,16 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     if (strncmp(dyndisk_header->magic, "cxsparse", 8))
         goto fail;
 
-    lseek(s->fd, be64_to_cpu(dyndisk_header->table_offset), SEEK_SET);
 
     s->max_table_entries = be32_to_cpu(dyndisk_header->max_table_entries);
     s->pagetable = qemu_malloc(s->max_table_entries * 4);
     if (!s->pagetable)
-	goto fail;
-    if (read(s->fd, s->pagetable, s->max_table_entries * 4) !=
-	s->max_table_entries * 4)
-	goto fail;
+        goto fail;
+
+    if (bdrv_pread(s->hd, be64_to_cpu(dyndisk_header->table_offset),
+            s->pagetable, s->max_table_entries * 4) != s->max_table_entries * 4)
+	    goto fail;
+
     for (i = 0; i < s->max_table_entries; i++)
 	be32_to_cpus(&s->pagetable[i]);
 
@@ -190,11 +189,15 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
 
     return 0;
  fail:
-    close(fd);
+    bdrv_delete(s->hd);
     return -1;
 }
 
-static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
+/*
+ * Returns the absolute byte offset of the given sector in the image file.
+ * If the sector is not allocated, -1 is returned instead.
+ */
+static inline int64_t get_sector_offset(BlockDriverState *bs, int64_t sector_num)
 {
     BDRVVPCState *s = bs->opaque;
     uint64_t offset = sector_num * 512;
@@ -241,9 +244,8 @@ static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num)
 	return -1; // not allocated
 #endif
 #endif
-    lseek(s->fd, block_offset, SEEK_SET);
 
-    return 0;
+    return block_offset;
 }
 
 static int vpc_read(BlockDriverState *bs, int64_t sector_num,
@@ -251,16 +253,19 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,
 {
     BDRVVPCState *s = bs->opaque;
     int ret;
+    int64_t offset;
 
     while (nb_sectors > 0) {
-	if (!seek_to_sector(bs, sector_num))
-	{
-	    ret = read(s->fd, buf, 512);
-	    if (ret != 512)
-		return -1;
-	}
-	else
+        offset = get_sector_offset(bs, sector_num);
+
+        if (offset == -1) {
             memset(buf, 0, 512);
+        } else {
+            ret = bdrv_pread(s->hd, offset, buf, 512);
+            if (ret != 512)
+                return -1;
+        }
+
         nb_sectors--;
         sector_num++;
         buf += 512;
@@ -275,7 +280,7 @@ static void vpc_close(BlockDriverState *bs)
 #ifdef CACHE
     qemu_free(s->pageentry_u8);
 #endif
-    close(s->fd);
+    bdrv_delete(s->hd);
 }
 
 BlockDriver bdrv_vpc = {
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/7] block-vpc: Write support

[Kevin Wolf]

Add write support for VHD images.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 150 insertions(+), 16 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index c1d61de..ecffb2d 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -2,6 +2,7 @@
  * Block driver for Conectix/Microsoft Virtual PC images
  *
  * Copyright (c) 2005 Alex Beregszaszi
+ * Copyright (c) 2009 Kevin Wolf <kwolf@suse.de>
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -107,10 +108,16 @@ struct vhd_dyndisk_header {
 typedef struct BDRVVPCState {
     BlockDriverState *hd;
 
+    uint8_t footer_buf[HEADER_SIZE];
+    uint64_t free_data_block_offset;
     int max_table_entries;
     uint32_t *pagetable;
+    uint64_t bat_offset;
+    uint64_t last_bitmap_offset;
 
     uint32_t block_size;
+    uint32_t bitmap_size;
+
 #ifdef CACHE
     uint8_t *pageentry_u8;
     uint32_t *pageentry_u32;
@@ -135,16 +142,14 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     struct vhd_dyndisk_header* dyndisk_header;
     uint8_t buf[HEADER_SIZE];
 
-    bs->read_only = 1; // no write support yet
-
     ret = bdrv_file_open(&s->hd, filename, flags);
     if (ret < 0)
         return ret;
 
-    if (bdrv_pread(s->hd, 0, buf, HEADER_SIZE) != HEADER_SIZE)
+    if (bdrv_pread(s->hd, 0, s->footer_buf, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
 
-    footer = (struct vhd_footer*) buf;
+    footer = (struct vhd_footer*) s->footer_buf;
     if (strncmp(footer->creator, "conectix", 8))
         goto fail;
 
@@ -158,26 +163,41 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
             != HEADER_SIZE)
         goto fail;
 
-    footer = NULL;
     dyndisk_header = (struct vhd_dyndisk_header*) buf;
 
     if (strncmp(dyndisk_header->magic, "cxsparse", 8))
         goto fail;
 
 
+    s->block_size = be32_to_cpu(dyndisk_header->block_size);
+    s->bitmap_size = ((s->block_size / (8 * 512)) + 511) & ~511;
+
     s->max_table_entries = be32_to_cpu(dyndisk_header->max_table_entries);
     s->pagetable = qemu_malloc(s->max_table_entries * 4);
     if (!s->pagetable)
         goto fail;
 
-    if (bdrv_pread(s->hd, be64_to_cpu(dyndisk_header->table_offset),
-            s->pagetable, s->max_table_entries * 4) != s->max_table_entries * 4)
+    s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
+    if (bdrv_pread(s->hd, s->bat_offset, s->pagetable,
+            s->max_table_entries * 4) != s->max_table_entries * 4)
 	    goto fail;
 
-    for (i = 0; i < s->max_table_entries; i++)
-	be32_to_cpus(&s->pagetable[i]);
+    s->free_data_block_offset =
+        (s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
+
+    for (i = 0; i < s->max_table_entries; i++) {
+        be32_to_cpus(&s->pagetable[i]);
+        if (s->pagetable[i] != 0xFFFFFFFF) {
+            int64_t next = (512 * (int64_t) s->pagetable[i]) +
+                s->bitmap_size + s->block_size;
+
+            if (next> s->free_data_block_offset)
+                s->free_data_block_offset = next;
+        }
+    }
+
+    s->last_bitmap_offset = (int64_t) -1;
 
-    s->block_size = be32_to_cpu(dyndisk_header->block_size);
 #ifdef CACHE
     s->pageentry_u8 = qemu_malloc(512);
     if (!s->pageentry_u8)
@@ -196,8 +216,12 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
 /*
  * Returns the absolute byte offset of the given sector in the image file.
  * If the sector is not allocated, -1 is returned instead.
+ *
+ * The parameter write must be 1 if the offset will be used for a write
+ * operation (the block bitmaps is updated then), 0 otherwise.
  */
-static inline int64_t get_sector_offset(BlockDriverState *bs, int64_t sector_num)
+static inline int64_t get_sector_offset(BlockDriverState *bs,
+    int64_t sector_num, int write)
 {
     BDRVVPCState *s = bs->opaque;
     uint64_t offset = sector_num * 512;
@@ -207,11 +231,24 @@ static inline int64_t get_sector_offset(BlockDriverState *bs, int64_t sector_num
     pagetable_index = offset / s->block_size;
     pageentry_index = (offset % s->block_size) / 512;
 
-    if (pagetable_index > s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
-	return -1; // not allocated
+    if (pagetable_index >= s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
+        return -1; // not allocated
 
     bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
-    block_offset = bitmap_offset + 512 + (512 * pageentry_index);
+    block_offset = bitmap_offset + s->bitmap_size + (512 * pageentry_index);
+
+    // We must ensure that we don't write to any sectors which are marked as
+    // unused in the bitmap. We get away with setting all bits in the block
+    // bitmap each time we write to a new block. This might cause Virtual PC to
+    // miss sparse read optimization, but it's not a problem in terms of
+    // correctness.
+    if (write && (s->last_bitmap_offset != bitmap_offset)) {
+        uint8_t bitmap[s->bitmap_size];
+
+        s->last_bitmap_offset = bitmap_offset;
+        memset(bitmap, 0xff, s->bitmap_size);
+        bdrv_pwrite(s->hd, bitmap_offset, bitmap, s->bitmap_size);
+    }
 
 //    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
 //	sector_num, pagetable_index, pageentry_index,
@@ -248,6 +285,75 @@ static inline int64_t get_sector_offset(BlockDriverState *bs, int64_t sector_num
     return block_offset;
 }
 
+/*
+ * Writes the footer to the end of the image file. This is needed when the
+ * file grows as it overwrites the old footer
+ *
+ * Returns 0 on success and < 0 on error
+ */
+static int rewrite_footer(BlockDriverState* bs)
+{
+    int ret;
+    BDRVVPCState *s = bs->opaque;
+    int64_t offset = s->free_data_block_offset;
+
+    ret = bdrv_pwrite(s->hd, offset, s->footer_buf, HEADER_SIZE);
+    if (ret < 0)
+        return ret;
+
+    return 0;
+}
+
+/*
+ * Allocates a new block. This involves writing a new footer and updating
+ * the Block Allocation Table to use the space at the old end of the image
+ * file (overwriting the old footer)
+ *
+ * Returns the sectors' offset in the image file on success and < 0 on error
+ */
+static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
+{
+    BDRVVPCState *s = bs->opaque;
+    int64_t bat_offset;
+    uint32_t index, bat_value;
+    int ret;
+    uint8_t bitmap[s->bitmap_size];
+
+    // Check if sector_num is valid
+    if ((sector_num < 0) || (sector_num > bs->total_sectors))
+        return -1;
+
+    // Write entry into in-memory BAT
+    index = (sector_num * 512) / s->block_size;
+    if (s->pagetable[index] != 0xFFFFFFFF)
+        return -1;
+
+    s->pagetable[index] = s->free_data_block_offset / 512;
+
+    // Initialize the block's bitmap
+    memset(bitmap, 0xff, s->bitmap_size);
+    bdrv_pwrite(s->hd, s->free_data_block_offset, bitmap, s->bitmap_size);
+
+    // Write new footer (the old one will be overwritten)
+    s->free_data_block_offset += s->block_size + s->bitmap_size;
+    ret = rewrite_footer(bs);
+    if (ret < 0)
+        goto fail;
+
+    // Write BAT entry to disk
+    bat_offset = s->bat_offset + (4 * index);
+    bat_value = be32_to_cpu(s->pagetable[index]);
+    ret = bdrv_pwrite(s->hd, bat_offset, &bat_value, 4);
+    if (ret < 0)
+        goto fail;
+
+    return get_sector_offset(bs, sector_num, 0);
+
+fail:
+    s->free_data_block_offset -= (s->block_size + s->bitmap_size);
+    return -1;
+}
+
 static int vpc_read(BlockDriverState *bs, int64_t sector_num,
                     uint8_t *buf, int nb_sectors)
 {
@@ -256,7 +362,7 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,
     int64_t offset;
 
     while (nb_sectors > 0) {
-        offset = get_sector_offset(bs, sector_num);
+        offset = get_sector_offset(bs, sector_num, 0);
 
         if (offset == -1) {
             memset(buf, 0, 512);
@@ -273,6 +379,34 @@ static int vpc_read(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
+static int vpc_write(BlockDriverState *bs, int64_t sector_num,
+    const uint8_t *buf, int nb_sectors)
+{
+    BDRVVPCState *s = bs->opaque;
+    int64_t offset;
+    int ret;
+
+    while (nb_sectors > 0) {
+        offset = get_sector_offset(bs, sector_num, 1);
+
+        if (offset == -1) {
+            offset = alloc_block(bs, sector_num);
+            if (offset < 0)
+                return -1;
+        }
+
+        ret = bdrv_pwrite(s->hd, offset, buf, 512);
+        if (ret != 512)
+            return -1;
+
+        nb_sectors--;
+        sector_num++;
+        buf += 512;
+    }
+
+    return 0;
+}
+
 static void vpc_close(BlockDriverState *bs)
 {
     BDRVVPCState *s = bs->opaque;
@@ -289,6 +423,6 @@ BlockDriver bdrv_vpc = {
     vpc_probe,
     vpc_open,
     vpc_read,
-    NULL,
+    vpc_write,
     vpc_close,
 };
-- 
1.6.0.2

[Qemu-devel] [PATCH 7/7] block-vpc: Create images

[Kevin Wolf]

Add an implementation to create VHD images.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 block-vpc.c |  169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 169 insertions(+), 0 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index ecffb2d..3eea506 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -37,6 +37,9 @@ enum vhd_type {
     VHD_DIFFERENCING    = 4,
 };
 
+// Seconds since Jan 1, 2000 0:00:00 (UTC)
+#define VHD_TIMESTAMP_BASE 946684800
+
 // always big-endian
 struct vhd_footer {
     char        creator[8]; // "conectix"
@@ -127,6 +130,18 @@ typedef struct BDRVVPCState {
 #endif
 } BDRVVPCState;
 
+static uint32_t vpc_checksum(uint8_t* buf, size_t size)
+{
+    uint32_t res = 0;
+    int i;
+
+    for (i = 0; i < size; i++)
+        res += buf[i];
+
+    return ~res;
+}
+
+
 static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
     if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
@@ -141,6 +156,7 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     struct vhd_footer* footer;
     struct vhd_dyndisk_header* dyndisk_header;
     uint8_t buf[HEADER_SIZE];
+    uint32_t checksum;
 
     ret = bdrv_file_open(&s->hd, filename, flags);
     if (ret < 0)
@@ -153,6 +169,12 @@ static int vpc_open(BlockDriverState *bs, const char *filename, int flags)
     if (strncmp(footer->creator, "conectix", 8))
         goto fail;
 
+    checksum = be32_to_cpu(footer->checksum);
+    footer->checksum = 0;
+    if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum)
+        fprintf(stderr, "block-vpc: The header checksum of '%s' is "
+            "incorrect.\n", filename);
+
     // The visible size of a image in Virtual PC depends on the geometry
     // rather than on the size stored in the footer (the size in the footer
     // is too large usually)
@@ -407,6 +429,152 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
+
+/*
+ * Calculates the number of cylinders, heads and sectors per cylinder
+ * based on a given number of sectors. This is the algorithm described
+ * in the VHD specification.
+ *
+ * Note that the geometry doesn't always exactly match total_sectors but
+ * may round it down.
+ */
+static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
+    uint8_t* heads, uint8_t* secs_per_cyl)
+{
+    uint32_t cyls_times_heads;
+
+    if (total_sectors > 65535 * 16 * 255)
+        total_sectors = 65535 * 16 * 255;
+
+    if (total_sectors > 65535 * 16 * 63) {
+        *secs_per_cyl = 255;
+        *heads = 16;
+        cyls_times_heads = total_sectors / *secs_per_cyl;
+    } else {
+        *secs_per_cyl = 17;
+        cyls_times_heads = total_sectors / *secs_per_cyl;
+        *heads = (cyls_times_heads + 1023) / 1024;
+
+        if (*heads < 4)
+            *heads = 4;
+
+        if (cyls_times_heads >= (*heads * 1024) || *heads > 16) {
+            *secs_per_cyl = 31;
+            *heads = 16;
+            cyls_times_heads = total_sectors / *secs_per_cyl;
+        }
+
+        if (cyls_times_heads >= (*heads * 1024)) {
+            *secs_per_cyl = 63;
+            *heads = 16;
+            cyls_times_heads = total_sectors / *secs_per_cyl;
+        }
+    }
+
+    // Note: Rounding up deviates from the Virtual PC behaviour
+    // However, we need this to avoid truncating images in qemu-img convert
+    *cyls = (cyls_times_heads + *heads - 1) / *heads;
+}
+
+static int vpc_create(const char *filename, int64_t total_sectors,
+    const char *backing_file, int flags)
+{
+    uint8_t buf[1024];
+    struct vhd_footer* footer = (struct vhd_footer*) buf;
+    struct vhd_dyndisk_header* dyndisk_header =
+        (struct vhd_dyndisk_header*) buf;
+    int fd, i;
+    uint16_t cyls;
+    uint8_t heads;
+    uint8_t secs_per_cyl;
+    size_t block_size, num_bat_entries;
+
+    if (backing_file != NULL)
+        return -ENOTSUP;
+
+    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
+    if (fd < 0)
+        return -EIO;
+
+    // Calculate matching total_size and geometry
+    calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl);
+    total_sectors = (int64_t) cyls * heads * secs_per_cyl;
+
+    // Prepare the Hard Disk Footer
+    memset(buf, 0, 1024);
+
+    strncpy(footer->creator, "conectix", 8);
+    // TODO Check if "qemu" creator_app is ok for VPC
+    strncpy(footer->creator_app, "qemu", 4);
+    strncpy(footer->creator_os, "Wi2k", 4);
+
+    footer->features = be32_to_cpu(0x02);
+    footer->version = be32_to_cpu(0x00010000);
+    footer->data_offset = be64_to_cpu(HEADER_SIZE);
+    footer->timestamp = be32_to_cpu(time(NULL) - VHD_TIMESTAMP_BASE);
+
+    // Version of Virtual PC 2007
+    footer->major = be16_to_cpu(0x0005);
+    footer->minor =be16_to_cpu(0x0003);
+
+    footer->orig_size = be64_to_cpu(total_sectors * 512);
+    footer->size = be64_to_cpu(total_sectors * 512);
+
+    footer->cyls = be16_to_cpu(cyls);
+    footer->heads = heads;
+    footer->secs_per_cyl = secs_per_cyl;
+
+    footer->type = be32_to_cpu(VHD_DYNAMIC);
+
+    // TODO uuid is missing
+
+    footer->checksum = be32_to_cpu(vpc_checksum(buf, HEADER_SIZE));
+
+    // Write the footer (twice: at the beginning and at the end)
+    block_size = 0x200000;
+    num_bat_entries = (total_sectors + block_size / 512) / (block_size / 512);
+
+    if (write(fd, buf, HEADER_SIZE) != HEADER_SIZE)
+        return -EIO;
+
+    if (lseek(fd, 1536 + ((num_bat_entries * 4 + 511) & ~511), SEEK_SET) < 0)
+        return -EIO;
+    if (write(fd, buf, HEADER_SIZE) != HEADER_SIZE)
+        return -EIO;
+
+    // Write the initial BAT
+    if (lseek(fd, 3 * 512, SEEK_SET) < 0)
+        return -EIO;
+
+    memset(buf, 0xFF, 512);
+    for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++)
+        if (write(fd, buf, 512) != 512)
+            return -EIO;
+
+
+    // Prepare the Dynamic Disk Header
+    memset(buf, 0, 1024);
+
+    strncpy(dyndisk_header->magic, "cxsparse", 8);
+
+    dyndisk_header->data_offset = be64_to_cpu(0xFFFFFFFF);
+    dyndisk_header->table_offset = be64_to_cpu(3 * 512);
+    dyndisk_header->version = be32_to_cpu(0x00010000);
+    dyndisk_header->block_size = be32_to_cpu(block_size);
+    dyndisk_header->max_table_entries = be32_to_cpu(num_bat_entries);
+
+    dyndisk_header->checksum = be32_to_cpu(vpc_checksum(buf, 1024));
+
+    // Write the header
+    if (lseek(fd, 512, SEEK_SET) < 0)
+        return -EIO;
+    if (write(fd, buf, 1024) != 1024)
+        return -EIO;
+
+    close(fd);
+    return 0;
+}
+
 static void vpc_close(BlockDriverState *bs)
 {
     BDRVVPCState *s = bs->opaque;
@@ -425,4 +593,5 @@ BlockDriver bdrv_vpc = {
     vpc_read,
     vpc_write,
     vpc_close,
+    vpc_create,
 };
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 0/7] block-vpc: Improve support for VHD images

[Jamie Lokier]

Kevin Wolf wrote:
> This patch series improves the support for Virtual PC images. Until
> now, support for this image format is read-only and broken for large
> images. With these patches applied, you can create and write to VHD
> Dynamic Disks and some bugs in the read support are fixed. They have
> been tested with qemu-img convert from and to raw images and a
> successful openSUSE installation on a fresh VHD image.

Ooh, I look forward to trying these patches. :-)

I have some VHD image of Windows which always resulted in lots of
scandisk errors when Windows booted from it (using -snapshot), and the
same when converted to a QCOW2 file by qemu-img.

The only thing that worked was using MS VirtualPC's convert-to-raw
facility from the disk image wized, then I could use that raw file
with QEMU.  That proved qemu-img was getting it wrong.

(By the way, VPC took *ridiculously* long - about 8 hours for an 80GB
sparse image.  VirtualPC appears to use unbuffered writes (equivalent
to O_DIRECT) at least when over SMB, but they are small writes and not
overlapped, so it trickles along very slowly).

So I look forward to trying these patches on that image, which I don't
use know but kept for this very purpose, and seeing if it's fixed the
errors.

-- Jamie

Re: [Qemu-devel] [PATCH 0/7] block-vpc: Improve support for VHD images

[Kevin Wolf]

Jamie Lokier schrieb:
> Kevin Wolf wrote:
>> This patch series improves the support for Virtual PC images. Until
>> now, support for this image format is read-only and broken for large
>> images. With these patches applied, you can create and write to VHD
>> Dynamic Disks and some bugs in the read support are fixed. They have
>> been tested with qemu-img convert from and to raw images and a
>> successful openSUSE installation on a fresh VHD image.
> 
> Ooh, I look forward to trying these patches. :-)
> 
> I have some VHD image of Windows which always resulted in lots of
> scandisk errors when Windows booted from it (using -snapshot), and the
> same when converted to a QCOW2 file by qemu-img.

I think there is only one integrity patch in the series which fixes
images > 4 GB. Is this the case with your image?

> (By the way, VPC took *ridiculously* long - about 8 hours for an 80GB
> sparse image.  VirtualPC appears to use unbuffered writes (equivalent
> to O_DIRECT) at least when over SMB, but they are small writes and not
> overlapped, so it trickles along very slowly).

Well, my implementation probably isn't that fast as well. It's the
simplest one I could think of and it's mostly to have at least something
to convert images. I don't think I would run a VM on it except for testing.

> So I look forward to trying these patches on that image, which I don't
> use know but kept for this very purpose, and seeing if it's fixed the
> errors.

Thanks for testing and please report back. I hope it works fine now.

Kevin