From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41914) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sx22j-0004kk-Id for qemu-devel@nongnu.org; Thu, 02 Aug 2012 16:30:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sx22i-0001jx-Cc for qemu-devel@nongnu.org; Thu, 02 Aug 2012 16:30:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17241) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sx22i-0001jt-2k for qemu-devel@nongnu.org; Thu, 02 Aug 2012 16:30:48 -0400 Date: Thu, 2 Aug 2012 17:19:41 -0300 From: Luiz Capitulino Message-ID: <20120802171941.3c0027f1@doriath.home> In-Reply-To: <20120730213422.21536.81486.sendpatchset@skannery.in.ibm.com> References: <20120730213409.21536.7589.sendpatchset@skannery.in.ibm.com> <20120730213422.21536.81486.sendpatchset@skannery.in.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [v2 Patch 1/9]block: Framework for reopening image files safely List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Supriya Kannery Cc: Kevin Wolf , Shrinidhi Joshi , Stefan Hajnoczi , Jeff Cody , qemu-devel@nongnu.org, Christoph Hellwig On Tue, 31 Jul 2012 03:04:22 +0530 Supriya Kannery wrote: > Struct BDRVReopenState along with three reopen related functions > introduced for handling reopening of images safely. This can be > extended by each of the block drivers to reopen respective > image files. > > Signed-off-by: Supriya Kannery > > --- > Index: qemu/block.c > =================================================================== > --- qemu.orig/block.c > +++ qemu/block.c > @@ -859,6 +859,60 @@ 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) > +{ > + BlockDriver *drv = bs->drv; > + > + drv->bdrv_reopen_commit(bs, rs); > +} > + > +void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs) > +{ > + BlockDriver *drv = bs->drv; > + > + drv->bdrv_reopen_abort(bs, rs); > +} > + > +void bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp) > +{ > + BlockDriver *drv = bs->drv; > + int ret = 0; > + BDRVReopenState *reopen_state = NULL; > + > + /* Quiesce IO for the given block device */ > + bdrv_drain_all(); > + ret = bdrv_flush(bs); > + if (ret != 0) { > + error_set(errp, QERR_IO_ERROR); > + return; > + } > + > + /* Use driver specific reopen() if available */ > + if (drv->bdrv_reopen_prepare) { > + ret = bdrv_reopen_prepare(bs, &reopen_state, bdrv_flags); > + if (ret < 0) { > + bdrv_reopen_abort(bs, reopen_state); Why do you have to call bdrv_reopen_abort()? I'd expect bdrv_reopen_prepare() (to be able) to undo anything it has done. > + error_set(errp, QERR_OPEN_FILE_FAILED, bs->filename); > + return; > + } > + > + bdrv_reopen_commit(bs, reopen_state); > + bs->open_flags = bdrv_flags; > + } else { > + error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, > + drv->format_name, bs->device_name, > + "reopening of file"); > + return; > + } > +} > + > void bdrv_close(BlockDriverState *bs) > { > bdrv_flush(bs); > Index: qemu/block_int.h > =================================================================== > --- qemu.orig/block_int.h > +++ qemu/block_int.h > @@ -136,6 +136,14 @@ struct BlockDriver { > int instance_size; > int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename); > int (*bdrv_probe_device)(const char *filename); > + > + /* For handling image reopen for split or non-split files */ > + int (*bdrv_reopen_prepare)(BlockDriverState *bs, > + BDRVReopenState **prs, > + int flags); > + void (*bdrv_reopen_commit)(BlockDriverState *bs, BDRVReopenState *rs); > + void (*bdrv_reopen_abort)(BlockDriverState *bs, BDRVReopenState *rs); > + > int (*bdrv_open)(BlockDriverState *bs, int flags); > int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, int flags); > int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num, > @@ -335,6 +343,10 @@ struct BlockDriverState { > BlockJob *job; > }; > > +struct BDRVReopenState { > + BlockDriverState *bs; > +}; > + > int get_tmp_filename(char *filename, int size); > > void bdrv_set_io_limits(BlockDriverState *bs, > Index: qemu/block.h > =================================================================== > --- qemu.orig/block.h > +++ qemu/block.h > @@ -129,6 +129,10 @@ int bdrv_parse_cache_flags(const char *m > int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags); > int bdrv_open(BlockDriverState *bs, const char *filename, int flags, > BlockDriver *drv); > +void bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp); > +int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags); > +void bdrv_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs); > +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); > Index: qemu/qemu-common.h > =================================================================== > --- qemu.orig/qemu-common.h > +++ qemu/qemu-common.h > @@ -223,6 +223,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; >