Stefan Hajnoczi wrote:
On Fri, Nov 11, 2011 at 6:48 AM, Supriya Kannery
<supri...@linux.vnet.ibm.com> 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 the
publick 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
