[Qemu-devel] [PATCH 4/7] block/vpc: Improve vpc_open (optimisation and error handling)

[Stefan Weil]

* 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 <address@hidden>
---
 block/vpc.c |   46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

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