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

[Peter Krempa]

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.
+     */

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170323/560da15e/attachment-0001.sig>