[PATCH 1/2] qemu: Delete nvram store for transient domains too

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

There are two ways how to use nvram variable store file in
libvirt:

  1) create it by hand and pass the path in domain XML

  2) let libvirt generate path and create the file

Now, we allow users to remove the file from case 2) by passing a
flag into the virDomainUndefineFlags(). But when implementing
this I forgot about transient domains. For them the file needs to
be removed in qemuProcessStop(). But not always, only if the file
was created by us.  For that reason new attribute to <nvram/> is
invented: @generated to keep the info between daemon restarts.
However, now that we are removing autogenerated file for
transient domains automatically, we should do the same for
persistent domains too without requiring the
VIR_DOMAIN_UNDEFINE_NVRAM flag.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/conf/domain_conf.c  | 10 ++++++++--
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_driver.c  |  3 ++-
 src/qemu/qemu_process.c | 14 +++++++++++---
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c65276..2bcf123 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13290,6 +13290,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 
             def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt);
             def->os.loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt);
+            if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS)
+                def->os.loader->generated = virXPathBoolean("boolean(./os/nvram[1]/@generated)", ctxt);
         }
     }
 
@@ -18694,7 +18696,8 @@ virDomainHugepagesFormat(virBufferPtr buf,
 
 static void
 virDomainLoaderDefFormat(virBufferPtr buf,
-                         virDomainLoaderDefPtr loader)
+                         virDomainLoaderDefPtr loader,
+                         unsigned int flags)
 {
     const char *readonly = virTristateBoolTypeToString(loader->readonly);
     const char *type = virDomainLoaderTypeToString(loader->type);
@@ -18710,6 +18713,9 @@ virDomainLoaderDefFormat(virBufferPtr buf,
     if (loader->nvram || loader->templt) {
         virBufferAddLit(buf, "<nvram");
         virBufferEscapeString(buf, " template='%s'", loader->templt);
+        if (loader->generated &&
+            flags & VIR_DOMAIN_XML_INTERNAL_STATUS)
+            virBufferAddLit(buf, " generated='yes'");
         if (loader->nvram)
             virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram);
         else
@@ -19056,7 +19062,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         virBufferEscapeString(buf, "<initarg>%s</initarg>\n",
                               def->os.initargv[i]);
     if (def->os.loader)
-        virDomainLoaderDefFormat(buf, def->os.loader);
+        virDomainLoaderDefFormat(buf, def->os.loader, flags);
     virBufferEscapeString(buf, "<kernel>%s</kernel>\n",
                           def->os.kernel);
     virBufferEscapeString(buf, "<initrd>%s</initrd>\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 530a3ca..19b282b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1692,6 +1692,7 @@ struct _virDomainLoaderDef {
     int readonly;   /* enum virTristateBool */
     virDomainLoader type;
     char *nvram;    /* path to non-volatile RAM */
+    bool generated; /* Is the @nvram path provided from XML or generated? */
     char *templt;   /* user override of path to master nvram */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index acf2b9a..9f51b98 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6816,7 +6816,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
     if (!virDomainObjIsActive(vm) &&
         vm->def->os.loader && vm->def->os.loader->nvram &&
         virFileExists(vm->def->os.loader->nvram)) {
-        if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
+        if (!vm->def->os.loader->generated &&
+            !(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                            _("cannot delete inactive domain with nvram"));
             goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 24e5f0f..c0ab341 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3909,7 +3909,6 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
     int srcFD = -1;
     int dstFD = -1;
     virDomainLoaderDefPtr loader = def->os.loader;
-    bool generated = false;
     bool created = false;
 
     /* Unless domain has RO loader of pflash type, we have
@@ -3938,7 +3937,7 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
                         LOCALSTATEDIR, def->name) < 0)
             goto cleanup;
 
-        generated = true;
+        loader->generated = true;
 
         if (virDomainSaveConfig(cfg->configDir, def) < 0)
             goto cleanup;
@@ -4022,8 +4021,10 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
     if (ret < 0) {
         if (created)
             unlink(loader->nvram);
-        if (generated)
+        if (loader->generated) {
             VIR_FREE(loader->nvram);
+            loader->generated = false;
+        }
     }
 
     VIR_FORCE_CLOSE(srcFD);
@@ -5035,6 +5036,13 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         }
     }
 
+    /* Remove nvram file, if needed */
+    if (!vm->persistent &&
+        def->os.loader &&
+        def->os.loader->nvram &&
+        def->os.loader->generated)
+        unlink(vm->def->os.loader->nvram);
+
     vm->taint = 0;
     vm->pid = -1;
     virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
-- 
2.0.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list