Patchwork [v8,5/6] Qemu: Framework for reopening images safely

login
register
mail settings
Submitter Supriya Kannery
Date Oct. 30, 2011, 10:35 a.m.
Message ID <20111030103509.31685.87434.sendpatchset@skannery.in.ibm.com>
Download mbox | patch
Permalink /patch/122595/
State New
Headers show

Comments

Supriya Kannery - Oct. 30, 2011, 10:35 a.m.
Struct BDRVReopenState along with three reopen related functions
introduced for handling reopen state of images safely. This can be
extended by each of the block drivers to reopen respective
image files.

Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
Kevin Wolf - Nov. 4, 2011, 10:05 a.m.
Am 30.10.2011 11:35, schrieb Supriya Kannery:
> Struct BDRVReopenState along with three reopen related functions
> introduced for handling reopen state of images safely. This can be
> extended by each of the block drivers to reopen respective
> image files.
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>

> Index: qemu/block_int.h
> ===================================================================
> --- qemu.orig/block_int.h
> +++ qemu/block_int.h
> @@ -55,6 +55,14 @@ struct BlockDriver {
>      int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
>      int (*bdrv_probe_device)(const char *filename);
>      int (*bdrv_open)(BlockDriverState *bs, int flags);
> +
> +    /* For handling image reopen for split or non-split files */
> +    int (*bdrv_reopen_prepare)(BlockDriverState *bs, BDRVReopenState **rs,
> +          int flags);
> +    void (*bdrv_reopen_commit)(BlockDriverState *bs, BDRVReopenState *rs,
> +          int flags);
> +    void (*bdrv_reopen_abort)(BlockDriverState *bs, BDRVReopenState *rs);
> +
>      int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, int flags);
>      int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
>                       uint8_t *buf, int nb_sectors);
> @@ -211,6 +219,14 @@ struct BlockDriverState {
>      void *private;
>  };
>  
> +struct BDRVReopenState {
> +    BlockDriverState *bs;
> +    int reopen_flags;
> +
> +    /* For raw-posix */
> +    int reopen_fd;
> +};

I think I commented the same on the previous version: BDRVReopenState
shouldn't contain any format specific fields. raw-posix must extend the
struct like this and use container_of() to get it from a BDRVReopenState
pointer:

struct BDRVRawReopenState {
    BDRVReopenState common;
    int reopen_fd;
};

Kevin
Supriya Kannery - Nov. 4, 2011, 11:10 a.m.
On 11/04/2011 03:35 PM, Kevin Wolf wrote:
> Am 30.10.2011 11:35, schrieb Supriya Kannery:
>>
>> +struct BDRVReopenState {
>> +    BlockDriverState *bs;
>> +    int reopen_flags;
>> +
>> +    /* For raw-posix */
>> +    int reopen_fd;
>> +};
>
> I think I commented the same on the previous version: BDRVReopenState
> shouldn't contain any format specific fields. raw-posix must extend the
> struct like this and use container_of() to get it from a BDRVReopenState
> pointer:
>
> struct BDRVRawReopenState {
>      BDRVReopenState common;
>      int reopen_fd;
> };
>

I don't recall this was suggested in prev version or may be I missed to 
notice..
ok, will have raw extending common BDRVReopenState struct.


> Kevin
>

Patch

Index: qemu/block.c
===================================================================
--- qemu.orig/block.c
+++ qemu/block.c
@@ -693,10 +693,32 @@  unlink_and_fail:
     return ret;
 }
 
+int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags)
+{
+     BlockDriver *drv = bs->drv;
+
+     return drv->bdrv_reopen_prepare(bs, prs, flags);
+}
+
+void bdrv_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs, int flags)
+{
+    BlockDriver *drv = bs->drv;
+
+    drv->bdrv_reopen_commit(bs, rs, flags);
+}
+
+void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BlockDriver *drv = bs->drv;
+
+    drv->bdrv_reopen_abort(bs, rs);
+}
+
 int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
 {
     BlockDriver *drv = bs->drv;
     int ret = 0, open_flags;
+    BDRVReopenState *rs = NULL;
 
     /* Quiesce IO for the given block device */
     qemu_aio_flush();
@@ -704,20 +726,35 @@  int bdrv_reopen(BlockDriverState *bs, in
         qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
         return ret;
     }
-    open_flags = bs->open_flags;
-    bdrv_close(bs);
 
-    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
-    if (ret < 0) {
-        /* Reopen failed. Try to open with original flags */
-        qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
-        ret = bdrv_open(bs, bs->filename, open_flags, drv);
+    /* Use driver specific reopen() if available */
+    if (drv->bdrv_reopen_prepare) {
+        ret = bdrv_reopen_prepare(bs, &rs, bdrv_flags);
         if (ret < 0) {
-            /* Reopen failed with orig and modified flags */
-            abort();
+            bdrv_reopen_abort(bs, rs);
+            qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+            return ret;
         }
-    }
+        bdrv_reopen_commit(bs, rs, bdrv_flags);
+
+    } else {
+        open_flags = bs->open_flags;
+        bdrv_close(bs);
 
+        ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
+        if (ret < 0) {
+            /* Reopen failed. Try to open with original flags */
+            qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+            ret = bdrv_open(bs, bs->filename, open_flags, drv);
+            if (ret < 0) {
+                /*
+                 * Reopen failed with orig and modified flags
+                 * For next access to fail, set drv to NULL
+                */
+                bs->drv = NULL;
+            }
+        }
+    }
     return ret;
 }
 
Index: qemu/block_int.h
===================================================================
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -55,6 +55,14 @@  struct BlockDriver {
     int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
     int (*bdrv_probe_device)(const char *filename);
     int (*bdrv_open)(BlockDriverState *bs, int flags);
+
+    /* For handling image reopen for split or non-split files */
+    int (*bdrv_reopen_prepare)(BlockDriverState *bs, BDRVReopenState **rs,
+          int flags);
+    void (*bdrv_reopen_commit)(BlockDriverState *bs, BDRVReopenState *rs,
+          int flags);
+    void (*bdrv_reopen_abort)(BlockDriverState *bs, BDRVReopenState *rs);
+
     int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, int flags);
     int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
                      uint8_t *buf, int nb_sectors);
@@ -211,6 +219,14 @@  struct BlockDriverState {
     void *private;
 };
 
+struct BDRVReopenState {
+    BlockDriverState *bs;
+    int reopen_flags;
+
+    /* For raw-posix */
+    int reopen_fd;
+};
+
 struct BlockDriverAIOCB {
     AIOPool *pool;
     BlockDriverState *bs;
Index: qemu/qemu-common.h
===================================================================
--- qemu.orig/qemu-common.h
+++ qemu/qemu-common.h
@@ -202,6 +202,7 @@  typedef struct NICInfo NICInfo;
 typedef struct HCIInfo HCIInfo;
 typedef struct AudioState AudioState;
 typedef struct BlockDriverState BlockDriverState;
+typedef struct BDRVReopenState BDRVReopenState;
 typedef struct DriveInfo DriveInfo;
 typedef struct DisplayState DisplayState;
 typedef struct DisplayChangeListener DisplayChangeListener;
Index: qemu/block.h
===================================================================
--- qemu.orig/block.h
+++ qemu/block.h
@@ -110,6 +110,9 @@  int bdrv_file_open(BlockDriverState **pb
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
               BlockDriver *drv);
 int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
+int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags);
+void bdrv_reopen_commit(BlockDriverState *bs,  BDRVReopenState *rs, int flags);
+void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs);
 void bdrv_close(BlockDriverState *bs);
 int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);