From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58255) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv94I-0001kK-WD for qemu-devel@nongnu.org; Wed, 08 Feb 2012 10:04:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rv94C-00046L-V6 for qemu-devel@nongnu.org; Wed, 08 Feb 2012 10:04:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42848) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv94C-00046B-NH for qemu-devel@nongnu.org; Wed, 08 Feb 2012 10:04:16 -0500 Message-ID: <4F328FB8.7080403@redhat.com> Date: Wed, 08 Feb 2012 16:07:36 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <20120201030557.2990.74150.sendpatchset@skannery.in.ibm.com> <20120201030658.2990.15176.sendpatchset@skannery.in.ibm.com> In-Reply-To: <20120201030658.2990.15176.sendpatchset@skannery.in.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC Patch 4/7]Qemu: Framework for reopening image files safely List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Supriya Kannery Cc: Stefan Hajnoczi , Christoph Hellwig , qemu-devel@nongnu.org, Luiz Capitulino Am 01.02.2012 04:06, schrieb Supriya Kannery: > 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 > @@ -808,10 +808,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) > +{ > + 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); > +} > + > int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) > { > BlockDriver *drv = bs->drv; > int ret = 0, open_flags; > + BDRVReopenState *reopen_state = NULL; > > /* Quiesce IO for the given block device */ > qemu_aio_flush(); > @@ -820,17 +842,32 @@ 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, &reopen_state, bdrv_flags); > + if (ret < 0) { > + bdrv_reopen_abort(bs, reopen_state); > + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); > + return ret; > + } > + > + bdrv_reopen_commit(bs, reopen_state); > + bs->open_flags = 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 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 failed with orig and modified flags */ > + bs->drv = NULL; > + } > } Most image formats don't have a bdrv_reopen_* implementation after this series, so usually you'll have something like qcow2 on top of file. This code uses bdrv_close/open for the whole stack, even though the file layer could actually make use of a bdrv_reopen_* implementation and the qcow2 open isn't likely to fail if the image file could be opened. I think we can use drv->bdrv_close/open to reopen only one layer and try using bdrv_reopen_* for the lower layer again. This is an improvement that can be done in a separate patch, though. Kevin