[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block: nbd: Fix dirty bitmap context name
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH] block: nbd: Fix dirty bitmap context name |
Date: |
Thu, 19 Dec 2019 14:23:49 +0000 |
19.12.2019 17:04, Kevin Wolf wrote:
> Am 19.12.2019 um 14:41 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Ahh, I see, it's documented as
>>
>> +# @bitmap: Also export the dirty bitmap reachable from @device, so the
>> +# NBD client can use NBD_OPT_SET_META_CONTEXT with
>> +# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
>>
>> and it is logical to assume that export name (which is @name argument) is
>> mentioned. But we never mentioned it. This is just documented after
>> removed experimenatl command x-nbd-server-add-bitmap,
>>
>> look at
>>
>> commit 7dc570b3806e5b0a4c9219061556ed5a4a0de80c
>> Author: Eric Blake <address@hidden>
>> Date: Fri Jan 11 13:47:18 2019 -0600
>>
>> nbd: Remove x-nbd-server-add-bitmap
>>
>> ...
>>
>> -# @bitmap-export-name: How the bitmap will be seen by nbd clients
>> -# (default @bitmap)
>> -#
>> -# Note: the client must use NBD_OPT_SET_META_CONTEXT with a query of
>> -# "qemu:dirty-bitmap:NAME" (where NAME matches @bitmap-export-name) to
>> access
>> -# the exposed bitmap.
>>
>>
>> So, this "NAME" is saved and now looks incorrect. What should be fixed, is
>> Qapi
>> documentation.
>
> Hm, I don't know these interfaces very well, but from you explanation it
> looks to me as if having a bitmap name made sense with
> x-nbd-server-add-bitmap because it could be called more than once for
> exporting multiple bitmaps.
>
> But now, we have only nbd-server-add, which takes a single bitmap name.
> As we don't have to distinguish multiple bitmaps any more, wouldn't it
> make more sense to use "qemu:dirty-bitmap" without any other
> information? Both export name and bitmap name are already identified by
> the connection.
I think, it will a bit in conflict with already documented
* "qemu:dirty-bitmap:" - returns list of all available dirty-bitmap
metadata contexts.
So, if we want some "default dirty bitmap", we'd better use something different
from just dirty-bitmap. For example,
"qemu:default-dirty-bitmap"
> But if we have to have something there, using the bitmap name (which may
> or may not be the same as used in the image file) makes a little more
> sense because it makes the interface extensible for the case that we
> ever want to re-introduce an nbd-server-add-bitmap.
Agree
>
> (By the way, even if the patch were correct, it lacks a Signed-off-by.)
>
> Kevin
>
--
Best regards,
Vladimir
- [PATCH] block: nbd: Fix dirty bitmap context name, Nir Soffer, 2019/12/19
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Vladimir Sementsov-Ogievskiy, 2019/12/19
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Kevin Wolf, 2019/12/19
- Re: [PATCH] block: nbd: Fix dirty bitmap context name,
Vladimir Sementsov-Ogievskiy <=
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Nir Soffer, 2019/12/19
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Vladimir Sementsov-Ogievskiy, 2019/12/19
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Nir Soffer, 2019/12/19
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Vladimir Sementsov-Ogievskiy, 2019/12/20
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Peter Krempa, 2019/12/20
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Vladimir Sementsov-Ogievskiy, 2019/12/20
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Peter Krempa, 2019/12/20
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Vladimir Sementsov-Ogievskiy, 2019/12/20
- Re: [PATCH] block: nbd: Fix dirty bitmap context name, Nir Soffer, 2019/12/25
Re: [PATCH] block: nbd: Fix dirty bitmap context name, Nir Soffer, 2019/12/19