From nobody Sun Feb 8 14:56:06 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1670506294; cv=none; d=zohomail.com; s=zohoarc; b=B9yatb/vpmEjl6zf0jqVRzD0wKXViJukyIYM3JVeYCpMk+JDO4PojbOJqtzgNbrTyZ7xI14oevvBNEOMsAjpLL593nQmUdxK7Hr88zqxUl7dYy+iQGPVvFr1u0fKj/qPLzf+oRXa5QMkpr2c2hJuIRZ2NvwGfSGylDur0U+G/AE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1670506294; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=cHYmSjWt46kQqExoeH7ZGv2A3ZbkAfe4sE6lT3uPFDM=; b=J7VZlKo9eYYNVyIEggvT5S0JoWc2KWRO1iE1i015ouFRolQqvT6SnPEhTvy/yZyqU1xmc5pDqv8FadDGOEpktZUPj65Xa5SlM4rLS58zwtet+oJxP6BcyExZ7OqpNbxYSSLCPEoTCR0OhVEfCyZTugAmzj/LD1aVhnXVctsHw6A= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1670506294424106.08362255571399; Thu, 8 Dec 2022 05:31:34 -0800 (PST) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-662-Ap9xhPX6MzmvVUiaDbSaAA-1; Thu, 08 Dec 2022 08:31:29 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C112C858F13; Thu, 8 Dec 2022 13:31:27 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 40D0717582; Thu, 8 Dec 2022 13:31:27 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 07ACE1946A72; Thu, 8 Dec 2022 13:31:27 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id CBC9C1946A41 for ; Thu, 8 Dec 2022 13:31:16 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id A5C6440C6EC5; Thu, 8 Dec 2022 13:31:16 +0000 (UTC) Received: from localhost.localdomain (ovpn-194-45.brq.redhat.com [10.40.194.45]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1D3E340C6EC2 for ; Thu, 8 Dec 2022 13:31:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670506293; h=from:from:sender:sender: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:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=cHYmSjWt46kQqExoeH7ZGv2A3ZbkAfe4sE6lT3uPFDM=; b=ie0JoD5L96hjDQXS0letbUNM9lFJVrV/WYK62ucxO36/ELUH/9jvkamGjoDr7GW9qacNeF JGPYVenmbxaQ3D+jpskwbC9wW74BMzRjFeFdJUI7B10HspF6JSHNrgYA3thR79A4M0ek6b 306XxxjZiWIuJ/N5vB1Obrtzu27J/+c= X-MC-Unique: Ap9xhPX6MzmvVUiaDbSaAA-1 X-Original-To: libvir-list@listman.corp.redhat.com From: Pavel Hrdina To: libvir-list@redhat.com Subject: [libvirt PATCH 02/30] qemu_block: extract block commit code to separate function Date: Thu, 8 Dec 2022 14:30:38 +0100 Message-Id: <4c5c2f73fcc26e9db5a00d81e1a4d88fcd0d4c9c.1670505851.git.phrdina@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libvir-list-bounces@redhat.com Sender: "libvir-list" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1670506296282100003 Content-Type: text/plain; charset="utf-8"; x-default="true" Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- src/qemu/qemu_block.c | 179 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 9 +++ src/qemu/qemu_driver.c | 162 +------------------------------------ 3 files changed, 189 insertions(+), 161 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8a6f601b29..4cca7555f3 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3196,3 +3196,182 @@ qemuBlockExportAddNBD(virDomainObj *vm, =20 return qemuMonitorBlockExportAdd(priv->mon, &nbdprops); } + + +int +qemuBlockCommit(virDomainObj *vm, + virDomainDiskDef *disk, + virStorageSource *baseSource, + virStorageSource *topSource, + virStorageSource *top_parent, + unsigned long bandwidth, + unsigned int flags) +{ + qemuDomainObjPrivate *priv =3D vm->privateData; + virQEMUDriver *driver =3D priv->driver; + int rc =3D -1; + bool clean_access =3D false; + g_autofree char *backingPath =3D NULL; + qemuBlockJobData *job =3D NULL; + g_autoptr(virStorageSource) mirror =3D NULL; + + if (virDomainObjCheckActive(vm) < 0) + return -1; + + /* Convert bandwidth MiB to bytes, if necessary */ + if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES)) { + if (bandwidth > LLONG_MAX >> 20) { + virReportError(VIR_ERR_OVERFLOW, + _("bandwidth must be less than %llu"), + LLONG_MAX >> 20); + return -1; + } + bandwidth <<=3D 20; + } + + if (!qemuDomainDiskBlockJobIsSupported(disk)) + return -1; + + if (virStorageSourceIsEmpty(disk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk %s has no source file to be committed"), + disk->dst); + return -1; + } + + if (qemuDomainDiskBlockJobIsActive(disk)) + return -1; + + if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) + return -1; + + if (topSource =3D=3D disk->src) { + /* XXX Should we auto-pivot when COMMIT_ACTIVE is not specified? */ + if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)) { + virReportError(VIR_ERR_INVALID_ARG, + _("commit of '%s' active layer requires active = flag"), + disk->dst); + return -1; + } + } else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { + virReportError(VIR_ERR_INVALID_ARG, + _("active commit requested but '%s' is not active"), + topSource->path); + return -1; + } + + if (!virStorageSourceHasBacking(topSource)) { + virReportError(VIR_ERR_INVALID_ARG, + _("top '%s' in chain for '%s' has no backing file"), + topSource->path, disk->src->path); + return -1; + } + + if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) && + baseSource !=3D topSource->backingStore) { + virReportError(VIR_ERR_INVALID_ARG, + _("base '%s' is not immediately below '%s' in chain= " + "for '%s'"), + baseSource->path, topSource->path, disk->src->path); + return -1; + } + + /* For an active commit, clone enough of the base to act as the mirror= */ + if (topSource =3D=3D disk->src) { + if (!(mirror =3D virStorageSourceCopy(baseSource, false))) + return -1; + if (virStorageSourceInitChainElement(mirror, + disk->src, + true) < 0) + return -1; + } + + if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE && + topSource !=3D disk->src) { + if (top_parent && + qemuBlockUpdateRelativeBacking(vm, top_parent, disk->src) < 0) + return -1; + + if (virStorageSourceGetRelativeBackingPath(topSource, baseSource, + &backingPath) < 0) + return -1; + + if (!backingPath) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("can't keep relative backing relationship")); + return -1; + } + } + + /* For the commit to succeed, we must allow qemu to open both the + * 'base' image and the parent of 'top' as read/write; 'top' might + * not have a parent, or might already be read-write. XXX It + * would also be nice to revert 'base' to read-only, as well as + * revoke access to files removed from the chain, when the commit + * operation succeeds, but doing that requires tracking the + * operation in XML across libvirtd restarts. */ + clean_access =3D true; + if (qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, + false, false, false) < 0) + goto error; + + if (top_parent && top_parent !=3D disk->src) { + /* While top_parent is topmost image, we don't need to remember its + * owner as it will be overwritten upon finishing the commit. Henc= e, + * pass chainTop =3D false. */ + if (qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + false, false, false) < 0) + goto error; + } + + if (!(job =3D qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSourc= e, + baseSource, + flags & VIR_DOMAIN_BLOCK_COMMIT_= DELETE, + flags))) + goto error; + + disk->mirrorState =3D VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + + if (!backingPath && top_parent && + !(backingPath =3D qemuBlockGetBackingStoreString(baseSource, false= ))) + goto error; + + qemuDomainObjEnterMonitor(vm); + + rc =3D qemuMonitorBlockCommit(priv->mon, + qemuDomainDiskGetTopNodename(disk), + job->name, + topSource->nodeformat, + baseSource->nodeformat, + backingPath, bandwidth); + + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + goto error; + + if (mirror) { + disk->mirror =3D g_steal_pointer(&mirror); + disk->mirrorJob =3D VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; + } + qemuBlockJobStarted(job, vm); + + return 0; + + error: + if (clean_access) { + virErrorPtr orig_err; + virErrorPreserveLast(&orig_err); + /* Revert access to read-only, if possible. */ + qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, + true, false, false); + if (top_parent && top_parent !=3D disk->src) + qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + true, false, false); + + virErrorRestore(&orig_err); + } + qemuBlockJobStartupFinalize(vm, job); + + return -1; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 8a3a10344e..85b4805d89 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -276,3 +276,12 @@ qemuBlockExportAddNBD(virDomainObj *vm, const char *exportname, bool writable, const char *bitmap); + +int +qemuBlockCommit(virDomainObj *vm, + virDomainDiskDef *disk, + virStorageSource *baseSource, + virStorageSource *topSource, + virStorageSource *top_parent, + unsigned long bandwidth, + unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d509582719..2f05da3d8c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15114,19 +15114,12 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned long bandwidth, unsigned int flags) { - virQEMUDriver *driver =3D dom->conn->privateData; - qemuDomainObjPrivate *priv; virDomainObj *vm =3D NULL; int ret =3D -1; virDomainDiskDef *disk =3D NULL; virStorageSource *topSource; virStorageSource *baseSource =3D NULL; virStorageSource *top_parent =3D NULL; - bool clean_access =3D false; - g_autofree char *backingPath =3D NULL; - unsigned long long speed =3D bandwidth; - qemuBlockJobData *job =3D NULL; - g_autoptr(virStorageSource) mirror =3D NULL; =20 virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | @@ -15136,7 +15129,6 @@ qemuDomainBlockCommit(virDomainPtr dom, =20 if (!(vm =3D qemuDomainObjFromDomain(dom))) goto cleanup; - priv =3D vm->privateData; =20 if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -15144,176 +15136,24 @@ qemuDomainBlockCommit(virDomainPtr dom, if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) goto cleanup; =20 - if (virDomainObjCheckActive(vm) < 0) - goto endjob; - - /* Convert bandwidth MiB to bytes, if necessary */ - if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES)) { - if (speed > LLONG_MAX >> 20) { - virReportError(VIR_ERR_OVERFLOW, - _("bandwidth must be less than %llu"), - LLONG_MAX >> 20); - goto endjob; - } - speed <<=3D 20; - } - if (!(disk =3D qemuDomainDiskByName(vm->def, path))) goto endjob; =20 - if (!qemuDomainDiskBlockJobIsSupported(disk)) - goto endjob; - - if (virStorageSourceIsEmpty(disk->src)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk %s has no source file to be committed"), - disk->dst); - goto endjob; - } - - if (qemuDomainDiskBlockJobIsActive(disk)) - goto endjob; - - if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) - goto endjob; - if (!top || STREQ(top, disk->dst)) topSource =3D disk->src; else if (!(topSource =3D virStorageSourceChainLookup(disk->src, NULL, = top, disk->dst, &top_par= ent))) goto endjob; =20 - if (topSource =3D=3D disk->src) { - /* XXX Should we auto-pivot when COMMIT_ACTIVE is not specified? */ - if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)) { - virReportError(VIR_ERR_INVALID_ARG, - _("commit of '%s' active layer requires active = flag"), - disk->dst); - goto endjob; - } - } else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { - virReportError(VIR_ERR_INVALID_ARG, - _("active commit requested but '%s' is not active"), - topSource->path); - goto endjob; - } - - if (!virStorageSourceHasBacking(topSource)) { - virReportError(VIR_ERR_INVALID_ARG, - _("top '%s' in chain for '%s' has no backing file"), - topSource->path, path); - goto endjob; - } - if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) baseSource =3D topSource->backingStore; else if (!(baseSource =3D virStorageSourceChainLookup(disk->src, topSo= urce, base, disk->dst, N= ULL))) goto endjob; =20 - if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) && - baseSource !=3D topSource->backingStore) { - virReportError(VIR_ERR_INVALID_ARG, - _("base '%s' is not immediately below '%s' in chain= " - "for '%s'"), - base, topSource->path, path); - goto endjob; - } - - /* For an active commit, clone enough of the base to act as the mirror= */ - if (topSource =3D=3D disk->src) { - if (!(mirror =3D virStorageSourceCopy(baseSource, false))) - goto endjob; - if (virStorageSourceInitChainElement(mirror, - disk->src, - true) < 0) - goto endjob; - } - - if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE && - topSource !=3D disk->src) { - if (top_parent && - qemuBlockUpdateRelativeBacking(vm, top_parent, disk->src) < 0) - goto endjob; - - if (virStorageSourceGetRelativeBackingPath(topSource, baseSource, - &backingPath) < 0) - goto endjob; - - if (!backingPath) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("can't keep relative backing relationship")); - goto endjob; - } - } - - /* For the commit to succeed, we must allow qemu to open both the - * 'base' image and the parent of 'top' as read/write; 'top' might - * not have a parent, or might already be read-write. XXX It - * would also be nice to revert 'base' to read-only, as well as - * revoke access to files removed from the chain, when the commit - * operation succeeds, but doing that requires tracking the - * operation in XML across libvirtd restarts. */ - clean_access =3D true; - if (qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, - false, false, false) < 0) - goto endjob; - - if (top_parent && top_parent !=3D disk->src) { - /* While top_parent is topmost image, we don't need to remember its - * owner as it will be overwritten upon finishing the commit. Henc= e, - * pass chainTop =3D false. */ - if (qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, - false, false, false) < 0) - goto endjob; - } - - if (!(job =3D qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSourc= e, - baseSource, - flags & VIR_DOMAIN_BLOCK_COMMIT_= DELETE, - flags))) - goto endjob; - - disk->mirrorState =3D VIR_DOMAIN_DISK_MIRROR_STATE_NONE; - - if (!backingPath && top_parent && - !(backingPath =3D qemuBlockGetBackingStoreString(baseSource, false= ))) - goto endjob; - - qemuDomainObjEnterMonitor(vm); - - ret =3D qemuMonitorBlockCommit(priv->mon, - qemuDomainDiskGetTopNodename(disk), - job->name, - topSource->nodeformat, - baseSource->nodeformat, - backingPath, speed); - - qemuDomainObjExitMonitor(vm); - - if (ret < 0) - goto endjob; - - if (mirror) { - disk->mirror =3D g_steal_pointer(&mirror); - disk->mirrorJob =3D VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; - } - qemuBlockJobStarted(job, vm); + ret =3D qemuBlockCommit(vm, disk, baseSource, topSource, top_parent, b= andwidth, flags); =20 endjob: - if (ret < 0 && clean_access) { - virErrorPtr orig_err; - virErrorPreserveLast(&orig_err); - /* Revert access to read-only, if possible. */ - qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, - true, false, false); - if (top_parent && top_parent !=3D disk->src) - qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, - true, false, false); - - virErrorRestore(&orig_err); - } - qemuBlockJobStartupFinalize(vm, job); virDomainObjEndJob(vm); =20 cleanup: --=20 2.38.1