| Message ID | 20210505135803.67896-4-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi & doc: deprecate drive-backup | expand |
On Wed, May 05, 2021 at 04:58:03PM +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. > > The documentation is updated in a minimal way, so that drive-backup is > noted only as a deprecated command, and blockdev-backup used in most of > places. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > > TODO: We also need to deprecate drive-backup transaction action.. > But union members in QAPI doesn't support 'deprecated' feature. I tried > to dig a bit, but failed :/ Markus, could you please help with it? At > least by advice? > > docs/interop/live-block-operations.rst | 47 +++++++++++++++++--------- > docs/system/deprecated.rst | 11 ++++++ > qapi/block-core.json | 5 ++- The core changes itself looks good; I have some minor nit-picks below, hope that's not annoying. :-) With those addressed: Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com> > 3 files changed, 46 insertions(+), 17 deletions(-) > > diff --git a/docs/interop/live-block-operations.rst b/docs/interop/live-block-operations.rst > index 1073b930dc..f71f79ae2a 100644 > --- a/docs/interop/live-block-operations.rst > +++ b/docs/interop/live-block-operations.rst > @@ -116,8 +116,8 @@ QEMU block layer supports. > (3) ``drive-mirror`` (and ``blockdev-mirror``): Synchronize a running > disk to another image. > > -(4) ``drive-backup`` (and ``blockdev-backup``): Point-in-time (live) copy > - of a block device to a destination. > +(4) ``blockdev-backup`` (and deprecated ``drive-backup``): Point-in-time > + (live) copy of a block device to a destination. nit: s/deprecated ``drive-backup``/the deprecated ``drive-backup``/ > > .. _`Interacting with a QEMU instance`: > @@ -553,13 +553,14 @@ Currently, there are four different kinds: > > (3) ``none`` -- Synchronize only the new writes from this point on. > > - .. note:: In the case of ``drive-backup`` (or ``blockdev-backup``), > - the behavior of ``none`` synchronization mode is different. > - Normally, a ``backup`` job consists of two parts: Anything > - that is overwritten by the guest is first copied out to > - the backup, and in the background the whole image is > - copied from start to end. With ``sync=none``, it's only > - the first part. > + .. note:: In the case of ``blockdev-backup`` (or deprecated > + ``drive-backup``), the behavior of ``none`` > + synchronization mode is different. Normally, a > + ``backup`` job consists of two parts: Anything that is > + overwritten by the guest is first copied out to the > + backup, and in the background the whole image is copied > + from start to end. With ``sync=none``, it's only the > + first part. > > (4) ``incremental`` -- Synchronize content that is described by the > dirty bitmap > @@ -924,19 +925,22 @@ Shutdown the guest, by issuing the ``quit`` QMP command:: > } > > > -Live disk backup --- ``drive-backup`` and ``blockdev-backup`` > -------------------------------------------------------------- > +Live disk backup --- ``blockdev-backup`` and deprecated``drive-backup`` > +----------------------------------------------------------------------- Here too, missing the article "the": "the deprecated". > -The ``drive-backup`` (and its newer equivalent ``blockdev-backup``) allows > +The ``blockdev-backup`` (and deprecated ``drive-backup``) allows > you to create a point-in-time snapshot. > > -In this case, the point-in-time is when you *start* the ``drive-backup`` > -(or its newer equivalent ``blockdev-backup``) command. > +In this case, the point-in-time is when you *start* the > +``blockdev-backup`` (or deprecated ``drive-backup``) command. > > > QMP invocation for ``drive-backup`` > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > +Note that ``drive-backup`` command is deprecated since Qemu 6.1 and > +will be removed in future. nit: Let's consistently spell QEMU in all caps, please: s/Qemu/QEMU/ > Yet again, starting afresh with our example disk image chain:: > > [A] <-- [B] <-- [C] <-- [D] > @@ -961,11 +965,22 @@ will be issued, indicating the live block device job operation has > completed, and no further action is required. > > > +Moving from deprecated ``drive-backup`` to newer ``blockdev-backup`` > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ s/from/from the/ > +``blockdev-backup`` differs in a way of specifying backup target. I might slightly rephrase it this way: ``blockdev-backup`` differs from ``drive-backup`` in how you specify the backup target. > +With ``blockdev-backup`` you can't specify filename as a target. > +Instead you use node-name of existing block node, which you may add Can use literals also for node-name: s/node-name/``node-name`` > +by ``blockdev-add`` or ``blockdev-create`` commands. Correspondingly, > +``blockdev-backup`` doesn't have ``mode`` and ``format`` arguments > +which don't apply to existing block node. See following sections for s/to/to an/ > +details and examples. > + > + > Notes on ``blockdev-backup`` > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > -The ``blockdev-backup`` command is equivalent in functionality to > -``drive-backup``, except that it operates at node-level in a Block Driver > +The ``blockdev-backup`` operates at node-level in a Block Driver > State (BDS) graph. s/``blockdev-backup``/``blockdev-backup`` command/ > E.g. the sequence of actions to create a point-in-time backup > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 80cae86252..676d72a1ed 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -186,6 +186,17 @@ 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.1) > +'''''''''''''''''''''''''''' > + > +Use ``blockdev-backup`` in pair with ``blockdev-add`` instead. nit: s/in pair/in combination/ > +This change primarily separates the creation/opening process of the backup > +target with explicit, separate steps. ``blockdev-backup`` uses mostly the > +same arguments as ``drive-backup``, except the ``format`` and ``mode`` > +options are removed in favor of using explicit ``blockdev-create`` and > +``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for > +details. The rest reads good to me. Thanks for fixing this. [...]
On 5/6/21 5:57 AM, Kashyap Chamarthy wrote: > TODO: We also need to deprecate drive-backup transaction action.. > But union members in QAPI doesn't support 'deprecated' feature. I tried > to dig a bit, but failed :/ Markus, could you please help with it? At > least by advice? Oho, I see. OK, I'm not Markus, but I've been getting into lots of trouble in the QAPI generator lately, so let me see if I can help get you started... https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ Here's a quick hack that might expose that feature. I suppose we can discuss this with Markus and turn these into real patches if that's the direction we wanna head. --js
15.05.2021 01:38, John Snow wrote: > On 5/6/21 5:57 AM, Kashyap Chamarthy wrote: >> TODO: We also need to deprecate drive-backup transaction action.. >> But union members in QAPI doesn't support 'deprecated' feature. I tried >> to dig a bit, but failed :/ Markus, could you please help with it? At >> least by advice? > > Oho, I see. > > OK, I'm not Markus, but I've been getting into lots of trouble in the QAPI generator lately, so let me see if I can help get you started... > > https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ > > Here's a quick hack that might expose that feature. I suppose we can discuss this with Markus and turn these into real patches if that's the direction we wanna head. > Hi! Markus is silent.. Maybe, you'll send patches ? )
On 5/24/21 10:06 AM, Vladimir Sementsov-Ogievskiy wrote: > 15.05.2021 01:38, John Snow wrote: >> On 5/6/21 5:57 AM, Kashyap Chamarthy wrote: >>> TODO: We also need to deprecate drive-backup transaction action.. >>> But union members in QAPI doesn't support 'deprecated' feature. I tried >>> to dig a bit, but failed :/ Markus, could you please help with it? At >>> least by advice? >> >> Oho, I see. >> >> OK, I'm not Markus, but I've been getting into lots of trouble in the >> QAPI generator lately, so let me see if I can help get you started... >> >> https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ >> >> Here's a quick hack that might expose that feature. I suppose we can >> discuss this with Markus and turn these into real patches if that's >> the direction we wanna head. >> > > Hi! Markus is silent.. Maybe, you'll send patches ? ) > > He just went on PTO for 2 weeks :') It's going to have to wait, I'm afraid ...
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: [...] > TODO: We also need to deprecate drive-backup transaction action.. > But union members in QAPI doesn't support 'deprecated' feature. I tried > to dig a bit, but failed :/ Markus, could you please help with it? At > least by advice? There are two closely related things in play here: the union branch and the corresponding enum value. So far, the QAPI schema language doesn't support tacking feature flags to either. If an enum value is deprecated, any union branches corresponding to it must also be deprecated (because their use requires using the deprecated enum value). The converse is not true, but I can't see a use for deprecating a union branch without also deprecating the enum member. I think we can implement feature flags just for enum members, then document that 'deprecated' enum value implies corresponding union branches are also deprecated. I have unfinished patches implementing feature flags for enum members. Since TransactionAction is a simple union, the corresponding enum is implicit. We can make it explicit by converting to a flat union. Simple unions need to die anyway. Does this make sense?
08.06.2021 14:12, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > > [...] > >> TODO: We also need to deprecate drive-backup transaction action.. >> But union members in QAPI doesn't support 'deprecated' feature. I tried >> to dig a bit, but failed :/ Markus, could you please help with it? At >> least by advice? > > There are two closely related things in play here: the union branch and > the corresponding enum value. > > So far, the QAPI schema language doesn't support tacking feature flags > to either. > > If an enum value is deprecated, any union branches corresponding to it > must also be deprecated (because their use requires using the deprecated > enum value). > > The converse is not true, but I can't see a use for deprecating a union > branch without also deprecating the enum member. > > I think we can implement feature flags just for enum members, then > document that 'deprecated' enum value implies corresponding union > branches are also deprecated. > > I have unfinished patches implementing feature flags for enum members. > > Since TransactionAction is a simple union, the corresponding enum is > implicit. We can make it explicit by converting to a flat union. > Simple unions need to die anyway. Does BlockStatsSpecific from qapi/block-core.json a correct example of flat union you mean? I can make patch to convert TransactionAction to be similar if that helps (discriminator field should be called "type", yes?). > > Does this make sense? > Yes if it helps) Did you also look at John's https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ ? I hope you and John will send patches that you have, I'll help with reviewing (keep me in CC), and finally we'll get the feature.
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 08.06.2021 14:12, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: >> >> [...] >> >>> TODO: We also need to deprecate drive-backup transaction action.. >>> But union members in QAPI doesn't support 'deprecated' feature. I tried >>> to dig a bit, but failed :/ Markus, could you please help with it? At >>> least by advice? >> >> There are two closely related things in play here: the union branch and >> the corresponding enum value. >> >> So far, the QAPI schema language doesn't support tacking feature flags >> to either. >> >> If an enum value is deprecated, any union branches corresponding to it >> must also be deprecated (because their use requires using the deprecated >> enum value). >> >> The converse is not true, but I can't see a use for deprecating a union >> branch without also deprecating the enum member. >> >> I think we can implement feature flags just for enum members, then >> document that 'deprecated' enum value implies corresponding union >> branches are also deprecated. >> >> I have unfinished patches implementing feature flags for enum members. >> >> Since TransactionAction is a simple union, the corresponding enum is >> implicit. We can make it explicit by converting to a flat union. >> Simple unions need to die anyway. > > > Does BlockStatsSpecific from qapi/block-core.json a correct example of flat union you mean? I can make patch to convert TransactionAction to be similar if that helps (discriminator field should be called "type", yes?). From docs/devel/qapi-code-gen.txt: A simple union can always be re-written as a flat union where the base class has a single member named 'type', and where each branch of the union has a struct with a single member named 'data'. That is, { 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } } is identical on the wire to: { 'enum': 'Enum', 'data': ['one', 'two'] } { 'struct': 'Branch1', 'data': { 'data': 'str' } } { 'struct': 'Branch2', 'data': { 'data': 'int' } } { 'union': 'Flat', 'base': { 'type': 'Enum' }, 'discriminator': 'type', 'data': { 'one': 'Branch1', 'two': 'Branch2' } } The generated C isn't identical, but adjusting the code using it should be straightforward. >> Does this make sense? >> > > Yes if it helps) > > Did you also look at John's https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ ? Not yet. > I hope you and John will send patches that you have, I'll help with reviewing (keep me in CC), and finally we'll get the feature. Sounds like a plan. I need to get my post-vacation e-mail pileup under control first.
09.06.2021 13:49, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > >> 08.06.2021 14:12, Markus Armbruster wrote: >>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: >>> >>> [...] >>> >>>> TODO: We also need to deprecate drive-backup transaction action.. >>>> But union members in QAPI doesn't support 'deprecated' feature. I tried >>>> to dig a bit, but failed :/ Markus, could you please help with it? At >>>> least by advice? >>> >>> There are two closely related things in play here: the union branch and >>> the corresponding enum value. >>> >>> So far, the QAPI schema language doesn't support tacking feature flags >>> to either. >>> >>> If an enum value is deprecated, any union branches corresponding to it >>> must also be deprecated (because their use requires using the deprecated >>> enum value). >>> >>> The converse is not true, but I can't see a use for deprecating a union >>> branch without also deprecating the enum member. >>> >>> I think we can implement feature flags just for enum members, then >>> document that 'deprecated' enum value implies corresponding union >>> branches are also deprecated. >>> >>> I have unfinished patches implementing feature flags for enum members. >>> >>> Since TransactionAction is a simple union, the corresponding enum is >>> implicit. We can make it explicit by converting to a flat union. >>> Simple unions need to die anyway. >> >> >> Does BlockStatsSpecific from qapi/block-core.json a correct example of flat union you mean? I can make patch to convert TransactionAction to be similar if that helps (discriminator field should be called "type", yes?). > > From docs/devel/qapi-code-gen.txt: > > A simple union can always be re-written as a flat union where the base > class has a single member named 'type', and where each branch of the > union has a struct with a single member named 'data'. That is, > > { 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } } > > is identical on the wire to: > > { 'enum': 'Enum', 'data': ['one', 'two'] } > { 'struct': 'Branch1', 'data': { 'data': 'str' } } > { 'struct': 'Branch2', 'data': { 'data': 'int' } } > { 'union': 'Flat', 'base': { 'type': 'Enum' }, 'discriminator': 'type', > 'data': { 'one': 'Branch1', 'two': 'Branch2' } } > > The generated C isn't identical, but adjusting the code using it should > be straightforward. > >>> Does this make sense? >>> >> >> Yes if it helps) >> >> Did you also look at John's https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ ? > > Not yet. > >> I hope you and John will send patches that you have, I'll help with reviewing (keep me in CC), and finally we'll get the feature. > > Sounds like a plan. I need to get my post-vacation e-mail pileup under > control first. > Hi! Kindly remind in the case you forget :) Or may be I miss some patches?
24.05.2021 21:37, John Snow wrote: > On 5/24/21 10:06 AM, Vladimir Sementsov-Ogievskiy wrote: >> 15.05.2021 01:38, John Snow wrote: >>> On 5/6/21 5:57 AM, Kashyap Chamarthy wrote: >>>> TODO: We also need to deprecate drive-backup transaction action.. >>>> But union members in QAPI doesn't support 'deprecated' feature. I tried >>>> to dig a bit, but failed :/ Markus, could you please help with it? At >>>> least by advice? >>> >>> Oho, I see. >>> >>> OK, I'm not Markus, but I've been getting into lots of trouble in the QAPI generator lately, so let me see if I can help get you started... >>> >>> https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ >>> >>> Here's a quick hack that might expose that feature. I suppose we can discuss this with Markus and turn these into real patches if that's the direction we wanna head. >>> >> >> Hi! Markus is silent.. Maybe, you'll send patches ? ) >> >> > > He just went on PTO for 2 weeks :') > > It's going to have to wait, I'm afraid ... > Hi! Any plans or updates? John, may be you just send your patches?
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 24.05.2021 21:37, John Snow wrote: >> On 5/24/21 10:06 AM, Vladimir Sementsov-Ogievskiy wrote: >>> 15.05.2021 01:38, John Snow wrote: >>>> On 5/6/21 5:57 AM, Kashyap Chamarthy wrote: >>>>> TODO: We also need to deprecate drive-backup transaction action.. >>>>> But union members in QAPI doesn't support 'deprecated' feature. I tried >>>>> to dig a bit, but failed :/ Markus, could you please help with it? At >>>>> least by advice? >>>> >>>> Oho, I see. >>>> >>>> OK, I'm not Markus, but I've been getting into lots of trouble in the QAPI generator lately, so let me see if I can help get you started... >>>> >>>> https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ >>>> >>>> Here's a quick hack that might expose that feature. I suppose we can discuss this with Markus and turn these into real patches if that's the direction we wanna head. >>>> >>> >>> Hi! Markus is silent.. Maybe, you'll send patches ? ) >>> >>> >> He just went on PTO for 2 weeks :') >> It's going to have to wait, I'm afraid ... >> > > Hi! > > Any plans or updates? John, may be you just send your patches? Yes, please! I intend to resume working on feature flags next week.
Markus Armbruster <armbru@redhat.com> writes: > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > >> 08.06.2021 14:12, Markus Armbruster wrote: >>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: >>> >>> [...] >>> >>>> TODO: We also need to deprecate drive-backup transaction action.. >>>> But union members in QAPI doesn't support 'deprecated' feature. I tried >>>> to dig a bit, but failed :/ Markus, could you please help with it? At >>>> least by advice? >>> >>> There are two closely related things in play here: the union branch and >>> the corresponding enum value. >>> >>> So far, the QAPI schema language doesn't support tacking feature flags >>> to either. >>> >>> If an enum value is deprecated, any union branches corresponding to it >>> must also be deprecated (because their use requires using the deprecated >>> enum value). >>> >>> The converse is not true, but I can't see a use for deprecating a union >>> branch without also deprecating the enum member. >>> >>> I think we can implement feature flags just for enum members, then >>> document that 'deprecated' enum value implies corresponding union >>> branches are also deprecated. >>> >>> I have unfinished patches implementing feature flags for enum members. >>> >>> Since TransactionAction is a simple union, the corresponding enum is >>> implicit. We can make it explicit by converting to a flat union. >>> Simple unions need to die anyway. >> >> >> Does BlockStatsSpecific from qapi/block-core.json a correct example of flat union you mean? I can make patch to convert TransactionAction to be similar if that helps (discriminator field should be called "type", yes?). > > From docs/devel/qapi-code-gen.txt: > > A simple union can always be re-written as a flat union where the base > class has a single member named 'type', and where each branch of the > union has a struct with a single member named 'data'. That is, > > { 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } } > > is identical on the wire to: > > { 'enum': 'Enum', 'data': ['one', 'two'] } > { 'struct': 'Branch1', 'data': { 'data': 'str' } } > { 'struct': 'Branch2', 'data': { 'data': 'int' } } > { 'union': 'Flat', 'base': { 'type': 'Enum' }, 'discriminator': 'type', > 'data': { 'one': 'Branch1', 'two': 'Branch2' } } > > The generated C isn't identical, but adjusting the code using it should > be straightforward. > >>> Does this make sense? >>> >> >> Yes if it helps) >> >> Did you also look at John's https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ ? > > Not yet. > >> I hope you and John will send patches that you have, I'll help with reviewing (keep me in CC), and finally we'll get the feature. > > Sounds like a plan. I need to get my post-vacation e-mail pileup under > control first. Just sent: Subject: [PATCH RFC 0/5] Subject: [PATCH RFC 0/5] qapi: Add feature flags to enum members Message-Id: <20210915192425.4104210-1-armbru@redhat.com> Yes, I mangled the subject %-/
diff --git a/docs/interop/live-block-operations.rst b/docs/interop/live-block-operations.rst index 1073b930dc..f71f79ae2a 100644 --- a/docs/interop/live-block-operations.rst +++ b/docs/interop/live-block-operations.rst @@ -116,8 +116,8 @@ QEMU block layer supports. (3) ``drive-mirror`` (and ``blockdev-mirror``): Synchronize a running disk to another image. -(4) ``drive-backup`` (and ``blockdev-backup``): Point-in-time (live) copy - of a block device to a destination. +(4) ``blockdev-backup`` (and deprecated ``drive-backup``): Point-in-time + (live) copy of a block device to a destination. .. _`Interacting with a QEMU instance`: @@ -553,13 +553,14 @@ Currently, there are four different kinds: (3) ``none`` -- Synchronize only the new writes from this point on. - .. note:: In the case of ``drive-backup`` (or ``blockdev-backup``), - the behavior of ``none`` synchronization mode is different. - Normally, a ``backup`` job consists of two parts: Anything - that is overwritten by the guest is first copied out to - the backup, and in the background the whole image is - copied from start to end. With ``sync=none``, it's only - the first part. + .. note:: In the case of ``blockdev-backup`` (or deprecated + ``drive-backup``), the behavior of ``none`` + synchronization mode is different. Normally, a + ``backup`` job consists of two parts: Anything that is + overwritten by the guest is first copied out to the + backup, and in the background the whole image is copied + from start to end. With ``sync=none``, it's only the + first part. (4) ``incremental`` -- Synchronize content that is described by the dirty bitmap @@ -924,19 +925,22 @@ Shutdown the guest, by issuing the ``quit`` QMP command:: } -Live disk backup --- ``drive-backup`` and ``blockdev-backup`` -------------------------------------------------------------- +Live disk backup --- ``blockdev-backup`` and deprecated``drive-backup`` +----------------------------------------------------------------------- -The ``drive-backup`` (and its newer equivalent ``blockdev-backup``) allows +The ``blockdev-backup`` (and deprecated ``drive-backup``) allows you to create a point-in-time snapshot. -In this case, the point-in-time is when you *start* the ``drive-backup`` -(or its newer equivalent ``blockdev-backup``) command. +In this case, the point-in-time is when you *start* the +``blockdev-backup`` (or deprecated ``drive-backup``) command. QMP invocation for ``drive-backup`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Note that ``drive-backup`` command is deprecated since Qemu 6.1 and +will be removed in future. + Yet again, starting afresh with our example disk image chain:: [A] <-- [B] <-- [C] <-- [D] @@ -961,11 +965,22 @@ will be issued, indicating the live block device job operation has completed, and no further action is required. +Moving from deprecated ``drive-backup`` to newer ``blockdev-backup`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``blockdev-backup`` differs in a way of specifying backup target. +With ``blockdev-backup`` you can't specify filename as a target. +Instead you use node-name of existing block node, which you may add +by ``blockdev-add`` or ``blockdev-create`` commands. Correspondingly, +``blockdev-backup`` doesn't have ``mode`` and ``format`` arguments +which don't apply to existing block node. See following sections for +details and examples. + + Notes on ``blockdev-backup`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The ``blockdev-backup`` command is equivalent in functionality to -``drive-backup``, except that it operates at node-level in a Block Driver +The ``blockdev-backup`` operates at node-level in a Block Driver State (BDS) graph. E.g. the sequence of actions to create a point-in-time backup diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 80cae86252..676d72a1ed 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -186,6 +186,17 @@ 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.1) +'''''''''''''''''''''''''''' + +Use ``blockdev-backup`` in pair with ``blockdev-add`` instead. +This change primarily separates the creation/opening process of the backup +target with explicit, separate steps. ``blockdev-backup`` uses mostly the +same arguments as ``drive-backup``, except the ``format`` and ``mode`` +options are removed in favor of using explicit ``blockdev-create`` and +``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for +details. + System accelerators ------------------- diff --git a/qapi/block-core.json b/qapi/block-core.json index 6d227924d0..8e2c6e1622 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1642,6 +1642,9 @@ # The operation can be stopped before it has completed using the # block-job-cancel command. # +# Features: +# @deprecated: This command is deprecated. Use @blockdev-backup instead. +# # Returns: - nothing on success # - If @device is not a valid block device, GenericError # @@ -1657,7 +1660,7 @@ # ## { 'command': 'drive-backup', 'boxed': true, - 'data': 'DriveBackup' } + 'data': 'DriveBackup', 'features': ['deprecated'] } ## # @blockdev-backup:
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. The documentation is updated in a minimal way, so that drive-backup is noted only as a deprecated command, and blockdev-backup used in most of places. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- TODO: We also need to deprecate drive-backup transaction action.. But union members in QAPI doesn't support 'deprecated' feature. I tried to dig a bit, but failed :/ Markus, could you please help with it? At least by advice? docs/interop/live-block-operations.rst | 47 +++++++++++++++++--------- docs/system/deprecated.rst | 11 ++++++ qapi/block-core.json | 5 ++- 3 files changed, 46 insertions(+), 17 deletions(-)