On 09/09/2017 04:46 AM, Pavel Hrdina wrote: > 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@xxxxxxxxxx> >> >> 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, > ACK to that Thanks, Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
