[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image file
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely |
Date: |
Mon, 09 Jul 2012 17:06:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 15.06.2012 22:47, 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 <address@hidden>
>
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,10 +858,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();
> @@ -870,17 +892,44 @@ 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_OPEN_FILE_FAILED, bs->filename);
> - ret = bdrv_open(bs, bs->filename, open_flags, drv);
> - if (ret < 0) {
> - /* Reopen failed with orig and modified flags */
> - abort();
> + /* 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_OPEN_FILE_FAILED, bs->filename);
> + return ret;
> + }
> +
> + bdrv_reopen_commit(bs, reopen_state);
> + bs->open_flags = bdrv_flags;
> +
> + } else {
> +
> + /* Try reopening the image using protocol or directly */
> + if (bs->file) {
> + open_flags = bs->open_flags;
> + drv->bdrv_close(bs);
> + if (drv->bdrv_file_open) {
> + ret = drv->bdrv_file_open(bs, bs->filename, bdrv_flags);
> + } else {
> + ret = bdrv_file_open(&bs->file, bs->filename, bdrv_flags);
Doesn't this forget to reopen bs itself? And bs->file wasn't even
closed. If think we need something more along the lines of:
if (bs->file) {
bdrv_reopen(bs->file)
}
if (drv->bdrv_file_open) {
drv->bdrv_file_open(bs)
} else {
drv->bdrv_open(bs)
}
In fact we would really want to be able to commit/abort the bdrv_reopen
of bs->file only after we know if bdrv_open(bs) has succeeded, but with
the current design we can't because bdrv_open wants to read from the new fd.
Maybe it would make sense to require bdrv_reopen_prepare() to do the
switch without throwing the old state away yet, but it sounds as if it
would make implementations for image formats quite a bit harder.
Maybe we should completely avoid this default implementation and just
fail bdrv_reopen if a driver doesn't support the explicit,
transactionable reopen functions.
Kevin
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely,
Kevin Wolf <=