Re: [libvirt] [PATCH v8 16/21] backup: qemu: Implement metadata tracking for checkpoint APIs
On 4/26/19 8:42 AM, Peter Krempa wrote:
On Wed, Apr 17, 2019 at 09:09:16 -0500, Eric Blake wrote:
> A lot of this work heavily copies from the existing snapshot
> APIs. The interaction with qemu during create/delete still
> needs to be implemented, but this takes care of all the libvirt
> metadata (saving and restoring XML, and tracking the relations
> between multiple checkpoints).
I'd prefer at least the 'qemu_conf' related stuff to be separate.
> @@ -242,6 +244,8 @@ virQEMUDriverConfigPtr
virQEMUDriverConfigNew(bool privileged)
> goto error;
> if (virAsprintf(&cfg->snapshotDir, "%s/qemu/snapshot",
cfg->configBaseDir) < 0)
> goto error;
> + if (virAsprintf(&cfg->checkpointDir, "%s/qemu/checkpoint",
cfg->configBaseDir) < 0)
> + goto error;
This directory will need to be versionable as we need to record
alternate histories when creating snapshots. I'm not sure how to achieve
thant though. We'll probably need to add a directory into 'snapshot'
folder for each snapshot which will copy the whole checkpoint tree.
No, we don't need versioned directories. Rather, we simply rely on the
fact that checkpoints, like snapshots, are already stored as a tree
relationship, where one checkpoint can have more than one child where
those multiple children are created across the original snapshot vs. the
revert to the snapshot. We may need one extra XML line in snapshots to
record which checkpoint to use as parent of any child created during a
revert, but that should be sufficient.
> + /* Easiest way to clone inactive portion of vm->def is
via
> + * conversion in and back out of xml. */
> + if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
> + true, true)) ||
> + !(def->common.dom = virDomainDefParseString(xml, caps, driver->xmlopt,
NULL,
> + VIR_DOMAIN_DEF_PARSE_INACTIVE |
> +
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
> + goto cleanup;
> +
> + if (virDomainCheckpointAlignDisks(def) < 0 ||
> + qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
This is always done when starting the qemu instance and on every update,
thus should not be necessary.
In my initial demos, I was running into instances where it was not
happening correctly. But if I can remove it and things work, all the better.
Also note that if necessary it must be guarded by
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) &&
With blockdev we shouldn't ever do that.
> +static int
> +qemuDomainHasCurrentCheckpoint(virDomainPtr domain,
> + unsigned int flags)
This API seems a bit pointless if you can call
qemuDomainCheckpointCurrent.
It mirrors what snapshots provide. But I can always try to factor it out
into a separate patch, and add it back in only if we need it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)