[PATCH] qapi: deprecate drive-backup

[Vladimir Sementsov-Ogievskiy]

23.04.2021 16:09, Daniel P. Berrangé wrote:
> On Fri, Apr 23, 2021 at 03:59:00PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Modern way is using blockdev-add + blockdev-backup, which provides a
>> lot more control on how target is opened.
>>
>> As example of drive-backup problems consider the following:
>>
>> User of drive-backup expects that target will be opened in the same
>> cache and aio mode as source. Corresponding logic is in
>> drive_backup_prepare(), where we take bs->open_flags of source.
>>
>> It works rather bad if source was added by blockdev-add. Assume source
>> is qcow2 image. On blockdev-add we should specify aio and cache options
>> for file child of qcow2 node. What happens next:
>>
>> drive_backup_prepare() looks at bs->open_flags of qcow2 source node.
>> But there no BDRV_O_NOCAHE neither BDRV_O_NATIVE_AIO: BDRV_O_NOCAHE is
>> places in bs->file->bs->open_flags, and BDRV_O_NATIVE_AIO is nowhere,
>> as file-posix parse options and simply set s->use_linux_aio.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov at virtuozzo.com>
>> ---
>>
>> Hi all! I remember, I suggested to deprecate drive-backup some time ago,
>> and nobody complain.. But that old patch was inside the series with
>> other more questionable deprecations and it did not landed.
>>
>> Let's finally deprecate what should be deprecated long ago.
>>
>> We now faced a problem in our downstream, described in commit message.
>> In downstream I've fixed it by simply enabling O_DIRECT and linux_aio
>> unconditionally for drive_backup target. But actually this just shows
>> that using drive-backup in blockdev era is a bad idea. So let's motivate
>> everyone (including Virtuozzo of course) to move to new interfaces and
>> avoid problems with all that outdated option inheritance.
>>
>>   docs/system/deprecated.rst | 5 +++++
>>   qapi/block-core.json       | 5 ++++-
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
>> index 80cae86252..b6f5766e17 100644
>> --- a/docs/system/deprecated.rst
>> +++ b/docs/system/deprecated.rst
>> @@ -186,6 +186,11 @@ Use the more generic commands ``block-export-add`` and ``block-export-del``
>>   instead.  As part of this deprecation, where ``nbd-server-add`` used a
>>   single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
>>   
>> +``drive-backup`` (since 6.0)
> 
> NB, it'll need to be 6.1 by time this patch has a chance to merge

Oops yes.


-- 
Best regards,
Vladimir