Re: [Qemu-block] [PATCH RFC 1/1] block: Handle NULL options correctly in raw_open

[Dong Jia Shi]

* Dong Jia Shi <address@hidden> [2017-03-08 17:31:05 +0800]:

> * Kevin Wolf <address@hidden> [2017-03-08 10:13:46 +0100]:
> 
> > Am 08.03.2017 um 03:15 hat Dong Jia Shi geschrieben:
> > > A normal call for raw_open should always pass in a non-NULL @options,
> > > but for some certain cases (e.g. trying to applying snapshot on a RBD
> > > image), they call raw_open with a NULL @options right after the calling
> > > for raw_close.
> > > 
> > > Let's take the NULL @options as a sign of trying to do raw_open again,
> > > and just simply return a success code.
> > > 
> > > Signed-off-by: Dong Jia Shi <address@hidden>
> > 
> > I think we rather need to fix bdrv_snapshot_goto() so that it doesn't
> > pass NULL, but the actual options that were given for the node (i.e.
> > bs->options).
> I've tried that before the current try. bs->options does not have the
> "file" key-value pair, so that leads to a fail too. Should we put "file"
> in to the options manually? I noticed that it was removed from
> bs->options during the calling of bdrv_open_inherit.
> 
Hi Kevin,

After thinking for quite some time, I still don't think we need to fix
the caller. The reason is that raw_close always does nothing, so no
matter what the caller passing in, raw_open should do nothing but just
return 0.

The following is another proposal. Looking forward for your comments.
Thanks,

diff --git a/block/raw-format.c b/block/raw-format.c
index 86fbc65..c309d4c 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -384,6 +384,11 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
     BDRVRawState *s = bs->opaque;
     int ret;
 
+    if (!bs->file) {
+        return 0;
+    }
+
+    assert(options != NULL);
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
                                false, errp);
     if (!bs->file) {

> > 
> > Kevin
> > 
> 
> -- 
> Dong Jia

-- 
Dong Jia