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

[Cole Robinson]

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 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,
> 

ACK to that

Thanks,
Cole