[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 3/5] block: qobject_is_equal() i
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 3/5] block: qobject_is_equal() in bdrv_reopen_prepare() |
Date: |
Mon, 3 Jul 2017 15:01:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 2017-07-03 14:51, Eric Blake wrote:
> On 07/03/2017 07:25 AM, Max Reitz wrote:
>> Currently, bdrv_reopen_prepare() assumes that all BDS options are
>> strings. However, this is not the case if the BDS has been created
>> through the json: pseudo-protocol or blockdev-add.
>>
>> Note that the user-invokable reopen command is an HMP command, so you
>
> s/invokable/invocable/
I was asking this myself when writing this commit message and actually
consulted Wiktionary: https://en.wiktionary.org/wiki/invokable
>> can only specify strings there. Therefore, specifying a non-string
>> option with the "same" value as it was when originally created will now
>> return an error because the values are supposedly similar (and there is
>> no way for the user to circumvent this but to just not specify the
>> option again -- however, this is still strictly better than just
>> crashing).
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> block.c | 31 ++++++++++++++++++-------------
>> 1 file changed, 18 insertions(+), 13 deletions(-)
>>
>
>> + /* TODO: When using -drive to specify blockdev options, all
>> values
>> + * will be strings; however, when using -blockdev, blockdev-add
>> or
>> + * filenames using the json:{} pseudo-protocol, they will be
>> + * correctly typed.
>> + * In contrast, reopening options are (currently) always strings
>> + * (because you can only specify them through qemu-io; all other
>> + * callers do not specify any options).
>> + * Therefore, when using anything other than -drive to create a
>> BDS,
>> + * this cannot detect non-string options as unchanged, because
>> + * qobject_is_equal() always returns false for objects of
>> different
>> + * type. In the future, this should be remedied by correctly
>> typing
>> + * all options. For now, this is not too big of an issue
>> because
>> + * the user simply can not specify options which cannot be
>> changed
>
> Seeing "can not" usually looks wrong for "cannot"; but here, it is
> grammatically correct. But better would be "the user can simply not
> specify" or "the user can simply avoid specifying options".
Awww, I deliberately designed the sentence so I could use "can not". :-)
(and even contrast it with the following "cannot"!)
>> + * anyway, so they will stay unchanged. */
>
> The grammar tweak is minor, so:
> Reviewed-by: Eric Blake <address@hidden>
Please let me keep it :-)
Max
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), (continued)
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Max Reitz, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Max Reitz, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Eric Blake, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Max Reitz, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Max Reitz, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Eric Blake, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Eric Blake, 2017/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal(), Max Reitz, 2017/07/05
[Qemu-block] [PATCH v3 3/5] block: qobject_is_equal() in bdrv_reopen_prepare(), Max Reitz, 2017/07/03
Re: [Qemu-block] [Qemu-devel] [PATCH v3 3/5] block: qobject_is_equal() in bdrv_reopen_prepare(), Markus Armbruster, 2017/07/05
[Qemu-block] [PATCH v3 4/5] iotests: Add test for non-string option reopening, Max Reitz, 2017/07/03
[Qemu-block] [PATCH v3 5/5] tests: Add check-qobject for equality tests, Max Reitz, 2017/07/03