From nobody Fri Apr 19 04:18:32 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=gmail.com ARC-Seal: i=1; a=rsa-sha256; t=1600137809; cv=none; d=zohomail.com; s=zohoarc; b=MD0IbUS4a+h5tl2pFsO9YtmoEqzmu+o7o+oOxWqFgenpYv9ykbcEv1hNVnVdpAS0wLCBcMSiVqgSqbK7u0rhGIMoXwQYnD7Io7ii4eYFKy2ZfI7KlpYxLPIeBtHwWckBB5Ne26o9CRl4Ga2uHrsAPEBd7hLpmsqs1upLrqcVqmg= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1600137809; h=Content-Type:Content-Transfer-Encoding:Cc: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=YWpxVgfNEHgvGHCY519hWUZMlvwP64bBF5wA1/JfDvo=; b=D8hwV4KfBlR1ZoffC/uRS4X6JqiUHs1stFuQJVWWQ6IcisRvIQsVO4TE4mMHZYqO8ATA/AXT3qulu/Jnj4htYwaYDVjRAJhwn/vJWrBgAyZzN01aFEO/pUfFeLK92VGGeslDSFgnKwrN1VhSPzViK3eNKQ+5Ffav/TAlfXmWjNg= ARC-Authentication-Results: i=1; mx.zohomail.com; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1600137809896669.8961486936649; Mon, 14 Sep 2020 19:43:29 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-63-C0uXV4HaN8KZzpT3lav03Q-1; Mon, 14 Sep 2020 22:43:26 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 53B04801AAB; Tue, 15 Sep 2020 02:43:21 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5F4351002393; Tue, 15 Sep 2020 02:43:19 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 2E52C1832FF6; Tue, 15 Sep 2020 02:43:16 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 08F2hEd8019533 for ; Mon, 14 Sep 2020 22:43:14 -0400 Received: by smtp.corp.redhat.com (Postfix) id A48791004C7E; Tue, 15 Sep 2020 02:43:14 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast03.extmail.prod.ext.rdu2.redhat.com [10.11.55.19]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9FB781006501 for ; Tue, 15 Sep 2020 02:43:14 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4483B803FFA for ; Tue, 15 Sep 2020 02:43:14 +0000 (UTC) Received: from mail-qv1-f45.google.com (mail-qv1-f45.google.com [209.85.219.45]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-528-PZcEwzorNK6wgx-BULN3Zg-1; Mon, 14 Sep 2020 22:43:12 -0400 Received: by mail-qv1-f45.google.com with SMTP id di5so1034356qvb.13 for ; Mon, 14 Sep 2020 19:43:12 -0700 (PDT) Received: from localhost.localdomain ([2804:431:c7c7:5342:27d9:20da:5ada:61cf]) by smtp.gmail.com with ESMTPSA id v16sm14897324qkg.37.2020.09.14.19.43.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Sep 2020 19:43:10 -0700 (PDT) X-MC-Unique: C0uXV4HaN8KZzpT3lav03Q-1 X-MC-Unique: PZcEwzorNK6wgx-BULN3Zg-1 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=YWpxVgfNEHgvGHCY519hWUZMlvwP64bBF5wA1/JfDvo=; b=Y/BOVpZI9NlZq8fz1SWWyDRzbcaJkATNebFJDG6c5XxeplJZpzef/nPm9rPXGilvj4 VYiHPnMY8mnzB7bZWNwnvGw+02GBa/Ts/RX4ENTSxP8isNsdigZS/FeFnFjNh3jtV8ML OKoUFDm6794w2CPCakV/fjemZgx0WAnuOp0W58WKtp6FhReBd+/dHKU5E6KPecEu6sP8 V9QFSsGfpW4y/R/b4UpW2+/aQFaW8CZGhV1K8Kj3faxUcGFBOlyEKY76mStlCAeRIoS2 ehZ00Lkb03asYto4KO6JvcCMbqyRjeuuAh+EaMFO+Gto5Eq5+gJB1cbFXb5ux/IrKdvC 3vpg== X-Gm-Message-State: AOAM5324UOnJ8N/jqbBQUOoFOFkZvvlQl2v6++aFxRRegfaXd8DxEOHs hY9u9MFDSM846uWfMWfAY6j5Q75Lc24= X-Google-Smtp-Source: ABdhPJwTKg6ArXSXXEI5LdueEF7nJjRbgo5TKZ2n9N7kcmrIzC8nUNtbHUUzzFsm9biDiL2R6G7wXw== X-Received: by 2002:a05:6214:5c8:: with SMTP id t8mr16849827qvz.86.1600137791129; Mon, 14 Sep 2020 19:43:11 -0700 (PDT) From: Daniel Henrique Barboza To: libvir-list@redhat.com Subject: [PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes Date: Mon, 14 Sep 2020 23:42:56 -0300 Message-Id: <20200915024259.193494-2-danielhb413@gmail.com> In-Reply-To: <20200915024259.193494-1-danielhb413@gmail.com> References: <20200915024259.193494-1-danielhb413@gmail.com> MIME-Version: 1.0 X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false; X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-loop: libvir-list@redhat.com Cc: Daniel Henrique Barboza X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0.002 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" In [1], changes were made to remove the existing auto-alignment for pSeries NVDIMM devices. That design promotes strange situations where the NVDIMM size reported in the domain XML is different from what QEMU is actually using. We removed the auto-alignment and relied on standard size validation. However, this goes against Libvirt design philosophy of not tampering with existing guest behavior, as pointed out by Daniel in [2]. Since we can't know for sure whether there are guests that are relying on the auto-alignment feature to work, the changes made in [1] are a direct violation of this rule. This patch reverts [1] entirely, re-enabling auto-alignment for pSeries NVDIMM as it was before. Changes will be made to ease the limitations of this design without hurting existing guests. This reverts the following commits: - commit 0ee56369c8b4f2f898b6aa1ff1f51ab033be1c02, "qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type". - commit 07de813924caf37e535855541c0c1183d9d382e2, "qemu_domain.c: do not auto-align ppc64 NVDIMMs" - commit 0ccceaa57c50e5ee528f7073fa8723afd62b88b7, "qemu_validate.c: add pSeries NVDIMM size alignment validation" - commit 4fa2202d884414ad34d9952e72fb39b1d93c7e14, "qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public" - commit 2d93cbdea9d1b8dbf36bc0ffee6cb73d83d208c7, "Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic= "" [1] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html [2] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html --- docs/formatdomain.rst | 6 +- src/qemu/qemu_domain.c | 57 +++++++++++++++++-- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_hotplug.c | 6 +- src/qemu/qemu_validate.c | 42 ++------------ ...ory-hotplug-nvdimm-ppc64.ppc64-latest.args | 2 +- .../memory-hotplug-nvdimm-ppc64.xml | 2 +- .../memory-hotplug-nvdimm-ppc64.xml | 2 +- 8 files changed, 70 insertions(+), 53 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 18e237c157..d5930a4ac1 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7216,8 +7216,10 @@ Example: usage of the memory devices following restrictions apply: =20 #. the minimum label size is 128KiB, - #. the remaining size (total-size - label-size) will be aligned - to 4KiB as default. + #. the remaining size (total-size - label-size), also called guest a= rea, + will be aligned to 4KiB as default. For pSeries guests, the guest= area + will be aligned down to 256MiB, and the minimum size of the guest= area + must be at least 256MiB. =20 ``readonly`` The ``readonly`` element is used to mark the vNVDIMM as read-only. O= nly diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 03917cf000..4f796bef4a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8037,7 +8037,7 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, } =20 =20 -unsigned long long +static unsigned long long qemuDomainGetMemorySizeAlignment(const virDomainDef *def) { /* PPC requires the memory sizes to be rounded to 256MiB increments, so @@ -8067,6 +8067,44 @@ qemuDomainGetMemoryModuleSizeAlignment(const virDoma= inDef *def, } =20 =20 +static int +qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + /* For NVDIMMs in ppc64 in we want to align down the guest + * visible space, instead of align up, to avoid writing + * beyond the end of file by adding a potential 256MiB + * to the user specified size. + * + * The label-size is mandatory for ppc64 as well, meaning that + * the guest visible space will be target_size-label_size. + * + * Finally, target_size must include label_size. + * + * The above can be summed up as follows: + * + * target_size =3D AlignDown(target_size - label_size) + label_size + */ + unsigned long long ppc64AlignSize =3D qemuDomainGetMemorySizeAlignment= (def); + unsigned long long guestArea =3D mem->size - mem->labelsize; + + /* Align down guest_area. 256MiB is the minimum size. Error + * out if target_size is smaller than 256MiB + label_size, + * since aligning it up will cause QEMU errors. */ + if (mem->size < (ppc64AlignSize + mem->labelsize)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("minimum target size for the NVDIMM " + "must be 256MB plus the label size")); + return -1; + } + + guestArea =3D (guestArea/ppc64AlignSize) * ppc64AlignSize; + mem->size =3D guestArea + mem->labelsize; + + return 0; +} + + int qemuDomainAlignMemorySizes(virDomainDefPtr def) { @@ -8113,8 +8151,11 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) =20 /* Align memory module sizes */ for (i =3D 0; i < def->nmems; i++) { - if (!(def->mems[i]->model =3D=3D VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - ARCH_IS_PPC64(def->os.arch))) { + if (def->mems[i]->model =3D=3D VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(def->os.arch)) { + if (qemuDomainNVDimmAlignSizePseries(def, def->mems[i]) < 0) + return -1; + } else { align =3D qemuDomainGetMemoryModuleSizeAlignment(def, def->mem= s[i]); def->mems[i]->size =3D VIR_ROUND_UP(def->mems[i]->size, align); } @@ -8143,15 +8184,19 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) * inplace. Default rounding is now to 1 MiB (qemu requires rounding to pa= ge, * size so this should be safe). */ -void +int qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, virDomainMemoryDefPtr mem) { - if (!(mem->model =3D=3D VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - ARCH_IS_PPC64(def->os.arch))) { + if (mem->model =3D=3D VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(def->os.arch)) { + return qemuDomainNVDimmAlignSizePseries(def, mem); + } else { mem->size =3D VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def)); } + + return 0; } =20 =20 diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index adba79aded..e6d4acb8d4 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -751,11 +751,9 @@ bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPt= r disk); bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only) ATTRIBUTE_NONNULL(1); =20 -unsigned long long qemuDomainGetMemorySizeAlignment(const virDomainDef *de= f); - int qemuDomainAlignMemorySizes(virDomainDefPtr def); -void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, - virDomainMemoryDefPtr mem); +int qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, + virDomainMemoryDefPtr mem); =20 virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def); =20 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e2c6e14c2e..3a780d00af 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2368,7 +2368,8 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, int id; int ret =3D -1; =20 - qemuDomainMemoryDeviceAlignSize(vm->def, mem); + if (qemuDomainMemoryDeviceAlignSize(vm->def, mem) < 0) + goto cleanup; =20 if (qemuDomainDefValidateMemoryHotplug(vm->def, priv->qemuCaps, mem) <= 0) goto cleanup; @@ -5612,7 +5613,8 @@ qemuDomainDetachPrepMemory(virDomainObjPtr vm, virDomainMemoryDefPtr mem; int idx; =20 - qemuDomainMemoryDeviceAlignSize(vm->def, match); + if (qemuDomainMemoryDeviceAlignSize(vm->def, match) < 0) + return -1; =20 if ((idx =3D virDomainMemoryFindByDef(vm->def, match)) < 0) { virReportError(VIR_ERR_DEVICE_MISSING, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 070f1c962b..c56ad70623 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4034,45 +4034,15 @@ qemuValidateDomainDeviceDefHub(virDomainHubDefPtr h= ub, } =20 =20 -static unsigned long long -qemuValidateGetNVDIMMAlignedSizePseries(virDomainMemoryDefPtr mem, - const virDomainDef *def) -{ - unsigned long long ppc64AlignSize =3D qemuDomainGetMemorySizeAlignment= (def); - unsigned long long guestArea =3D mem->size - mem->labelsize; - - /* NVDIMM is already aligned */ - if (guestArea % ppc64AlignSize =3D=3D 0) - return mem->size; - - /* Suggested aligned size is rounded up */ - guestArea =3D (guestArea/ppc64AlignSize + 1) * ppc64AlignSize; - return guestArea + mem->labelsize; -} - static int qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, - const virDomainDef *def, virQEMUCapsPtr qemuCaps) { - if (mem->model =3D=3D VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvdimm isn't supported by this QEMU binary")= ); - return -1; - } - - if (qemuDomainIsPSeries(def)) { - unsigned long long alignedNVDIMMSize =3D - qemuValidateGetNVDIMMAlignedSizePseries(mem, def); - - if (mem->size !=3D alignedNVDIMMSize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("nvdimm size is not aligned. Suggested al= igned " - "size: %llu KiB"), alignedNVDIMMSize); - return -1; - } - } + if (mem->model =3D=3D VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvdimm isn't supported by this QEMU binary")); + return -1; } =20 return 0; @@ -4190,7 +4160,7 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef = *dev, break; =20 case VIR_DOMAIN_DEVICE_MEMORY: - ret =3D qemuValidateDomainDeviceDefMemory(dev->data.memory, def, q= emuCaps); + ret =3D qemuValidateDomainDeviceDefMemory(dev->data.memory, qemuCa= ps); break; =20 case VIR_DOMAIN_DEVICE_LEASE: diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-lates= t.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.ar= gs index 58e3f9e161..eff80dcf80 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.args +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.args @@ -19,7 +19,7 @@ file=3D/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -smp 2,sockets=3D2,dies=3D1,cores=3D1,threads=3D1 \ -numa node,nodeid=3D0,cpus=3D0-1,mem=3D1024 \ -object memory-backend-file,id=3Dmemnvdimm0,prealloc=3Dyes,mem-path=3D/tmp= /nvdimm,\ -size=3D805437440 \ +size=3D537001984 \ -device nvdimm,node=3D0,label-size=3D131072,\ uuid=3D49545eb3-75e1-2d0a-acdd-f0294406c99e,memdev=3Dmemnvdimm0,id=3Dnvdim= m0,slot=3D0 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml b/tests= /qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml index 10c146e8cf..ae5a17d3c8 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml @@ -38,7 +38,7 @@ /tmp/nvdimm - 786560 + 550000 0