On Fri, Feb 01, 2013 at 10:51:31PM +0100, Stefan Weil wrote:
> * Always read the footer at the end of the image.
> The footer exists for all kinds of VHD images, so there is no need
> to read its copy at the start of dynamic VHD images.
>
> * Return error codes from bdrv_pread like in other block drivers.
>
> * Return -EMEDIUMTYPE for wrong medium (missing signature).
>
> * It's not necessary to return 0 on success, >= 0 is sufficient.
>
> Signed-off-by: Stefan Weil <s...@weilnetz.de>
> ---
> block/vpc.c | 46 ++++++++++++++++++++++------------------------
> 1 file changed, 22 insertions(+), 24 deletions(-)
>
Hi Stefan,
I was going to test against some VHD images I have, as well as run
qemu-io tests, but this patch series has conflicts with the current
master. Patches 1 & 3 have minor conflicts with commit 59294e4, but
this patch has pretty significant conflicts, so I didn't bother
resolving it to test.
Could you rebase, and then repost?
Thanks,
Jeff
> diff --git a/block/vpc.c b/block/vpc.c
> index bcc2ace..fff103b 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -164,34 +164,30 @@ static int vpc_open(BlockDriverState *bs, int flags)
> {
> BDRVVPCState *s = bs->opaque;
> int i;
> - struct vhd_footer *footer;
> + struct vhd_footer *footer = (struct vhd_footer *)s->footer_buf;
> struct vhd_dyndisk_header *dyndisk_header;
> uint8_t buf[FOOTER_SIZE];
> uint32_t checksum;
> - int err = -1;
> + int ret;
> int disk_type = VHD_DYNAMIC;
> + int64_t offset = bdrv_getlength(bs->file);
>
> - if (bdrv_pread(bs->file, 0, s->footer_buf, FOOTER_SIZE) != FOOTER_SIZE) {
> + if (offset < FOOTER_SIZE) {
> + ret = -EINVAL;
> goto fail;
> }
>
> - footer = (struct vhd_footer *)s->footer_buf;
> + /* The footer is always found at the end of the file. */
> + ret = bdrv_pread(bs->file, offset-FOOTER_SIZE, s->footer_buf,
> FOOTER_SIZE);
> + if (ret < 0) {
> + goto fail;
> + }
> if (strncmp(footer->creator, "conectix", 8)) {
> - int64_t offset = bdrv_getlength(bs->file);
> - if (offset < FOOTER_SIZE) {
> - goto fail;
> - }
> - /* If a fixed disk, the footer is found only at the end of the file
> */
> - if (bdrv_pread(bs->file, offset-FOOTER_SIZE, s->footer_buf,
> FOOTER_SIZE)
> - != FOOTER_SIZE) {
> - goto fail;
> - }
> - if (strncmp(footer->creator, "conectix", 8)) {
> - goto fail;
> - }
> - disk_type = VHD_FIXED;
> + ret = -EMEDIUMTYPE;
> + goto fail;
> }
>
> + disk_type = be32_to_cpu(footer->type);
> checksum = be32_to_cpu(footer->checksum);
> footer->checksum = 0;
> if (vpc_checksum(s->footer_buf, FOOTER_SIZE) != checksum) {
> @@ -210,19 +206,21 @@ static int vpc_open(BlockDriverState *bs, int flags)
>
> /* Allow a maximum disk size of approximately 2 TB */
> if (bs->total_sectors >= 65535LL * 255 * 255) {
> - err = -EFBIG;
> + ret = -EFBIG;
> goto fail;
> }
>
> if (disk_type == VHD_DYNAMIC) {
> - if (bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
> - FOOTER_SIZE) != FOOTER_SIZE) {
> + ret = bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
> + FOOTER_SIZE);
> + if (ret < 0) {
> goto fail;
> }
>
> dyndisk_header = (struct vhd_dyndisk_header *) buf;
>
> if (strncmp(dyndisk_header->magic, "cxsparse", 8)) {
> + ret = -EINVAL;
> goto fail;
> }
>
> @@ -233,8 +231,9 @@ static int vpc_open(BlockDriverState *bs, int flags)
> s->pagetable = g_malloc(s->max_table_entries * 4);
>
> s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
> - if (bdrv_pread(bs->file, s->bat_offset, s->pagetable,
> - s->max_table_entries * 4) != s->max_table_entries * 4) {
> + ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
> + s->max_table_entries * 4);
> + if (ret < 0) {
> goto fail;
> }
>
> @@ -271,9 +270,8 @@ static int vpc_open(BlockDriverState *bs, int flags)
> "vpc", bs->device_name, "live migration");
> migrate_add_blocker(s->migration_blocker);
>
> - return 0;
> fail:
> - return err;
> + return ret;
> }
>
> static int vpc_reopen_prepare(BDRVReopenState *state,
> --
> 1.7.10.4
>
