* [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
@ 2019-07-12 10:46 Stefano Garzarella
2019-07-12 18:35 ` Max Reitz
2019-07-13 12:24 ` no-reply
0 siblings, 2 replies; 8+ messages in thread
From: Stefano Garzarella @ 2019-07-12 10:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, integration, qemu-block, Max Reitz
When the backing_file is specified as a JSON object, the
qemu_gluster_reopen_prepare() fails with this message:
invalid URI json:{"server.0.host": ...}
In this case, we should call qemu_gluster_init() using the QDict
'state->options' that contains the parameters already parsed.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
block/gluster.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/block/gluster.c b/block/gluster.c
index 62f8ff2147..26971db1ea 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
gconf->has_debug = true;
gconf->logfile = g_strdup(s->logfile);
gconf->has_logfile = true;
- reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
+ /*
+ * If 'bs->filename' starts with "json:", then 'state->options' will
+ * contain the parameters already parsed.
+ */
+ if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
+ reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
+ errp);
+ } else {
+ reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
+ }
if (reop_s->glfs == NULL) {
ret = -errno;
goto exit;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-12 10:46 [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object Stefano Garzarella
@ 2019-07-12 18:35 ` Max Reitz
2019-07-15 8:16 ` Stefano Garzarella
2019-07-13 12:24 ` no-reply
1 sibling, 1 reply; 8+ messages in thread
From: Max Reitz @ 2019-07-12 18:35 UTC (permalink / raw)
To: Stefano Garzarella, qemu-devel; +Cc: Kevin Wolf, integration, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 1529 bytes --]
On 12.07.19 12:46, Stefano Garzarella wrote:
> When the backing_file is specified as a JSON object, the
> qemu_gluster_reopen_prepare() fails with this message:
> invalid URI json:{"server.0.host": ...}
>
> In this case, we should call qemu_gluster_init() using the QDict
> 'state->options' that contains the parameters already parsed.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> block/gluster.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 62f8ff2147..26971db1ea 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> gconf->has_debug = true;
> gconf->logfile = g_strdup(s->logfile);
> gconf->has_logfile = true;
> - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
> + /*
> + * If 'bs->filename' starts with "json:", then 'state->options' will
> + * contain the parameters already parsed.
> + */
> + if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
> + reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
> + errp);
> + } else {
> + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
> + }
Hmmm, aren’t they always in state->options?
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-12 10:46 [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object Stefano Garzarella
2019-07-12 18:35 ` Max Reitz
@ 2019-07-13 12:24 ` no-reply
1 sibling, 0 replies; 8+ messages in thread
From: no-reply @ 2019-07-13 12:24 UTC (permalink / raw)
To: sgarzare; +Cc: kwolf, integration, qemu-devel, qemu-block, mreitz
Patchew URL: https://patchew.org/QEMU/20190712104617.94707-1-sgarzare@redhat.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC audio/noaudio.o
CC audio/wavaudio.o
CC audio/mixeng.o
/tmp/qemu-test/src/block/gluster.c:938:20: error: address of array 'state->bs->filename' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
~~~~~~~~~~~^~~~~~~~ ~~
1 error generated.
The full log is available at
http://patchew.org/logs/20190712104617.94707-1-sgarzare@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-12 18:35 ` Max Reitz
@ 2019-07-15 8:16 ` Stefano Garzarella
2019-07-15 10:53 ` Max Reitz
0 siblings, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2019-07-15 8:16 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, integration, qemu-devel, qemu-block
On Fri, Jul 12, 2019 at 08:35:12PM +0200, Max Reitz wrote:
> On 12.07.19 12:46, Stefano Garzarella wrote:
> > When the backing_file is specified as a JSON object, the
> > qemu_gluster_reopen_prepare() fails with this message:
> > invalid URI json:{"server.0.host": ...}
> >
> > In this case, we should call qemu_gluster_init() using the QDict
> > 'state->options' that contains the parameters already parsed.
> >
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > block/gluster.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/gluster.c b/block/gluster.c
> > index 62f8ff2147..26971db1ea 100644
> > --- a/block/gluster.c
> > +++ b/block/gluster.c
> > @@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> > gconf->has_debug = true;
> > gconf->logfile = g_strdup(s->logfile);
> > gconf->has_logfile = true;
> > - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
> > + /*
> > + * If 'bs->filename' starts with "json:", then 'state->options' will
> > + * contain the parameters already parsed.
> > + */
> > + if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
> > + reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
> > + errp);
> > + } else {
> > + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
> > + }
>
> Hmmm, aren’t they always in state->options?
Yes, you are rigth, but the qemu_gluster_parse() doesn't search for the
'filename' in the QDict *options.
Maybe I can simply modify it in this way in order to hanlde this case,
calling qemu_gluster_init() only with 'state->options'.
diff --git a/block/gluster.c b/block/gluster.c
index 26971db1ea..91d674cd2b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -695,6 +695,11 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
QDict *options, Error **errp)
{
int ret;
+
+ if (!filename) {
+ filename = qdict_get_try_str(options, GLUSTER_OPT_FILENAME);
+ }
+
if (filename) {
ret = qemu_gluster_parse_uri(gconf, filename);
if (ret < 0) {
Do you think it is better/cleaner?
Thanks,
Stefano
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-15 8:16 ` Stefano Garzarella
@ 2019-07-15 10:53 ` Max Reitz
2019-07-15 12:50 ` Stefano Garzarella
0 siblings, 1 reply; 8+ messages in thread
From: Max Reitz @ 2019-07-15 10:53 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: Kevin Wolf, integration, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 2836 bytes --]
On 15.07.19 10:16, Stefano Garzarella wrote:
> On Fri, Jul 12, 2019 at 08:35:12PM +0200, Max Reitz wrote:
>> On 12.07.19 12:46, Stefano Garzarella wrote:
>>> When the backing_file is specified as a JSON object, the
>>> qemu_gluster_reopen_prepare() fails with this message:
>>> invalid URI json:{"server.0.host": ...}
>>>
>>> In this case, we should call qemu_gluster_init() using the QDict
>>> 'state->options' that contains the parameters already parsed.
>>>
>>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>> ---
>>> block/gluster.c | 11 ++++++++++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/gluster.c b/block/gluster.c
>>> index 62f8ff2147..26971db1ea 100644
>>> --- a/block/gluster.c
>>> +++ b/block/gluster.c
>>> @@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
>>> gconf->has_debug = true;
>>> gconf->logfile = g_strdup(s->logfile);
>>> gconf->has_logfile = true;
>>> - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
>>> + /*
>>> + * If 'bs->filename' starts with "json:", then 'state->options' will
>>> + * contain the parameters already parsed.
>>> + */
>>> + if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
>>> + reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
>>> + errp);
>>> + } else {
>>> + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
>>> + }
>>
>> Hmmm, aren’t they always in state->options?
>
> Yes, you are rigth, but the qemu_gluster_parse() doesn't search for the
> 'filename' in the QDict *options.
>
> Maybe I can simply modify it in this way in order to hanlde this case,
> calling qemu_gluster_init() only with 'state->options'.
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 26971db1ea..91d674cd2b 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -695,6 +695,11 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
> QDict *options, Error **errp)
> {
> int ret;
> +
> + if (!filename) {
> + filename = qdict_get_try_str(options, GLUSTER_OPT_FILENAME);
> + }
> +
> if (filename) {
> ret = qemu_gluster_parse_uri(gconf, filename);
> if (ret < 0) {
>
>
> Do you think it is better/cleaner?
No, because the rest of the function completely ignores @options if
@filename is set.
Hm. I can’t think of anything better than your original solution, then.
Except the “state->bs->filename” should probably be a
“state->bs->filename[0]” (as Patchew has pointed out).
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-15 10:53 ` Max Reitz
@ 2019-07-15 12:50 ` Stefano Garzarella
2019-07-15 13:00 ` Max Reitz
0 siblings, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2019-07-15 12:50 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, integration, qemu-devel, qemu-block
On Mon, Jul 15, 2019 at 12:53:57PM +0200, Max Reitz wrote:
> On 15.07.19 10:16, Stefano Garzarella wrote:
> > On Fri, Jul 12, 2019 at 08:35:12PM +0200, Max Reitz wrote:
> >> On 12.07.19 12:46, Stefano Garzarella wrote:
> >>> When the backing_file is specified as a JSON object, the
> >>> qemu_gluster_reopen_prepare() fails with this message:
> >>> invalid URI json:{"server.0.host": ...}
> >>>
> >>> In this case, we should call qemu_gluster_init() using the QDict
> >>> 'state->options' that contains the parameters already parsed.
> >>>
> >>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
> >>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >>> ---
> >>> block/gluster.c | 11 ++++++++++-
> >>> 1 file changed, 10 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/block/gluster.c b/block/gluster.c
> >>> index 62f8ff2147..26971db1ea 100644
> >>> --- a/block/gluster.c
> >>> +++ b/block/gluster.c
> >>> @@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> >>> gconf->has_debug = true;
> >>> gconf->logfile = g_strdup(s->logfile);
> >>> gconf->has_logfile = true;
> >>> - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
> >>> + /*
> >>> + * If 'bs->filename' starts with "json:", then 'state->options' will
> >>> + * contain the parameters already parsed.
> >>> + */
> >>> + if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
> >>> + reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
> >>> + errp);
> >>> + } else {
> >>> + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
> >>> + }
> >>
> >> Hmmm, aren’t they always in state->options?
> >
> > Yes, you are rigth, but the qemu_gluster_parse() doesn't search for the
> > 'filename' in the QDict *options.
> >
> > Maybe I can simply modify it in this way in order to hanlde this case,
> > calling qemu_gluster_init() only with 'state->options'.
> >
> > diff --git a/block/gluster.c b/block/gluster.c
> > index 26971db1ea..91d674cd2b 100644
> > --- a/block/gluster.c
> > +++ b/block/gluster.c
> > @@ -695,6 +695,11 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
> > QDict *options, Error **errp)
> > {
> > int ret;
> > +
> > + if (!filename) {
> > + filename = qdict_get_try_str(options, GLUSTER_OPT_FILENAME);
> > + }
> > +
> > if (filename) {
> > ret = qemu_gluster_parse_uri(gconf, filename);
> > if (ret < 0) {
> >
> >
> > Do you think it is better/cleaner?
>
> No, because the rest of the function completely ignores @options if
> @filename is set.
>
> Hm. I can’t think of anything better than your original solution, then.
> Except the “state->bs->filename” should probably be a
> “state->bs->filename[0]” (as Patchew has pointed out).
Sure, I'll fix it in the v2.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-15 12:50 ` Stefano Garzarella
@ 2019-07-15 13:00 ` Max Reitz
2019-07-15 13:25 ` Stefano Garzarella
0 siblings, 1 reply; 8+ messages in thread
From: Max Reitz @ 2019-07-15 13:00 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: Kevin Wolf, integration, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 3305 bytes --]
On 15.07.19 14:50, Stefano Garzarella wrote:
> On Mon, Jul 15, 2019 at 12:53:57PM +0200, Max Reitz wrote:
>> On 15.07.19 10:16, Stefano Garzarella wrote:
>>> On Fri, Jul 12, 2019 at 08:35:12PM +0200, Max Reitz wrote:
>>>> On 12.07.19 12:46, Stefano Garzarella wrote:
>>>>> When the backing_file is specified as a JSON object, the
>>>>> qemu_gluster_reopen_prepare() fails with this message:
>>>>> invalid URI json:{"server.0.host": ...}
>>>>>
>>>>> In this case, we should call qemu_gluster_init() using the QDict
>>>>> 'state->options' that contains the parameters already parsed.
>>>>>
>>>>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
>>>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>>>> ---
>>>>> block/gluster.c | 11 ++++++++++-
>>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/block/gluster.c b/block/gluster.c
>>>>> index 62f8ff2147..26971db1ea 100644
>>>>> --- a/block/gluster.c
>>>>> +++ b/block/gluster.c
>>>>> @@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
>>>>> gconf->has_debug = true;
>>>>> gconf->logfile = g_strdup(s->logfile);
>>>>> gconf->has_logfile = true;
>>>>> - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
>>>>> + /*
>>>>> + * If 'bs->filename' starts with "json:", then 'state->options' will
>>>>> + * contain the parameters already parsed.
>>>>> + */
>>>>> + if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
>>>>> + reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
>>>>> + errp);
>>>>> + } else {
>>>>> + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
>>>>> + }
>>>>
>>>> Hmmm, aren’t they always in state->options?
>>>
>>> Yes, you are rigth, but the qemu_gluster_parse() doesn't search for the
>>> 'filename' in the QDict *options.
>>>
>>> Maybe I can simply modify it in this way in order to hanlde this case,
>>> calling qemu_gluster_init() only with 'state->options'.
>>>
>>> diff --git a/block/gluster.c b/block/gluster.c
>>> index 26971db1ea..91d674cd2b 100644
>>> --- a/block/gluster.c
>>> +++ b/block/gluster.c
>>> @@ -695,6 +695,11 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
>>> QDict *options, Error **errp)
>>> {
>>> int ret;
>>> +
>>> + if (!filename) {
>>> + filename = qdict_get_try_str(options, GLUSTER_OPT_FILENAME);
>>> + }
>>> +
>>> if (filename) {
>>> ret = qemu_gluster_parse_uri(gconf, filename);
>>> if (ret < 0) {
>>>
>>>
>>> Do you think it is better/cleaner?
>>
>> No, because the rest of the function completely ignores @options if
>> @filename is set.
>>
>> Hm. I can’t think of anything better than your original solution, then.
>> Except the “state->bs->filename” should probably be a
>> “state->bs->filename[0]” (as Patchew has pointed out).
>
> Sure, I'll fix it in the v2.
Oh, wait. You can actually just use state->bs->exact_filename. That
will never be a json:{} filename, so you don’t have to do the prefix
check (just see whether it is empty).
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object
2019-07-15 13:00 ` Max Reitz
@ 2019-07-15 13:25 ` Stefano Garzarella
0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2019-07-15 13:25 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, integration, qemu-devel, qemu-block
On Mon, Jul 15, 2019 at 03:00:29PM +0200, Max Reitz wrote:
> On 15.07.19 14:50, Stefano Garzarella wrote:
> > On Mon, Jul 15, 2019 at 12:53:57PM +0200, Max Reitz wrote:
> >> On 15.07.19 10:16, Stefano Garzarella wrote:
> >>> On Fri, Jul 12, 2019 at 08:35:12PM +0200, Max Reitz wrote:
> >>>> On 12.07.19 12:46, Stefano Garzarella wrote:
> >>>>> When the backing_file is specified as a JSON object, the
> >>>>> qemu_gluster_reopen_prepare() fails with this message:
> >>>>> invalid URI json:{"server.0.host": ...}
> >>>>>
> >>>>> In this case, we should call qemu_gluster_init() using the QDict
> >>>>> 'state->options' that contains the parameters already parsed.
> >>>>>
> >>>>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1542445
> >>>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >>>>> ---
> >>>>> block/gluster.c | 11 ++++++++++-
> >>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/block/gluster.c b/block/gluster.c
> >>>>> index 62f8ff2147..26971db1ea 100644
> >>>>> --- a/block/gluster.c
> >>>>> +++ b/block/gluster.c
> >>>>> @@ -931,7 +931,16 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
> >>>>> gconf->has_debug = true;
> >>>>> gconf->logfile = g_strdup(s->logfile);
> >>>>> gconf->has_logfile = true;
> >>>>> - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
> >>>>> + /*
> >>>>> + * If 'bs->filename' starts with "json:", then 'state->options' will
> >>>>> + * contain the parameters already parsed.
> >>>>> + */
> >>>>> + if (state->bs->filename && !strstart(state->bs->filename, "json:", NULL)) {
> >>>>> + reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL,
> >>>>> + errp);
> >>>>> + } else {
> >>>>> + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp);
> >>>>> + }
> >>>>
> >>>> Hmmm, aren’t they always in state->options?
> >>>
> >>> Yes, you are rigth, but the qemu_gluster_parse() doesn't search for the
> >>> 'filename' in the QDict *options.
> >>>
> >>> Maybe I can simply modify it in this way in order to hanlde this case,
> >>> calling qemu_gluster_init() only with 'state->options'.
> >>>
> >>> diff --git a/block/gluster.c b/block/gluster.c
> >>> index 26971db1ea..91d674cd2b 100644
> >>> --- a/block/gluster.c
> >>> +++ b/block/gluster.c
> >>> @@ -695,6 +695,11 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
> >>> QDict *options, Error **errp)
> >>> {
> >>> int ret;
> >>> +
> >>> + if (!filename) {
> >>> + filename = qdict_get_try_str(options, GLUSTER_OPT_FILENAME);
> >>> + }
> >>> +
> >>> if (filename) {
> >>> ret = qemu_gluster_parse_uri(gconf, filename);
> >>> if (ret < 0) {
> >>>
> >>>
> >>> Do you think it is better/cleaner?
> >>
> >> No, because the rest of the function completely ignores @options if
> >> @filename is set.
> >>
> >> Hm. I can’t think of anything better than your original solution, then.
> >> Except the “state->bs->filename” should probably be a
> >> “state->bs->filename[0]” (as Patchew has pointed out).
> >
> > Sure, I'll fix it in the v2.
>
> Oh, wait. You can actually just use state->bs->exact_filename. That
> will never be a json:{} filename, so you don’t have to do the prefix
> check (just see whether it is empty).
Yeah, thanks for the suggestion!
Stefano
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-15 13:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-12 10:46 [Qemu-devel] [PATCH] gluster: fix .bdrv_reopen_prepare when backing file is a JSON object Stefano Garzarella
2019-07-12 18:35 ` Max Reitz
2019-07-15 8:16 ` Stefano Garzarella
2019-07-15 10:53 ` Max Reitz
2019-07-15 12:50 ` Stefano Garzarella
2019-07-15 13:00 ` Max Reitz
2019-07-15 13:25 ` Stefano Garzarella
2019-07-13 12:24 ` no-reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).