[libvirt] [PATCH 2/2] virDomainUndefineFlags: Allow NVRAM unlinking

[Michal Privoznik]

When a domain is undefined, there are options to remove it's
managed save state or snapshots. However, there's another file
that libvirt creates per domain: the NVRAM variable store file.
Make sure that the file is not left behind if the domain is
undefined.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 include/libvirt/libvirt.h.in |  2 ++
 src/qemu/qemu_driver.c       | 19 ++++++++++++++++++-
 tools/virsh-domain.c         | 15 ++++++++++++---
 tools/virsh.pod              |  6 +++++-
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 94b942c..3c2a51a 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2257,6 +2257,8 @@ typedef enum {
     VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain,
                                                           then also remove any
                                                           snapshot metadata */
+    VIR_DOMAIN_UNDEFINE_NVRAM              = (1 << 2), /* Also remove any
+                                                          nvram file */
 
     /* Future undefine control flags should come here. */
 } virDomainUndefineFlagsValues;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a8cda43..7f17fee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6403,7 +6403,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
     virQEMUDriverConfigPtr cfg = NULL;
 
     virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
-                  VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
+                  VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA |
+                  VIR_DOMAIN_UNDEFINE_NVRAM, -1);
 
     if (!(vm = qemuDomObjFromDomain(dom)))
         return -1;
@@ -6452,6 +6453,22 @@ qemuDomainUndefineFlags(virDomainPtr dom,
         }
     }
 
+    if (!virDomainObjIsActive(vm) &&
+        vm->def->os.loader && vm->def->os.loader->nvram) {
+        if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
+            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                           _("cannot delete inactive domain with nvram"));
+            goto cleanup;
+        }
+
+        if (unlink(vm->def->os.loader->nvram) < 0) {
+            virReportSystemError(errno,
+                                 _("failed to remove nvram: %s"),
+                                 vm->def->os.loader->nvram);
+            goto cleanup;
+        }
+    }
+
     if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
         goto cleanup;
 
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6aa8631..db52271 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3248,6 +3248,10 @@ static const vshCmdOptDef opts_undefine[] = {
      .type = VSH_OT_BOOL,
      .help = N_("remove all domain snapshot metadata, if inactive")
     },
+    {.name = "nvram",
+     .type = VSH_OT_BOOL,
+     .help = N_("remove nvram file, if inactive")
+    },
     {.name = NULL}
 };
 
@@ -3270,6 +3274,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
     bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata");
     bool wipe_storage = vshCommandOptBool(cmd, "wipe-storage");
     bool remove_all_storage = vshCommandOptBool(cmd, "remove-all-storage");
+    bool nvram = vshCommandOptBool(cmd, "nvram");
     /* Positive if these items exist.  */
     int has_managed_save = 0;
     int has_snapshots_metadata = 0;
@@ -3313,6 +3318,9 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
         snapshots_safe = true;
     }
+    if (nvram) {
+        flags |= VIR_DOMAIN_UNDEFINE_NVRAM;
+    }
 
     if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
         return false;
@@ -3504,10 +3512,11 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
      * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA flag was not present
      * until 0.9.5; skip to piecewise emulation if we couldn't prove
      * above that the new API is safe.  */
-    if (managed_save_safe && snapshots_safe) {
+    if ((managed_save_safe && snapshots_safe) || nvram) {
         rc = virDomainUndefineFlags(dom, flags);
-        if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT &&
-                        last_error->code != VIR_ERR_INVALID_ARG))
+        if (rc == 0 || nvram ||
+            (last_error->code != VIR_ERR_NO_SUPPORT &&
+             last_error->code != VIR_ERR_INVALID_ARG))
             goto out;
         vshResetLibvirtError();
     }
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 60ee515..ae396d2 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2083,7 +2083,7 @@ Output the device used for the TTY console of the domain. If the information
 is not available the processes will provide an exit code of 1.
 
 =item B<undefine> I<domain> [I<--managed-save>] [I<--snapshots-metadata>]
-[ {I<--storage> B<volumes> | I<--remove-all-storage>} I<--wipe-storage>]
+[I<--nvram>} [ {I<--storage> B<volumes> | I<--remove-all-storage>} I<--wipe-storage>]
 
 Undefine a domain. If the domain is running, this converts it to a
 transient domain, without stopping it. If the domain is inactive,
@@ -2099,6 +2099,10 @@ domain.  Without the flag, attempts to undefine an inactive domain with
 snapshot metadata will fail.  If the domain is active, this flag is
 ignored.
 
+The I<--nvram> flag ensures no nvram (/domain/os/nvram/) file is
+left behind. If the domain has a nvram file and the flag is
+omitted the undefine will fail.
+
 The I<--storage> flag takes a parameter B<volumes>, which is a comma separated
 list of volume target names or source paths of storage volumes to be removed
 along with the undefined domain. Volumes can be undefined and thus removed only
-- 
1.8.5.5