Fix UI rename with firmware='efi'

Our code to duplicate nvram wasn't expecting the XML to be devoid
of an nvram path.

Resolves: https://github.com/virt-manager/virt-manager/issues/372

Signed-off-by: Cole Robinson <crobinso@redhat.com>

tests/uitests/data/live/uitests-firmware-efi.xml

@@ -0,0 +1,14 @@
+<domain type="kvm">
+  <name>uitests-firmware-efi</name>
+  <memory>65536</memory>
+  <currentMemory>65536</currentMemory>
+  <vcpu>1</vcpu>
+  <os firmware='efi'>
+    <type arch="x86_64">hvm</type>
+    <boot dev="hd"/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+</domain>
+

tests/uitests/test_livetests.py

@@ -38,7 +38,7 @@ def _vm_wrapper(vmname, uri="qemu:///system", opts=None):
                 except Exception:
                     pass
                 try:
-                    dom.undefine()
+                    dom.undefineFlags(libvirt.VIR_DOMAIN_UNDEFINE_NVRAM)
                     dom.destroy()
                 except Exception:
                     pass
@@ -499,3 +499,45 @@ def testLiveHotplug(app, dom):
             pool.undefine()
         except Exception:
             log.debug("Error cleaning up pool", exc_info=True)
+
+
+@_vm_wrapper("uitests-firmware-efi")
+def testFirmwareRename(app, dom):
+    from virtinst import cli, DeviceDisk
+    win = app.topwin
+    dom.destroy()
+
+    # First we refresh the 'nvram' pool, so we can reliably
+    # check if nvram files are created/deleted as expected
+    conn = cli.getConnection(app.conn.getURI())
+    origname = dom.name()
+    nvramdir = conn.get_libvirt_data_root_dir() + "/qemu/nvram"
+
+    fakedisk = DeviceDisk(conn)
+    fakedisk.set_source_path(nvramdir + "/FAKE-UITEST-FILE")
+    nvram_pool = fakedisk.get_parent_pool()
+    nvram_pool.refresh()
+
+    origpath = "%s/%s_VARS.fd" % (nvramdir, origname)
+    newname = "uitests-firmware-efi-renamed"
+    newpath = "%s/%s_VARS.fd" % (nvramdir, newname)
+    assert DeviceDisk.path_definitely_exists(app.conn, origpath)
+    assert not DeviceDisk.path_definitely_exists(app.conn, newpath)
+
+    # Now do the actual UI clickage
+    win.find("Details", "radio button").click()
+    win.find("Hypervisor Details", "label")
+    win.find("Overview", "table cell").click()
+
+    newname = "uitests-firmware-efi-renamed"
+    win.find("Name:", "text").set_text(newname)
+    appl = win.find("config-apply")
+    appl.click()
+    lib.utils.check(lambda: not appl.sensitive)
+
+    # Confirm window was updated
+    app.find_window("%s on" % newname)
+
+    # Confirm nvram paths were altered as expected
+    assert not DeviceDisk.path_definitely_exists(app.conn, origpath)
+    assert DeviceDisk.path_definitely_exists(app.conn, newpath)

virtManager/object/domain.py

@@ -433,10 +433,8 @@ class vmmDomain(vmmLibvirtObject):
         return False
 
     def has_nvram(self):
-        return bool(self.get_xmlobj().os.firmware == 'efi' or
-                    (self.get_xmlobj().os.loader_ro is True and
-                     self.get_xmlobj().os.loader_type == "pflash" and
-                     self.get_xmlobj().os.nvram))
+        return bool(self.get_xmlobj().is_uefi() or
+                    self.get_xmlobj().os.nvram)
 
     def is_persistent(self):
         return bool(self._backend.isPersistent())
@@ -540,9 +538,32 @@ class vmmDomain(vmmLibvirtObject):
         We need to do this copy magic because there is no Libvirt storage API
         to rename storage volume.
         """
+        if not self.has_nvram():
+            return None, None
+
+        old_nvram_path = self.get_xmlobj().os.nvram
+        if not old_nvram_path:
+            # Probably using firmware=efi which doesn't put nvram
+            # path in the XML. Build the implied path
+            old_nvram_path = os.path.join(
+                self.conn.get_backend().get_libvirt_data_root_dir(),
+                self.conn.get_backend().get_uri_driver(),
+                "nvram", "%s_VARS.fd" % self.get_name())
+            log.debug("Guest is expected to use <nvram> but we didn't "
+                      "find one in the XML. Generated implied path=%s",
+                      old_nvram_path)
+
+        if not DeviceDisk.path_definitely_exists(
+                self.conn.get_backend(),
+                old_nvram_path):  # pragma: no cover
+            log.debug("old_nvram_path=%s but it doesn't appear to exist. "
+                      "skipping rename nvram duplication", old_nvram_path)
+            return None, None
+
+
         from virtinst import Cloner
         old_nvram = DeviceDisk(self.conn.get_backend())
-        old_nvram.set_source_path(self.get_xmlobj().os.nvram)
+        old_nvram.set_source_path(old_nvram_path)
 
         nvram_dir = os.path.dirname(old_nvram.get_source_path())
         new_nvram_path = os.path.join(nvram_dir,
@@ -564,10 +585,7 @@ class vmmDomain(vmmLibvirtObject):
             return
         Guest.validate_name(self.conn.get_backend(), str(new_name))
 
-        new_nvram = None
-        old_nvram = None
-        if self.has_nvram():
-            new_nvram, old_nvram = self._copy_nvram_file(new_name)
+        new_nvram, old_nvram = self._copy_nvram_file(new_name)
 
         try:
             self.define_name(new_name)