|
From: | supriya kannery |
Subject: | Re: [Qemu-devel] [v9 Patch 5/6]Qemu: Framework for reopening images safely |
Date: | Mon, 21 Nov 2011 17:43:46 +0530 |
User-agent: | Thunderbird 2.0.0.14 (X11/20080501) |
Stefan Hajnoczi wrote:
On Fri, Nov 11, 2011 at 6:48 AM, Supriya Kannery <address@hidden> wrote:@@ -708,17 +731,31 @@ 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) {This seems weird to me because we're saying a driver may have drv->bdrv_reopen_prepare == NULL but the public bdrv_reopen_prepare() function doesn't check and return -ENOTSUP.
If drv->bdrv_reopen_prepare == NULL , then we are not calling thepublick bdrv_reopen_prepare at all. Unless we later call public bdrv_reopen_prepare
from elsewhere without checking drv->bdrv_reopen_prepare, a check for -ENOTSUP inside the public one is not needed right? Also, we are handling reopening for even those drivers which don't have its own bdrv_reopen_prepare defined, by taking the "else" control path. So condition for reporting "ENOTSUP" shouldn't come up as of now. Please let me know your thoughts.
This check can be moved into bdrv_reopen_prepare(). We can test for the -ENOTSUP return value here instead.+ ret = bdrv_reopen_prepare(bs, &reopen_state, bdrv_flags); + if (ret < 0) {Indentation is off here.
sure..will take care in next version.
Stefan
[Prev in Thread] | Current Thread | [Next in Thread] |