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

[Cole Robinson]

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?

Thanks,
Cole

> ---
>  tests/clitest.py       | 1 +
>  virtinst/cli.py        | 1 +
>  virtinst/devicedisk.py | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/clitest.py b/tests/clitest.py
> index 56148cc0..6f7a84c6 100644
> --- a/tests/clitest.py
> +++ b/tests/clitest.py
> @@ -623,6 +623,7 @@ c.add_valid("--disk /dev/zero")  # Referencing a local unmanaged /dev node
>  c.add_valid("--disk pool=default,size=.00001")  # Building 'default' pool
>  c.add_valid("--disk %(AUTOMANAGEIMG)s,size=.1")  # autocreate the pool
>  c.add_valid("--disk %(NEWIMG1)s,sparse=true,size=100000000 --check disk_size=off")  # Don't warn about fully allocated file exceeding disk space
> +c.add_valid("--disk %(EXISTIMG1)s,snapshot=no")  # Disable snasphot for disk
>  c.add_invalid("--file %(NEWIMG1)s --file-size 100000 --nonsparse")  # Nonexisting file, size too big
>  c.add_invalid("--file %(NEWIMG1)s --file-size 100000")  # Huge file, sparse, but no prompting
>  c.add_invalid("--file %(NEWIMG1)s")  # Nonexisting file, no size
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index ece9b86d..f238655c 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -2069,6 +2069,7 @@ ParserDisk.add_arg("source_host_transport", "source_host_transport")
>  
>  ParserDisk.add_arg("path", "path")
>  ParserDisk.add_arg("device", "device")
> +ParserDisk.add_arg("snapshot", "snapshot")
>  ParserDisk.add_arg("bus", "bus")
>  ParserDisk.add_arg("removable", "removable", is_onoff=True)
>  ParserDisk.add_arg("driver_cache", "cache")
> diff --git a/virtinst/devicedisk.py b/virtinst/devicedisk.py
> index abd2be99..266595d0 100644
> --- a/virtinst/devicedisk.py
> +++ b/virtinst/devicedisk.py
> @@ -468,7 +468,7 @@ class VirtualDisk(VirtualDevice):
>  
>  
>      _XML_PROP_ORDER = [
> -        "type", "device",
> +        "type", "device", "snapshot",
>          "driver_name", "driver_type",
>          "driver_cache", "driver_discard", "driver_detect_zeroes",
>          "driver_io", "error_policy",
> @@ -727,6 +727,7 @@ class VirtualDisk(VirtualDevice):
>  
>      device = XMLProperty("./@device",
>                           default_cb=lambda s: s.DEVICE_DISK)
> +    snapshot = XMLProperty("./@snapshot")
>      driver_name = XMLProperty("./driver/@name",
>                                default_cb=_get_default_driver_name)
>      driver_type = XMLProperty("./driver/@type",
>