04.01.2019 2:21, Eric Blake wrote: > On 12/21/18 3:35 AM, John Snow wrote: >> The 'x' prefix was added because I was uncertain of the direction we'd >> take for the libvirt API. With the general approach solidified, I feel >> comfortable committing to this API for 4.0. >> >> Signed-off-by: John Snow <js...@redhat.com> >> Reviewed-by: Eric Blake <ebl...@redhat.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> blockdev.c | 22 +++++++++++----------- >> qapi/block-core.json | 34 +++++++++++++++++----------------- >> qapi/transaction.json | 12 ++++++------ >> tests/qemu-iotests/223 | 4 ++-- >> 4 files changed, 36 insertions(+), 36 deletions(-) > > Misses the conversion of x-nbd-server-add-bitmap in qapi/block.json > (could be a separate patch), but that's also one of the commands that > libvirt will need to use alongside all the bitmap changes. > > Speaking of x-nbd-server-add-bitmap, does it actually have to be a > separate command, or should it just be an additional parameter to the > existing nbd-server-add? There are two directions this could go: > > As a single command, it enforces that there is no window of time where > the export is available without the dirty bitmap. However, it also means > we can only export at most one bitmap, unless we make the parameter take > an array of bitmaps to export. Fewer commands also means less roll-back > scenarios in libvirt (no need to worry about the export being added but > the bitmap failing). > > As a separate command, we could in the future allow the server to export > multiple bitmaps at once by calling the add-bitmap command more than > once; but right now, the code base does not allow multiple bitmap > exports (a second bitmap export attempt fails). > > I'm leaning towards the former (drop x-nbd-server-add-bitmap altogether, > and instead add a bitmap parameter to nbd-server-add). Also, I'm > leaning towards locking in that we only ever export one > qemu:dirty-bitmap:FOO bitmap per export, and thus not worry about an > array. Besides, if we make the command take an array now but enforce > that the array has at most one element, then later relax it to take more > than one element, there is no way to introspect that change; but if we > make the command take a single string now, and later have a strong > reason why exporting more than one bitmap at once makes sense, we could > still maintain backcompat by using a QAPI alternate type between a > string and an array, which would be introspectible. > > I guess I should propose a patch for that, then... >
Reasonable, I've no objections. -- Best regards, Vladimir
