Re: [PATCH] qapi: deprecate drive-backup

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(a)virtuozzo.com&gt; > --- > > 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