Skip to contentSkip to conversation

block: clean up bdrv_open2 structure a bit

PATCH Applied Christoph Hellwig 2 participants 2 messages Jan 20, 2010 – Jan 26, 2010

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 block: clean up bdrv_open2 structure a bit

Check the whitelist as early as possible instead of continuing the setup, and move all the error handling code to the end of the function.

block.c +19 -12
Index: qemu/block.c
===================================================================
@@ -428,10 +428,16 @@ int bdrv_open2(BlockDriverState *bs, con
             drv = find_image_format(filename);
         }
     }
+
     if (!drv) {
         ret = -ENOENT;
         goto unlink_and_fail;
     }
+    if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
+        ret = -ENOTSUP;
+        goto unlink_and_fail;
+    }
+
     bs->drv = drv;
     bs->opaque = qemu_mallocz(drv->instance_size);
 
@@ -453,20 +459,12 @@ int bdrv_open2(BlockDriverState *bs, con
     } else {
         open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
     }
-    if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
-        ret = -ENOTSUP;
-    } else {
-        ret = drv->bdrv_open(bs, filename, open_flags);
-    }
+
+    ret = drv->bdrv_open(bs, filename, open_flags);
     if (ret < 0) {
-        qemu_free(bs->opaque);
-        bs->opaque = NULL;
-        bs->drv = NULL;
-    unlink_and_fail:
-        if (bs->is_temporary)
-            unlink(filename);
-        return ret;
+        goto free_and_fail;
     }
+
     if (drv->bdrv_getlength) {
         bs->total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
     }
@@ -499,6 +497,15 @@ int bdrv_open2(BlockDriverState *bs, con
             bs->change_cb(bs->change_opaque);
     }
     return 0;
+
+free_and_fail:
+    qemu_free(bs->opaque);
+    bs->opaque = NULL;
+    bs->drv = NULL;
+unlink_and_fail:
+    if (bs->is_temporary)
+        unlink(filename);
+    return ret;
 }
 
 void bdrv_close(BlockDriverState *bs)
#1 lore raw
Quoted 6 lines · Christoph Hellwig

Check the whitelist as early as possible instead of continuing the setup, and move all the error handling code to the end of the function.

Applied all. Thanks.

Regards,

Anthony Liguori

Quoted 66 lines

Index: qemu/block.c
===================================================================

block.c +19 -12
@@ -428,10 +428,16 @@ int bdrv_open2(BlockDriverState *bs, con
              drv = find_image_format(filename);
          }
      }
+
      if (!drv) {
          ret = -ENOENT;
          goto unlink_and_fail;
      }
+    if (use_bdrv_whitelist&&  !bdrv_is_whitelisted(drv)) {
+        ret = -ENOTSUP;
+        goto unlink_and_fail;
+    }
+
      bs->drv = drv;
      bs->opaque = qemu_mallocz(drv->instance_size);
 
@@ -453,20 +459,12 @@ int bdrv_open2(BlockDriverState *bs, con
      } else {
          open_flags = flags&  ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
      }
-    if (use_bdrv_whitelist&&  !bdrv_is_whitelisted(drv)) {
-        ret = -ENOTSUP;
-    } else {
-        ret = drv->bdrv_open(bs, filename, open_flags);
-    }
+
+    ret = drv->bdrv_open(bs, filename, open_flags);
      if (ret<  0) {
-        qemu_free(bs->opaque);
-        bs->opaque = NULL;
-        bs->drv = NULL;
-    unlink_and_fail:
-        if (bs->is_temporary)
-            unlink(filename);
-        return ret;
+        goto free_and_fail;
      }
+
      if (drv->bdrv_getlength) {
          bs->total_sectors = bdrv_getlength(bs)>>  BDRV_SECTOR_BITS;
      }
@@ -499,6 +497,15 @@ int bdrv_open2(BlockDriverState *bs, con
              bs->change_cb(bs->change_opaque);
      }
      return 0;
+
+free_and_fail:
+    qemu_free(bs->opaque);
+    bs->opaque = NULL;
+    bs->drv = NULL;
+unlink_and_fail:
+    if (bs->is_temporary)
+        unlink(filename);
+    return ret;
  }
 
  void bdrv_close(BlockDriverState *bs)