Re: [libvirt] [PATCH v2 09/11] virsh: Expose bulk snapshot dumpxml/redefine
On 2/27/19 10:54 AM, John Ferlan wrote:
On 2/23/19 4:24 PM, Eric Blake wrote:
> Add flags to the 'dumpxml' and 'snapshot-create' commands to pass
> the newly-added bulk snapshot flags through.
>
> For command-line convenience, I intentionally made --redefine-list
> imply --redefine, even though the counterpart C flags are distinct
> (and you get an error if you pass _REDEFINE_LIST without _REDEFINE).
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> tools/virsh-domain.c | 7 +++++++
> tools/virsh-snapshot.c | 14 ++++++++++++++
> 2 files changed, 21 insertions(+)
>
Need to update virsh.pod as well to describe new arguments. Do you think
that a separation of the dumpxml and the parse/create into order
pertinent patches would be better? That is the --snapshots for dumpxml
after patch7 (which probably could swap places w/ patch6) so that the
dumpxml and parse/create functionality is all in logical order. It's not
that important.
Yeah, I waffled on that. v1 was definitely more of a "do all dumpxml
changes, then do all redefine changes" breakup. But I intentionally
refactored this way (do all API changes, then all virsh changes),
especially since the virsh changes are so small that it's easy to review
both at once. Depending on how you feel about the qemu changes, I'm
still open to the idea of doing dumpxml separate from redefine in each
of the drivers, if it makes it easier to backport one but not the other.
Furthermore, your comments against v4 of the incremental backup sparked
my idea of possibly using VIR_DOMAIN_XML_SNAPSHOTS internally (so that
domains have just ONE xml file on disk, rather than the main domain xml
+ one xml per snapshot), and that may have fallout on making the dumpxml
portion reasonable to backport in isolation.
I'll have to see how things play out in the rest of the series before
deciding if splitting this one is worth it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org