[virt-tools-list] [virt-manager PATCH RFC 4/4] virt-manager: delete nvram file on VM deletion

[Laszlo Ersek]

On 09/11/14 18:56, Giuseppe Scrivano wrote:
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
>  virtManager/delete.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/virtManager/delete.py b/virtManager/delete.py
> index 1aa8180..0a7da67 100644
> --- a/virtManager/delete.py
> +++ b/virtManager/delete.py
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (C) 2009, 2012-2013 Red Hat, Inc.
> +# Copyright (C) 2009, 2012-2014 Red Hat, Inc.
>  # Copyright (C) 2009 Cole Robinson <crobinso at redhat.com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -238,6 +238,7 @@ def populate_storage_list(storage_list, vm, conn):
>      diskdata.append(("kernel", vm.get_xmlobj().os.kernel, True, False))
>      diskdata.append(("initrd", vm.get_xmlobj().os.initrd, True, False))
>      diskdata.append(("dtb", vm.get_xmlobj().os.dtb, True, False))
> +    diskdata.append(("nvram", vm.get_xmlobj().os.nvram, True, False))
>  
>      for target, path, ro, shared in diskdata:
>          if not path:
> 

Repeating earlier points from others and myself, publicly:

- (at some later point (definitely not now), cloning a VM should also
clone (ie. detach) the varstore as well,)

- deleting a VM should delete (optionally or maybe mandatorily) the VMs
varstore as well. And this patch does add it to the "delete these
storage files too?" dialolog -- but that only happens if the nvram
element (with a text() child) is explicit in the domain XML. It can be
implicit too however, in which case libvirtd automatically derives a
default pathname for the varstore.

Michal has a patch in libvirtd (commit 273b6581) that allows the caller
of "undefine" to request removal of the varstore. (In fact IIRC the
caller of undefine *must* specify this flag, if the VM to be undefined
has a varstore file.) I believe it would be best to drop 4/4, and just
hardcode the VIR_DOMAIN_UNDEFINE_NVRAM flag, *if* the loader is
read-only *and* has type pflash *and* libvirtd supports that undefine flag.

(The condition (loader/@readonly = 'yes' and loader/@type = 'pflash')
implies that there will be an nvram (ie. varstore) file for the VM,
either under the pathname that the nvram element specifies explicitly,
or under the pathname that libvirtd derives automatically. Note that
this is independent of where the *contents* of the varstore file
originate from; we only care if the varstore file exists as a separate
file, and if so, where. See libvirt commit 742b08e3. Hence "loader is
r/o pflash && libvirt supports VIR_DOMAIN_UNDEFINE_NVRAM" should result
in that flag being set in the undefine request.)

Thanks,
Laszlo