20.12.2019 13:40, Peter Krempa wrote: > On Fri, Dec 20, 2019 at 10:06:17 +0000, Vladimir Sementsov-Ogievskiy wrote: >> 20.12.2019 12:56, Peter Krempa wrote: >>> On Fri, Dec 20, 2019 at 09:39:17 +0000, Vladimir Sementsov-Ogievskiy wrote: >>>> 19.12.2019 18:55, Nir Soffer wrote: >>>>> On Thu, Dec 19, 2019 at 5:17 PM Vladimir Sementsov-Ogievskiy >>>>> <vsement...@virtuozzo.com> wrote: >>>>>> >>>>>> 19.12.2019 17:59, Nir Soffer wrote: >>>>>>> On Thu, Dec 19, 2019 at 4:04 PM Kevin Wolf <kw...@redhat.com> 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 <ebl...@redhat.com> >>>>>>>>> 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. >>>>>>> >>>>>>> We can use empty string (like the default export name), so the bitmap >>>>>>> would be exposed as: >>>>>>> >>>>>>> "qemu:dirty-bitmap:" >>>>>>> >>>>>>> This would solve the issue for users, and keep the API extensible. >>>>>> >>>>>> As I already said, we can not. If we really wont such thing, use another >>>>>> name, >>>>>> likq qemu:default-dirty-bitmap.. >>>>>> >>>>>> Or call bitmap export "default", to produce >>>>>> "qemu:dirty-bitmaps:default" >>>>>> >>>>>> But don't change default behavior of nbd-server-add >>>>>> >>>>>>> >>>>>>>> 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. >>>>>>> >>>>>>> But using the bitmap name means user of the NBD server need to know >>>>>>> this name. >>>>>> >>>>>> Why not? What is your case exactly? User knows, what kind of bitmap you >>>>>> are >>>>>> exporting, so user is in some relation with exporting process anyway. Why >>>>>> shouldn't it know the name? >>>>> >>>>> Because the user configuring qemu (libvirt) is not the same user >>>>> accessing qemu NBD >>>>> server (ovirt, or some backup application). >>>>> >>>>>> This name may be defined in you exporting protocol.. What are you >>>>>> exporting? >>>>> >>>>> We export HTTP API, allowing getting dirty extents and reading data: >>>>> https://www.ovirt.org/develop/release-management/features/storage/incremental-backup.html#map-request >>>>> (this document is outdated, but it gives the general idea) >>>>> >>>>> Or provide the NBD URL directly to user (future). >>>>> >>>>>> Note also, that client may use NBD_OPT_LIST_META_CONTEXT with query >>>>>> "qemu:dirty-bitmap:", to get list of all exported bitmaps. >>>>> >>>>> This is another option, I did not try to use this yet. We can use the >>>>> single >>>>> exported bitmap and fail if we get more than one. This is probably better >>>>> than changing the entire stack to support bitmap name. >>>>> >>>>>>> One option is that libvirt would publish the name of the bitmap in the >>>>>>> xml describing >>>>>>> the backup, and oVirt will have to propagate this name to the actual >>>>>>> program accessing >>>>>>> the NBD server, which may be a user program in the case when we expose >>>>>>> the NBD >>>>>>> URL to users (planned for future version). >>>>>>> >>>>>>> Another option is that the user will control this name, and libvirt >>>>>>> will use the name specified >>>>>>> by the user. This means oVirt will have to provide API to set this >>>>>>> name and pass it to libvirt. >>>>>>> >>>>>>> Both cases require lot of effort which does not help anyone in the >>>>>>> task of getting dirty >>>>>>> extents from an image - which is our current goal. We need to have >>>>>>> good defaults that >>>>>>> save unneeded effort in the entire stack. >>>>>> >>>>>> So, you implementing some protocol, and need to export only one bitmap >>>>>> for >>>>>> your specified scenario. Why not just give a constant name? Like >>>>>> ovirt-bitmap, >>>>>> or something like this? >>>>> >>>>> But we don't use qemu directly. We use libvirt, and libvirt does not >>>>> expose >>>>> the name of the bitmap, or let use control this name. >>>>> >>>>> This is a simplified flow: >>>>> 1. libvirt starts a backup, creating the "backup-exportname" bitmap >>>> >>>> But do you manage exportname, or not? >>> >>> They can't manage the export name either, but apparently the default >>> that libvirt uses suits them. >>> >>> I can add possibility to name the actual backup output bitmap >>> arbitrarily. Obviously the user then has to pass a sensible name of a >>> non existant bitmap to proceed. >>> >>> Making both configurable at the same time may be worth doing as it will >>> be basically the same code. >>> >> >> Intuitively, I think it's better leave bitmap name as is: libvirt manages >> bitmaps, so let it manage their names as it wants. And I don't think that >> Nir want to care about existing bitmaps. > > Actually for an incremental backup we are creating a new bitmap which is > used for merging all the appropriate bitmaps for the expected backup > range. This is done also when only one bitmap would be selected this way > as there's no point in optimizing this algorithm. > > This means we can name the new bitmap arbitrarily (if it doesn't clash > with existing bitmap) and since that name will be also exported it's the > same they request. >
Hm, clear. Then it should work too. -- Best regards, Vladimir
