On 03/23/17 10:54, Peter Krempa wrote:
On Thu, Mar 23, 2017 at 10:48:01 +0100, Laszlo Ersek wrote:
On 03/23/17 10:29, Peter Krempa wrote:
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: 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 | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 676295208..202d190b3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13873,6 +13873,16 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, goto cleanup; }
+ /* Internal snapshots don't work with VMs with OVMF loader since qemu does + * not snapshot the variable store */ + if (found_internal && + vm->def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("internal snapshots of a VM with pflash based " + "firmware are not supported")); + goto cleanup; + } + /* Alter flags to let later users know what we learned. */ if (external && !active) *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
I apparently forgot to commit this despite mentioning it in the commit message:
@@ -13873,8 +13873,14 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, goto cleanup; }
- /* Internal snapshots don't work with VMs with OVMF loader since qemu does - * not snapshot the variable store */ + /* 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 + * + * Avoid the issues by forbidding this completely. + */
Yeah I saw the OVMF mention in the v2 comment, despite "dropped mentions of OVMF", but I figured I wouldn't obsess about it. :) Anyway, if you post a v3 with the comment updated, you can keep my R-b with it -- the new comment looks great. Thanks! Laszlo