On 05/17/2018 02:42 PM, John Ferlan wrote:
> Before we generate the command line for qemu, if the domain about to
> be launched desires to utilize the VM Generation ID functionality, then
> handle both the regenerating the GUID value for backup recovery (restore
> operation) and the startup after snapshot as both require a new GUID to
> be generated to allow the guest operating system to recognize the VM
> is re-executing something that has already executed before.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 17 ++++++++++++++---
> src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_process.h | 1 +
> 3 files changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b697838070..7e968f475d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6554,7 +6554,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
> if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL,
> asyncJob, "stdio", *fd, path, NULL,
> VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
> - VIR_QEMU_PROCESS_START_PAUSED) == 0)
> + VIR_QEMU_PROCESS_START_PAUSED |
> + VIR_QEMU_PROCESS_START_GEN_VMID) == 0)
> restored = true;
>
> if (intermediatefd != -1) {
> @@ -15827,6 +15828,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> compatible = qemuDomainCheckABIStability(driver, vm, config);
> }
>
> + /* If using VM GenID, there is no way currently to change
> + * the genid for the running guest, so set an error and
> + * mark as incompatible. */
> + if (compatible && config->genidRequested) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("domain genid update requires restart"));
> + compatible = false;
> + }
> +
> if (!compatible) {
> virErrorPtr err = virGetLastError();
>
> @@ -15907,7 +15917,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> cookie ? cookie->cpu : NULL,
> QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap,
> VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> - VIR_QEMU_PROCESS_START_PAUSED);
> + VIR_QEMU_PROCESS_START_PAUSED |
> + VIR_QEMU_PROCESS_START_GEN_VMID);
> virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
> event = virDomainEventLifecycleNewFromObj(vm,
> @@ -15993,7 +16004,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
> /* Flush first event, now do transition 2 or 3 */
> bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
> - unsigned int start_flags = 0;
> + unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
>
> start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 5b73a61962..f9bba69f80 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6072,6 +6072,43 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
> }
>
>
> +/**
> + * qemuProcessGenID:
> + * @vm: Pointer to domain object
> + * @flags: qemuProcessStartFlags
> + *
> + * If this domain is requesting to use genid
> + */
> +static int
> +qemuProcessGenID(virDomainObjPtr vm,
> + unsigned int flags)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> + if (!vm->def->genidRequested)
> + return 0;
> +
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VMGENID)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("this QEMU does not support the 'genid' capability"));
> + return -1;
> + }
This looks misplaced to me. Firstly, if domain is required to generate
new vmid this function should do that. Secondly, this check is missing
when generating command line.
> +
> + /* If we are coming from a path where we must provide a new gen id
> + * value regardless of whether it was previously generated or provided,
> + * then generate a new GUID value before we build the command line. */
> + if (flags & VIR_QEMU_PROCESS_START_GEN_VMID) {
> + if (virUUIDGenerate(vm->def->genid)) {
Please do explicit comparison here. It might bite us in the future when
we want virUUIDGenerate to return a positive value (which would still be
success).
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to regenerate genid"));
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +
> /**
> * qemuProcessLaunch:
> *
> @@ -6124,7 +6161,8 @@ qemuProcessLaunch(virConnectPtr conn,
> virCheckFlags(VIR_QEMU_PROCESS_START_COLD |
> VIR_QEMU_PROCESS_START_PAUSED |
> VIR_QEMU_PROCESS_START_AUTODESTROY |
> - VIR_QEMU_PROCESS_START_NEW, -1);
> + VIR_QEMU_PROCESS_START_NEW |
> + VIR_QEMU_PROCESS_START_GEN_VMID, -1);
>
> cfg = virQEMUDriverGetConfig(driver);
>
> @@ -6148,6 +6186,9 @@ qemuProcessLaunch(virConnectPtr conn,
> goto cleanup;
> logfile = qemuDomainLogContextGetWriteFD(logCtxt);
>
> + if (qemuProcessGenID(vm, flags) < 0)
> + goto cleanup;
> +
> VIR_DEBUG("Building emulator command line");
> if (!(cmd = qemuBuildCommandLine(driver,
> qemuDomainLogContextGetManager(logCtxt),
> @@ -6513,7 +6554,8 @@ qemuProcessStart(virConnectPtr conn,
>
> virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD |
> VIR_QEMU_PROCESS_START_PAUSED |
> - VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup);
> + VIR_QEMU_PROCESS_START_AUTODESTROY |
> + VIR_QEMU_PROCESS_START_GEN_VMID, cleanup);
>
> if (!migrateFrom && !snapshot)
> flags |= VIR_QEMU_PROCESS_START_NEW;
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index a0e34b1c85..5098eacfe8 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -80,6 +80,7 @@ typedef enum {
> VIR_QEMU_PROCESS_START_AUTODESTROY = 1 << 2,
> VIR_QEMU_PROCESS_START_PRETEND = 1 << 3,
> VIR_QEMU_PROCESS_START_NEW = 1 << 4, /* internal, new VM is starting */
> + VIR_QEMU_PROCESS_START_GEN_VMID = 1 << 5, /* Generate a new VMID */
I wonder if we can do without this flag. Basically, only a handful of
paths require VMID regeneration. But I haven't read older versions so
maybe you had what I'm suggesting and were told to switch to flag.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
