From nobody Wed Nov 27 04:15:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1731660253894286.7178635427092; Fri, 15 Nov 2024 00:44:13 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id CD6B71828; Fri, 15 Nov 2024 03:44:12 -0500 (EST) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 62E5A1A60; Fri, 15 Nov 2024 03:40:09 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id 8339119E1; Fri, 15 Nov 2024 03:40:03 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 141E617BE for ; Fri, 15 Nov 2024 03:39:49 -0500 (EST) Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-674-1_4KtNAGN2eyyNffyGfQQQ-1; Fri, 15 Nov 2024 03:39:46 -0500 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id A6E691944EAE for ; Fri, 15 Nov 2024 08:39:45 +0000 (UTC) Received: from speedmetal.lan (unknown [10.45.242.6]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id D2C7C1953880 for ; Fri, 15 Nov 2024 08:39:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1731659988; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UOMKOgUQsTreemoXppJb/jfU3jo4ncWchH33+RYh1ws=; b=K85zQ2hegl+RLhTXiaWpu4OK9Gl6h7oC2xlrp9E3Jh1dcwVs/iOIqOOYVAwPdZ+hygrqjo BoE1lzPirPDQb47SdshbOBFmW7uGCSdkAeWBt0L8xSsPucUE4tFLPjdxiZ8kPo3uz4UFVy pBlK3FQoiWbHaN0EaibwKWl44no6jqQ= X-MC-Unique: 1_4KtNAGN2eyyNffyGfQQQ-1 X-Mimecast-MFC-AGG-ID: 1_4KtNAGN2eyyNffyGfQQQ From: Peter Krempa To: devel@lists.libvirt.org Subject: [PATCH 9/9] qemu: Avoid use of '-loadvm' commandline argument for internal snapshot reversion Date: Fri, 15 Nov 2024 09:39:30 +0100 Message-ID: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: jkD8_tgyZ-nPQA60EC6u0TV4-EMClcH-OWkB0wkNLns_1731659985 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Message-ID-Hash: EK655I4LD5J5UKRNJUOPM2Q6AXV4SP6B X-Message-ID-Hash: EK655I4LD5J5UKRNJUOPM2Q6AXV4SP6B X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1731660255226116600 Content-Type: text/plain; charset="utf-8" The '-loadvm' commandline parameter has exactly the same semantics as the HMP 'loadvm' command. This includes the selection of which block device is considered to contain the 'vmstate' section. Since libvirt recently switched to the new QMP commands which allow a free selection of where the 'vmstate' is placed, snapshot reversion will no longer work if libvirt's algorithm disagrees with qemu's. This is the case when the VM has UEFI NVRAM image, in qcow2 format, present. To solve this we'll use the QMP counterpart 'snapshot-load' to load the snapshot instead of using '-loadvm'. We'll do this before resuming processors after startup of qemu and thus the behaviour is identical to what we had before. The logic for selecting the images now checks both the snapshot metadata and the VM definition. In case images not covered by the snapshot definition do have the snapshot it's included in the reversion, but it's fatal if the snapshot is not present in a disk covered in snapshot metadata. The vmstate is selected based on where it's present as libvirt doesn't store this information. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 5 +- src/qemu/qemu_process.c | 7 ++ src/qemu/qemu_snapshot.c | 232 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_snapshot.h | 5 + 4 files changed, 248 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 696f891b47..f4430275dc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10567,7 +10567,10 @@ qemuBuildCommandLine(virDomainObj *vm, if (qemuBuildSecCommandLine(vm, cmd, def->sec) < 0) return NULL; - if (snapshot) + /* Internal snapshot reversion happens via QMP command after startup if + * supported */ + if (snapshot && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SNAPSHOT_INTERNAL_QMP)) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); if (def->namespaceData) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0815bffe3c..1fda8bdd65 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7944,6 +7944,13 @@ qemuProcessLaunch(virConnectPtr conn, qemuDomainVcpuPersistOrder(vm->def); + if (snapshot && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_INTERNAL_QMP)) { + VIR_DEBUG("reverting internal snapshot via QMP"); + if (qemuSnapshotInternalRevert(vm, snapshot, asyncJob) < 0) + goto cleanup; + } + VIR_DEBUG("Verifying and updating provided guest CPU"); if (qemuProcessUpdateAndVerifyCPU(vm, asyncJob) < 0) goto cleanup; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index aab06a09c6..5b3aadcbf0 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -4361,3 +4361,235 @@ qemuSnapshotDelete(virDomainObj *vm, return ret; } + + +static char ** +qemuSnapshotInternalRevertGetDevices(virDomainObj *vm, + virDomainSnapshotDef *snapdef, + char **vmstate, + virDomainAsyncJob asyncJob) + +{ + g_autoptr(GHashTable) blockNamedNodeData =3D NULL; + const char *snapname =3D snapdef->parent.name; + g_auto(GStrv) devices =3D g_new0(char *, vm->def->ndisks + 2); + size_t ndevs =3D 0; + size_t i =3D 0; + const char *vmstate_candidate =3D NULL; + g_autoptr(GHashTable) snapdisks =3D virHashNew(NULL); + /* following variables add debug information */ + g_auto(virBuffer) errExtraSnap =3D VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errSnapWithoutMetadata =3D VIR_BUFFER_INITIALIZER; + + if (!(blockNamedNodeData =3D qemuBlockGetNamedNodeData(vm, asyncJob))) + return NULL; + + /* Look up snapshot data from the snapshot object config itself */ + for (i =3D 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk =3D snapdef->disks + i; + virDomainDiskDef *domdisk =3D virDomainDiskByTarget(vm->def, snapd= isk->name); + const char *format_nodename; + qemuBlockNamedNodeData *d =3D NULL; + qemuBlockNamedNodeDataSnapshot *sn =3D NULL; + + if (!domdisk) { + /* This can happen only if the snapshot metadata doesn't match= the + * domain XML stored in the snapshot */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("VM doesn't have disk '%1$s' referenced by sn= apshot '%2$s'"), + snapdisk->name, snapname); + return NULL; + } + + /* later we'll check if all disks from the VM definition XML were = considered */ + g_hash_table_insert(snapdisks, g_strdup(snapdisk->name), NULL); + + format_nodename =3D qemuBlockStorageSourceGetFormatNodename(domdis= k->src); + + /* Internal snapshots require image format which supports them, th= us + * this effectively rejects any raw images */ + if (format_nodename) + d =3D g_hash_table_lookup(blockNamedNodeData, format_nodename); + + if (d && d->snapshots) + sn =3D g_hash_table_lookup(d->snapshots, snapname); + + if (sn) { + if (sn->vmstate) { + if (vmstate_candidate) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("two disks images contain vm state se= ction for internal snapshot '%1$s'"), + snapname); + return NULL; + } + vmstate_candidate =3D format_nodename; + } + + devices[ndevs++] =3D g_strdup(format_nodename); + } + + switch (snapdisk->snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + if (!sn) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("image of disk '%1$s' does not have inter= nal snapshot '%2$s'"), + snapdisk->name, snapname); + return NULL; + } + + break; + + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + if (sn) { + /* Unexpected internal snapshot present in image even if t= he + * snapshot metadata claims otherwise */ + virBufferAsprintf(&errExtraSnap, "%s ", snapdisk->name); + } + break; + } + } + + /* check if all VM disks were covered */ + for (i =3D 0; i < vm->def->ndisks; i++) { + virDomainDiskDef *domdisk =3D vm->def->disks[i]; + const char *format_nodename; + qemuBlockNamedNodeData *d =3D NULL; + qemuBlockNamedNodeDataSnapshot *sn =3D NULL; + + if (g_hash_table_contains(snapdisks, domdisk->dst)) + continue; + + format_nodename =3D qemuBlockStorageSourceGetFormatNodename(domdis= k->src); + + if (format_nodename) + d =3D g_hash_table_lookup(blockNamedNodeData, format_nodename); + + if (d && d->snapshots) + sn =3D g_hash_table_lookup(d->snapshots, snapname); + + if (sn) { + virBufferAsprintf(&errSnapWithoutMetadata, "%s ", domdisk->dst= ); + + if (sn->vmstate) { + if (vmstate_candidate) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("two disks images contain vm state se= ction for internal snapshot '%1$s'"), + snapname); + return NULL; + } + vmstate_candidate =3D format_nodename; + } + + devices[ndevs++] =3D g_strdup(format_nodename); + } + } + + /* pflash */ + if (vm->def->os.loader && + vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format =3D=3D VIR_STORAGE_FILE_QCOW2) { + const char *format_nodename; + qemuBlockNamedNodeData *d =3D NULL; + qemuBlockNamedNodeDataSnapshot *sn =3D NULL; + + if ((format_nodename =3D qemuBlockStorageSourceGetFormatNodename(v= m->def->os.loader->nvram)) && + (d =3D virHashLookup(blockNamedNodeData, format_nodename)) && + d->snapshots && + (sn =3D g_hash_table_lookup(d->snapshots, snapname))) { + if (sn->vmstate) { + if (vmstate_candidate) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("two disks images contain vm state se= ction for internal snapshot '%1$s'"), + snapname); + return NULL; + } + + vmstate_candidate =3D format_nodename; + } + + devices[ndevs++] =3D g_strdup(format_nodename); + } + } + + if (virBufferUse(&errExtraSnap) > 0 || + virBufferUse(&errSnapWithoutMetadata) > 0) { + VIR_WARN("inconsistent internal snapshot state (reversion): VM=3D'= %s' snapshot=3D'%s' no-snapshot=3D'%s' no-metadata=3D'%s'", + vm->def->name, snapname, + virBufferCurrentContent(&errExtraSnap), + virBufferCurrentContent(&errSnapWithoutMetadata)); + } + + *vmstate =3D g_strdup(vmstate_candidate); + return g_steal_pointer(&devices); +} + + +int +qemuSnapshotInternalRevert(virDomainObj *vm, + virDomainMomentObj *snapshot, + virDomainAsyncJob asyncJob) +{ + virDomainSnapshotDef *snapdef =3D virDomainSnapshotObjGetDef(snapshot); + g_autofree char *jobname =3D g_strdup_printf("internal-snapshot-load-%= s", snapdef->parent.name); + qemuBlockJobData *job =3D NULL; + g_auto(GStrv) devices =3D NULL; + g_autofree char *vmstate =3D NULL; + int rc =3D 0; + int ret =3D -1; + + if (!(devices =3D qemuSnapshotInternalRevertGetDevices(vm, snapdef, &v= mstate, asyncJob))) + return -1; + + if (!vmstate) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("missing vmstate section when reverting active in= ternal snapshot '%1$s'"), + snapshot->def->name); + return -1; + } + + if (!(job =3D qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHO= T_LOAD, + jobname))) + return -1; + + qemuBlockJobSyncBegin(job); + + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) + goto cleanup; + + rc =3D qemuMonitorSnapshotLoad(qemuDomainGetMonitor(vm), jobname, snap= def->parent.name, + vmstate, (const char **) devices); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + goto cleanup; + + qemuBlockJobStarted(job, vm); + + while (true) { + qemuBlockJobUpdate(vm, job, asyncJob); + + if (job->state =3D=3D VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("load of internal snapshot '%1$s' job failed:= %2$s"), + snapdef->parent.name, NULLSTR(job->errmsg)); + goto cleanup; + } + + if (job->state =3D=3D VIR_DOMAIN_BLOCK_JOB_COMPLETED) + break; + + if (qemuDomainObjWait(vm) < 0) + goto cleanup; + } + + ret =3D 0; + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + + return ret; +} diff --git a/src/qemu/qemu_snapshot.h b/src/qemu/qemu_snapshot.h index 38437a2fd7..f38c2acfb3 100644 --- a/src/qemu/qemu_snapshot.h +++ b/src/qemu/qemu_snapshot.h @@ -84,3 +84,8 @@ qemuSnapshotDiskCreate(qemuSnapshotDiskContext *snapctxt); virDomainSnapshotDiskDef * qemuSnapshotGetTransientDiskDef(virDomainDiskDef *domdisk, const char *suffix); + +int +qemuSnapshotInternalRevert(virDomainObj *vm, + virDomainMomentObj *snapshot, + virDomainAsyncJob asyncJob); --=20 2.47.0