Re: [libvirt] [PATCH v2] qemu: snapshot: Forbid internal snapshots with pflash firmware

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; >