qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [v9 Patch 5/6]Qemu: Framework for reopening images safe


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 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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]