[libvirt] [PATCH v3 3/3] qemu: snapshot: Forbid internal snapshots with pflash firmware

[Peter Krempa]

If the variable store (<nvram>) file is raw qemu can't do a snapshot of
it and thus the snapshot would be incomplete. QEMU does no reject such
snapshot.

Additionally allowing to use a qcow2 variable store backing file would
solve this issue but then it would become eligible to become target of
the memory dump.

Offline internal snapshot would be incomplete too with either storage
format since libvirt does not handle the pflash file in this case.

Forbid such snapshot so that we can avoid problems.
---

Notes:
    v3:
    - allow overriding of the check by specifying VIR_DOMAIN_SNAPSHOT_CREATE_UNSAFE
    - report VIR_ERR_OPERATION_UNSAFE (instead of VIR_ERR_OPERATION_UNSUPPORTED)
    - tweaked commend in code (since it's not forbidden completely)
    - tweaked error message
    
    v2:
    - changed error code to OPERATION_UNSUPPORTED (from CONFIG_UNSUPPORTED)
    - dropped mention of QEMU from the error message
    - dropped mentions of OVMF or the firmware itself altoghether, the culprit is
      the pflash device regardless of the software it contains
    - mentioned all the stuff in the commit message and comment
    
    We also will need to introduce a way to snapshot the pflash for external
    snapshots which is currently impossible as well, but fortunately does not
    have inherent drawbacks as internal snapshots.

 src/qemu/qemu_driver.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 02cdd2f6b..2ca839f1c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13754,6 +13754,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
     bool active = virDomainObjIsActive(vm);
     bool reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
     bool atomic = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0;
+    bool unsafe = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_UNSAFE) != 0;
     bool found_internal = false;
     bool forbid_internal = false;
     int external = 0;
@@ -13873,6 +13874,20 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
         goto cleanup;
     }

+    /* internal snapshots + pflash based loader have the following problems:
+     * - if the variable store is raw, the snapshot is incomplete
+     * - alowing a qcow2 image as the varstore would make it eligible to receive
+     *   the vmstate dump, which would make it huge
+     * - offline snapshot would not snapshot the varstore at all
+     */
+    if (!unsafe && found_internal &&
+        vm->def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH) {
+        virReportError(VIR_ERR_OPERATION_UNSAFE, "%s",
+                       _("internal snapshots of a VM with pflash based "
+                         "firmware can corrupt the nvram data"));
+        goto cleanup;
+    }
+
     /* Alter flags to let later users know what we learned.  */
     if (external && !active)
         *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
-- 
2.12.1