[virt-tools-list] [virt-manager PATCH] cli: introduce snapshot parameter for disk device

[Pavel Hrdina]

On Fri, Sep 08, 2017 at 04:04:47PM -0400, Cole Robinson wrote:
> On 09/06/2017 03:18 AM, Pavel Hrdina wrote:
> > This allows to configure snapshot behavior for each disk.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430642
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> 
> Hmm. Part of my worry naming this 'snapshot' on the cli is that people will
> think it maps to qemu -drive snapshot=on behavior, and a mistake in that area
> could lead to data loss if someone just blindly uses it. But then again
> generally we try to have the virt-install cli mirror libvirt XML naming...
> 
> I think I prefer naming it snapshot_policy making its function a bit more
> clear, what do you think?

Naming it snapshot_policy works for me as well, but this got me thinking
that we need to update virt-install man page as well.  Especially if we
decide to name it snapshot_policy since it wouldn't be clear to which
XML element/attribute is this option mapped.

At first I wanted to write that I prefer "snapshot", but after looking
at libvirt documentation I think "snapshot_policy" is better.  There is
a <snapshot> element in <source> element and that could also confuse
users and would force us to use some different name if someone ask to
add a support for that element.

I'll change it to "snapshot_policy" with the man page addition.

Thanks for the review, I've pushed the other patches.

Pavel

diff --git a/man/virt-install.pod b/man/virt-install.pod
index f2a036a3..3e1dd3f2 100644
--- a/man/virt-install.pod
+++ b/man/virt-install.pod
@@ -722,6 +722,12 @@ WD-WMAP9A966149
 It defines what to do with the disk if the source file is not accessible.  See
 possible values in L<http://www.libvirt.org/formatdomain.html#elementsDisks>

+=item B<snapshot_policy>
+
+Defines default behavior of the disk during disk snapshots.  See possible
+values in L<http://www.libvirt.org/formatdomain.html#elementsDisks>,
+"snapshot" attribute of the <disk> element.
+
 =back

 See the examples section for some uses. This option deprecates -f/--file,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170909/ee79b653/attachment.sig>