On Thu, Mar 23, 2017 at 10:54:04 +0100, 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.
> + */
ACK with this updated commit message.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
