qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC] Safely reopening image files by stashing fds


From: Supriya Kannery
Subject: Re: [Qemu-devel] [RFC] Safely reopening image files by stashing fds
Date: Wed, 17 Aug 2011 00:48:55 +0530
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090513 Fedora/3.0-2.3.beta2.fc11 Thunderbird/3.0b2

On 08/09/2011 03:02 PM, supriya kannery wrote:
> Kevin Wolf wrote:
>> Am 09.08.2011 11:22, schrieb supriya kannery:
>>> Kevin Wolf wrote:
>>
>> What I meant is that in the end, with a generic bdrv_reopen(), we can
>> have raw-posix only call dup() and fcntl() instead of doing a
>> close()/open() sequence if it can satisfy the new flags this way. But
>> this would be an implementation detail and not be visible in the
>> interface.
>>
>> Kevin
>
> ok
> - thanks, Supriya
>

Though I started the RFC patch with defining BDRVReopenState, ended up
in enhancing struct BlockDriver with .bdrv_reopen. bdrv_reopen mplementation specific to respective driver is assigned to this
function pointer.

Please find the implementation of O_DIRECT flag change, done in
raw-posix.c. Similar implementation can be done for vmdk (with
bdrv_reopen_commit and bdrv_reopen_abort as internal functions in
vmdk.c for opening split files), sheepdog, nbd etc..

Request for comments on this approach.

Note: Following patch is for demonstrating the approach using
raw-posix  as sample driver. This patch is not complete.

- thanks, Supriya

---
 block.c           |   33 +++++++++++++++++++++++----------
 block/raw-posix.c |   34 ++++++++++++++++++++++++++++++++++
 block_int.h       |    1 +
 3 files changed, 58 insertions(+), 10 deletions(-)

Index: qemu/block/raw-posix.c
===================================================================
--- qemu.orig/block/raw-posix.c
+++ qemu/block/raw-posix.c
@@ -276,6 +276,39 @@ static int raw_open(BlockDriverState *bs
     return raw_open_common(bs, filename, flags, 0);
 }

+static int raw_reopen(BlockDriverState *bs, int flags)
+{
+    BDRVRawState *s = bs->opaque;
+    int new_fd;
+    int ret = 0;
+
+    /* Handle request for toggling O_DIRECT */
+    if ((bs->open_flags | BDRV_O_NOCACHE) ^
+        (flags | BDRV_O_NOCACHE))
+    {
+        if ((new_fd = dup(s->fd)) <=  0) {
+            return -1;
+        }
+
+        if ((ret = fcntl_setfl(new_fd, flags)) < 0) {
+            close(new_fd);
+            return ret;
+        }
+
+        /* Set new flags, so replace old fd with new one */
+        close(s->fd);
+        s->fd = new_fd;
+        s->open_flags = flags;
+        bs->open_flags = flags;
+    }
+
+    /*
+     * TBD: handle O_DSYNC and other flags for which image
+     * file has to be reopened
+     */
+    return 0;
+}
+
 /* XXX: use host sector size if necessary with:
 #ifdef DIOCGSECTORSIZE
         {
@@ -886,6 +919,7 @@ static BlockDriver bdrv_file = {
     .instance_size = sizeof(BDRVRawState),
     .bdrv_probe = NULL, /* no probe for protocols */
     .bdrv_file_open = raw_open,
+    .bdrv_reopen = raw_reopen,
     .bdrv_read = raw_read,
     .bdrv_write = raw_write,
     .bdrv_close = raw_close,
Index: qemu/block.c
===================================================================
--- qemu.orig/block.c
+++ qemu/block.c
@@ -687,20 +687,33 @@ 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) {
+        if ((ret = drv->bdrv_reopen(bs, bdrv_flags)) < 0) {
+            qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+            return ret;
+        }
+    } else {
+        /* Use reopen procedure common for drivers */
+        open_flags = bs->open_flags;
+        bdrv_close(bs);
+
+        ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
         if (ret < 0) {
-            /* Reopen failed with orig and modified flags */
-            abort();
+            /*
+             * 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 with orig and modified flags failed
+                 */
+                abort();
+            }
         }
     }
-
     return ret;
 }

Index: qemu/block_int.h
===================================================================
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -54,6 +54,7 @@ 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);
+    int (*bdrv_reopen)(BlockDriverState *bs, int flags);
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);



reply via email to

[Prev in Thread] Current Thread [Next in Thread]