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.
---
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);
