[libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

[Martin Kletzander]

On Mon, Aug 07, 2017 at 02:20:06PM +0200, Michal Privoznik wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1467245
>
>Currently, there's a bug when undefining a domain with NVRAM
>store. Basically, the unlink() of the NVRAM store file happens
>during the undefine procedure iff domain is inactive. So, if
>domain is running and undefine is called the file is left behind.
>It won't be removed in the domain cleanup process either
>(qemuProcessStop). One of the solutions is to remove if
>regardless of the domain state and rely on qemu having the file
>opened. This still has a downside that if the domain is defined
>back the NVRAM store file is going to be new, any changes to the
>current one are lost (just like with any other file that is
>deleted while a process has it opened). But is it really a
>downside?
>

It might be.  Why don't we disable removing it when the domain is
running?  We have some precedence for this.  The place which already
deals with this possibility is tools/virsh-domain.c in the cmdUndefine()
where we handle --remove-all-storage.  If you look at the help of that
command it also says:

  --nvram          remove nvram file, if inactive

And that makes sense to me.  What doesn't, on the other hand, is that it
also says:

  --keep-nvram     keep nvram file, if inactive

I don't get the "if inactive" there.  But I'm not going to check who
pushed that.  At least not again =)

>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/qemu/qemu_driver.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 574c351ae..992ae2a2e 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -7367,8 +7367,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>         }
>     }
>
>-    if (!virDomainObjIsActive(vm) &&
>-        vm->def->os.loader && vm->def->os.loader->nvram &&
>+    if (vm->def->os.loader &&
>+        vm->def->os.loader->nvram &&
>         virFileExists(vm->def->os.loader->nvram)) {
>         if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
>             if (unlink(vm->def->os.loader->nvram) < 0) {
>--
>2.13.0
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170809/bdff96ac/attachment-0001.sig>