2:29 a.m.
Patch 2/2 is where the actual logic is happening. It is also depending
on the following QEMU patch:
https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01979.html
Once merged I'll replace those two occurrences of 'QEMU commit XXX' with
proper commit hash.
Michal Prívozník (2):
qemu_capabilities: Introduce
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
qemu: Do not Use canonical path for system memory
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 30 ++++++++++++++++---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
.../caps_4.0.0.aarch64.xml | 1 +
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
.../caps_4.0.0.riscv32.xml | 1 +
.../caps_4.0.0.riscv64.xml | 1 +
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
.../caps_4.0.0.x86_64.xml | 1 +
.../caps_4.1.0.x86_64.xml | 1 +
.../caps_4.2.0.aarch64.xml | 1 +
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
.../caps_4.2.0.x86_64.xml | 1 +
.../caps_5.0.0.aarch64.xml | 1 +
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
.../caps_5.0.0.riscv64.xml | 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.aarch64.xml | 1 +
.../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
.../caps_5.2.0.riscv64.xml | 1 +
.../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../hugepages-memaccess3.x86_64-latest.args | 4 +--
28 files changed, 56 insertions(+), 8 deletions(-)
--
2.26.2
2:29 a.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
This capability tracks whether memory-backend-file has
"x-use-canonical-path-for-ramblock-id" attribute. Introduced into
QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered
experimental or internal to QEMU, the next commit justifies its
use.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
24 files changed, 25 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d132defbd..9850693000 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -609,6 +609,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"ncr53c90",
"dc390",
"am53c974",
+ "memory-backend-file.x-use-canonical-path-for-ramblock-id",
);
@@ -1665,6 +1666,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsMemoryBackendFile[] =
{ "discard-data", QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD },
{ "align", QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN },
{ "pmem", QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM },
+ { "x-use-canonical-path-for-ramblock-id",
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 0f90efa459..16639b416e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -589,6 +589,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
QEMU_CAPS_SCSI_NCR53C90, /* built-in SCSI */
QEMU_CAPS_SCSI_DC390, /* -device dc-390 */
QEMU_CAPS_SCSI_AM53C974, /* -device am53c974 */
+ QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID, /* -object
memory-backend-file,x-use-canonical-path-for-ramblock-id= */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
index 4722557eaf..e4ce221a1a 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
@@ -180,6 +180,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>61700240</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index 6f549902ca..02e8a7c921 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -188,6 +188,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>42900240</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
index b1dc08eb4d..7f2d900752 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
@@ -181,6 +181,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
index babb8fb8ab..0f1df27bbf 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
@@ -181,6 +181,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index 5a15848f88..f67b5027b0 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
@@ -144,6 +144,7 @@
<flag name='migration-param.downtime'/>
<flag name='migration-param.xbzrle-cache-size'/>
<flag name='fsdev.createmode'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100240</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
index 43b70ccc94..5db02c1baf 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
@@ -225,6 +225,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100240</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index 58774fddcc..036dfd732e 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -231,6 +231,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4001000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
index 77fdc73415..35b8bb8904 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
@@ -193,6 +193,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4001050</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>61700242</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
index 15eaac77a6..0e83bf9ca9 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
@@ -194,6 +194,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4001050</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>42900242</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
index 42a7cca50a..e7d02d52a6 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
@@ -155,6 +155,7 @@
<flag name='migration-param.xbzrle-cache-size'/>
<flag name='blockdev-hostdev-scsi'/>
<flag name='fsdev.createmode'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100242</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
index 1ba8c09374..b2e1f73494 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
@@ -240,6 +240,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>4002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100242</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
index d584642bff..d297a35e62 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
@@ -203,6 +203,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>61700241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
index 596bccd70a..fda8ec2c78 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
@@ -212,6 +212,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>42900241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml
b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml
index eb760f2911..f3cac396ca 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml
@@ -199,6 +199,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
index 849727eb40..aaf1e8c6cc 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
@@ -247,6 +247,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml
b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml
index a293437850..704ca704e0 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml
@@ -115,6 +115,7 @@
<flag name='netdev.vhost-vdpa'/>
<flag name='fsdev.createmode'/>
<flag name='ncr53c90'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5001000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
index ff5f42a563..6b2744e2e7 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
@@ -249,6 +249,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5001000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100242</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml
index cac9b40528..3cc7412b75 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml
@@ -207,6 +207,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>61700243</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml
index e92201ad43..62a25a5608 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml
@@ -214,6 +214,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>42900243</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml
index bee7f547c7..a2530874f6 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml
@@ -201,6 +201,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml
index 15e7ee84c6..a7ea218456 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml
@@ -162,6 +162,7 @@
<flag name='block-export-add'/>
<flag name='netdev.vhost-vdpa'/>
<flag name='fsdev.createmode'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100243</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index cebacc249d..9810c620a9 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -250,6 +250,7 @@
<flag name='fsdev.createmode'/>
<flag name='dc390'/>
<flag name='am53c974'/>
+ <flag name='memory-backend-file.x-use-canonical-path-for-ramblock-id'/>
<version>5002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100243</microcodeVersion>
--
2.26.2
5:19 a.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
This capability tracks whether memory-backend-file has
"x-use-canonical-path-for-ramblock-id" attribute. Introduced into
QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered
Please use a commit hash instead of this.
experimental or internal to QEMU, the next commit justifies its
use.
NACK unless qemu adds a statement to their code and documentation that
the this property is considered stable despite the 'x-prefix' and you
add a link to the appropriate qemu upstream commit once it's done.
We don't want to depend on experimental stuff so we need a strong
excuse.
5:29 a.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On 1/12/21 12:19 PM, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
> This capability tracks whether memory-backend-file has
> "x-use-canonical-path-for-ramblock-id" attribute. Introduced into
> QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered
Please use a commit hash instead of this.
> experimental or internal to QEMU, the next commit justifies its
> use.
NACK unless qemu adds a statement to their code and documentation that
the this property is considered stable despite the 'x-prefix' and you
add a link to the appropriate qemu upstream commit once it's done.
We don't want to depend on experimental stuff so we need a strong
excuse.
That's done in the next commit. Do you want me to copy it here too? I
figured I'd put the justification where I'm actually setting the
internal knob.
BTW: I'm linking qemu patch in the cover letter that documents that
x-use-canonical-.. is stable and shouldn't be removed/mangled with.
Michal
5:35 a.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote:
On 1/12/21 12:19 PM, Peter Krempa wrote:
> On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
> > This capability tracks whether memory-backend-file has
> > "x-use-canonical-path-for-ramblock-id" attribute. Introduced into
> > QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered
>
> Please use a commit hash instead of this.
>
> > experimental or internal to QEMU, the next commit justifies its
> > use.
>
> NACK unless qemu adds a statement to their code and documentation that
> the this property is considered stable despite the 'x-prefix' and you
> add a link to the appropriate qemu upstream commit once it's done.
>
> We don't want to depend on experimental stuff so we need a strong
> excuse.
>
That's done in the next commit. Do you want me to copy it here too? I
figured I'd put the justification where I'm actually setting the internal
knob.
Yes, because this is also mentioning the an 'x-' prefixed property. I
want to be absolutely clear in any places (including a comment in the
code, which you also should add into the capability code) that this is
extraordinary circumstance and that qemu is actually considering that
property stable.
I want to prevent that this commit will be used as an excuse to depend
on experimental properties which are not actually considered
non-experimental.
5:41 a.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On 1/12/21 12:35 PM, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote:
> On 1/12/21 12:19 PM, Peter Krempa wrote:
>> On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
>>> This capability tracks whether memory-backend-file has
>>> "x-use-canonical-path-for-ramblock-id" attribute. Introduced into
>>> QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered
>>
>> Please use a commit hash instead of this.
>>
>>> experimental or internal to QEMU, the next commit justifies its
>>> use.
>>
>> NACK unless qemu adds a statement to their code and documentation that
>> the this property is considered stable despite the 'x-prefix' and you
>> add a link to the appropriate qemu upstream commit once it's done.
>>
>> We don't want to depend on experimental stuff so we need a strong
>> excuse.
>>
>
> That's done in the next commit. Do you want me to copy it here too? I
> figured I'd put the justification where I'm actually setting the internal
> knob.
Yes, because this is also mentioning the an 'x-' prefixed property. I
want to be absolutely clear in any places (including a comment in the
code, which you also should add into the capability code) that this is
extraordinary circumstance and that qemu is actually considering that
property stable.
I want to prevent that this commit will be used as an excuse to depend
on experimental properties which are not actually considered
non-experimental.
Alight, fair enough. Let me fix that in v2.
Michal
12:20 p.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, 12 Jan 2021 12:35:19 +0100
Peter Krempa <pkrempa(a)redhat.com> wrote:
On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote:
> On 1/12/21 12:19 PM, Peter Krempa wrote:
> > On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
> > > This capability tracks whether memory-backend-file has
> > > "x-use-canonical-path-for-ramblock-id" attribute. Introduced
into
> > > QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered
> >
> > Please use a commit hash instead of this.
> >
> > > experimental or internal to QEMU, the next commit justifies its
> > > use.
> >
> > NACK unless qemu adds a statement to their code and documentation that
> > the this property is considered stable despite the 'x-prefix' and you
> > add a link to the appropriate qemu upstream commit once it's done.
> >
> > We don't want to depend on experimental stuff so we need a strong
> > excuse.
> >
>
> That's done in the next commit. Do you want me to copy it here too? I
> figured I'd put the justification where I'm actually setting the internal
> knob.
Yes, because this is also mentioning the an 'x-' prefixed property. I
want to be absolutely clear in any places (including a comment in the
code, which you also should add into the capability code) that this is
extraordinary circumstance and that qemu is actually considering that
property stable.
the only reason to keep x- prefix in this case is to cause less issues for
downstream QEMUs. Since this compat property is copied to their own machine types.
If we keep prefix downstream doesn't have to do anything, if we rename it,
then downstreams have to carry a separate patch that does the same for
their old machine types.
I want to prevent that this commit will be used as an excuse to
depend
on experimental properties which are not actually considered
non-experimental.
12:28 p.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, Jan 12, 2021 at 19:20:58 +0100, Igor Mammedov wrote:
On Tue, 12 Jan 2021 12:35:19 +0100
Peter Krempa <pkrempa(a)redhat.com> wrote:
> On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote:
> > On 1/12/21 12:19 PM, Peter Krempa wrote:
> > > On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
> > > > This capability tracks whether memory-backend-file has
> > > > "x-use-canonical-path-for-ramblock-id" attribute.
Introduced into
> > > > QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is
considered
> > >
> > > Please use a commit hash instead of this.
> > >
> > > > experimental or internal to QEMU, the next commit justifies its
> > > > use.
> > >
> > > NACK unless qemu adds a statement to their code and documentation that
> > > the this property is considered stable despite the 'x-prefix' and
you
> > > add a link to the appropriate qemu upstream commit once it's done.
> > >
> > > We don't want to depend on experimental stuff so we need a strong
> > > excuse.
> > >
> >
> > That's done in the next commit. Do you want me to copy it here too? I
> > figured I'd put the justification where I'm actually setting the
internal
> > knob.
>
> Yes, because this is also mentioning the an 'x-' prefixed property. I
> want to be absolutely clear in any places (including a comment in the
> code, which you also should add into the capability code) that this is
> extraordinary circumstance and that qemu is actually considering that
> property stable.
the only reason to keep x- prefix in this case is to cause less issues for
downstream QEMUs. Since this compat property is copied to their own machine types.
If we keep prefix downstream doesn't have to do anything, if we rename it,
then downstreams have to carry a separate patch that does the same for
their old machine types.
That would be okay if it's limited to past versions, but in this
instance it is not. Allowing x-prefixed properties for any future
release is a dangerous precedent. If we want to allow to detect the
capability also for future release, we must declare that it's for a very
particular reason and also that qemu will not delete it at will.
This is to prevent any future discussions of unwaranted usage of
x-prefixed properties in libvirt.
12:41 p.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, Jan 12, 2021 at 07:28:45PM +0100, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 19:20:58 +0100, Igor Mammedov wrote:
> On Tue, 12 Jan 2021 12:35:19 +0100
> Peter Krempa <pkrempa(a)redhat.com> wrote:
>
> > On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote:
> > > On 1/12/21 12:19 PM, Peter Krempa wrote:
> > > > On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
> > > > > This capability tracks whether memory-backend-file has
> > > > > "x-use-canonical-path-for-ramblock-id" attribute.
Introduced into
> > > > > QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is
considered
> > > >
> > > > Please use a commit hash instead of this.
> > > >
> > > > > experimental or internal to QEMU, the next commit justifies its
> > > > > use.
> > > >
> > > > NACK unless qemu adds a statement to their code and documentation
that
> > > > the this property is considered stable despite the 'x-prefix'
and you
> > > > add a link to the appropriate qemu upstream commit once it's
done.
> > > >
> > > > We don't want to depend on experimental stuff so we need a
strong
> > > > excuse.
> > > >
> > >
> > > That's done in the next commit. Do you want me to copy it here too? I
> > > figured I'd put the justification where I'm actually setting the
internal
> > > knob.
> >
> > Yes, because this is also mentioning the an 'x-' prefixed property. I
> > want to be absolutely clear in any places (including a comment in the
> > code, which you also should add into the capability code) that this is
> > extraordinary circumstance and that qemu is actually considering that
> > property stable.
>
> the only reason to keep x- prefix in this case is to cause less issues for
> downstream QEMUs. Since this compat property is copied to their own machine types.
> If we keep prefix downstream doesn't have to do anything, if we rename it,
> then downstreams have to carry a separate patch that does the same for
> their old machine types.
That would be okay if it's limited to past versions, but in this
instance it is not. Allowing x-prefixed properties for any future
release is a dangerous precedent. If we want to allow to detect the
capability also for future release, we must declare that it's for a very
particular reason and also that qemu will not delete it at will.
This is to prevent any future discussions of unwaranted usage of
x-prefixed properties in libvirt.
Yeah it is pretty dubious on the QEMU side to have used an "x-" prefix
here at all, when use of this option is mandatory to make migration
work :-(
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1:24 p.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, 12 Jan 2021 18:41:38 +0000
Daniel P. Berrangé <berrange(a)redhat.com> wrote:
On Tue, Jan 12, 2021 at 07:28:45PM +0100, Peter Krempa wrote:
> On Tue, Jan 12, 2021 at 19:20:58 +0100, Igor Mammedov wrote:
> > On Tue, 12 Jan 2021 12:35:19 +0100
> > Peter Krempa <pkrempa(a)redhat.com> wrote:
> >
> > > On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote:
> > > > On 1/12/21 12:19 PM, Peter Krempa wrote:
> > > > > On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote:
> > > > > > This capability tracks whether memory-backend-file has
> > > > > > "x-use-canonical-path-for-ramblock-id" attribute.
Introduced into
> > > > > > QEMU by commit v4.0.0-rc0~189^2. While "x-"
prefix is considered
> > > > >
> > > > > Please use a commit hash instead of this.
> > > > >
> > > > > > experimental or internal to QEMU, the next commit justifies
its
> > > > > > use.
> > > > >
> > > > > NACK unless qemu adds a statement to their code and
documentation that
> > > > > the this property is considered stable despite the
'x-prefix' and you
> > > > > add a link to the appropriate qemu upstream commit once it's
done.
> > > > >
> > > > > We don't want to depend on experimental stuff so we need a
strong
> > > > > excuse.
> > > > >
> > > >
> > > > That's done in the next commit. Do you want me to copy it here
too? I
> > > > figured I'd put the justification where I'm actually setting
the internal
> > > > knob.
> > >
> > > Yes, because this is also mentioning the an 'x-' prefixed
property. I
> > > want to be absolutely clear in any places (including a comment in the
> > > code, which you also should add into the capability code) that this is
> > > extraordinary circumstance and that qemu is actually considering that
> > > property stable.
> >
> > the only reason to keep x- prefix in this case is to cause less issues for
> > downstream QEMUs. Since this compat property is copied to their own machine
types.
> > If we keep prefix downstream doesn't have to do anything, if we rename it,
> > then downstreams have to carry a separate patch that does the same for
> > their old machine types.
>
> That would be okay if it's limited to past versions, but in this
> instance it is not. Allowing x-prefixed properties for any future
> release is a dangerous precedent. If we want to allow to detect the
> capability also for future release, we must declare that it's for a very
> particular reason and also that qemu will not delete it at will.
>
> This is to prevent any future discussions of unwaranted usage of
> x-prefixed properties in libvirt.
Yeah it is pretty dubious on the QEMU side to have used an "x-" prefix
here at all, when use of this option is mandatory to make migration
work :-(
if generic consensus is to drop prefix, I can post a QEMU patch to do so
and let downstream(s) to carry burden.
Regards,
Daniel
1:59 p.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, Jan 12, 2021 at 20:24:44 +0100, Igor Mammedov wrote:
On Tue, 12 Jan 2021 18:41:38 +0000
Daniel P. Berrangé <berrange(a)redhat.com> wrote:
> On Tue, Jan 12, 2021 at 07:28:45PM +0100, Peter Krempa wrote:
> > On Tue, Jan 12, 2021 at 19:20:58 +0100, Igor Mammedov wrote:
> > > On Tue, 12 Jan 2021 12:35:19 +0100
> > > Peter Krempa <pkrempa(a)redhat.com> wrote
[...]
> Yeah it is pretty dubious on the QEMU side to have used an
"x-" prefix
> here at all, when use of this option is mandatory to make migration
> work :-(
if generic consensus is to drop prefix, I can post a QEMU patch to do so
and let downstream(s) to carry burden.
It really depends on the situation, because the commit messages don't
seem to describe it satisfactory.
Basically we don't want to ever use a qemu property knob, which is qemu
free to change arbitrarily.
If the property is to be used with any upcoming qemu version we must get
a guarantee that it will not change. There are two options basically:
1) 'x-' is dropped
1a) we will use it with qemu-6.0 and later
( everything is clean, but users will have to update qemu to fix it )
1b) we will carry code which will use 'x-' prefixed version from it's
inception until qemu-5.2, when we will hard-mask it out and add
plenty comments outlining that this is not what we do normally
(it will be okay for past releases, since they will not change)
2) qemu declares the option stable with the 'x-' prefix
We'll require that any place even in the code which declares the
option has an appropriate comment preventing anybody from changing
it.
We'll then add also cautionary comments discouraging use of it.
3) qemu fixes the issue without libvirt's involvment
For us really 1a) and 3 is acceptable without any comments. Other
options will require extraordinary measures to prevent using this as
prior art in using any other x-prefixed features from qemu.
in 1a) case, downstreams can obviously backport the qemu patch renaming
the feature and libvirt will require no change at all
Now the question is whether we want to make migration work between the
affected releases which will depend what to use.
6:08 p.m.
New subject: [PATCH 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Tue, 12 Jan 2021 20:59:11 +0100
Peter Krempa <pkrempa(a)redhat.com> wrote:
On Tue, Jan 12, 2021 at 20:24:44 +0100, Igor Mammedov wrote:
> On Tue, 12 Jan 2021 18:41:38 +0000
> Daniel P. Berrangé <berrange(a)redhat.com> wrote:
>
> > On Tue, Jan 12, 2021 at 07:28:45PM +0100, Peter Krempa wrote:
> > > On Tue, Jan 12, 2021 at 19:20:58 +0100, Igor Mammedov wrote:
> > > > On Tue, 12 Jan 2021 12:35:19 +0100
> > > > Peter Krempa <pkrempa(a)redhat.com> wrote
[...]
> > Yeah it is pretty dubious on the QEMU side to have used an "x-"
prefix
> > here at all, when use of this option is mandatory to make migration
> > work :-(
>
> if generic consensus is to drop prefix, I can post a QEMU patch to do so
> and let downstream(s) to carry burden.
It really depends on the situation, because the commit messages don't
seem to describe it satisfactory.
Basically we don't want to ever use a qemu property knob, which is qemu
free to change arbitrarily.
If the property is to be used with any upcoming qemu version we must get
a guarantee that it will not change. There are two options basically:
1) 'x-' is dropped
1a) we will use it with qemu-6.0 and later
( everything is clean, but users will have to update qemu to fix it )
I have
thought about it some more, (modulo downstream issue)
dropping prefix will effectively exclude old QEMU-(5.0-5.2)
even though feature is available there.
1b) we will carry code which will use 'x-' prefixed version
from it's
inception until qemu-5.2, when we will hard-mask it out and add
plenty comments outlining that this is not what we do normally
(it will be okay for past releases, since they will not change)
5.2 is not
enough, it should be carried till machine type 4.0 exists.
On QEMU side we once 4.0 machine type is removed we can deprecate and
remove no longer needed option so libvirt (with this patches) would see
that it no longer exists and not put it on CLI anymore. Only after that
it probably is ok to drop code for it.
2) qemu declares the option stable with the 'x-' prefix
We'll require that any place even in the code which declares the
option has an appropriate comment preventing anybody from changing
it.
We'll then add also cautionary comments discouraging use of it.
I've
just resent v2 of QEMU patch that incorporates your suggestions.
3) qemu fixes the issue without libvirt's involvment
if it were possible the
option, I'd go for it in the first place.
unfortunately, it's too late for it now.
For us really 1a) and 3 is acceptable without any comments. Other
options will require extraordinary measures to prevent using this as
prior art in using any other x-prefixed features from qemu.
in 1a) case, downstreams can obviously backport the qemu patch renaming
the feature and libvirt will require no change at all
Now the question is whether we want to make migration work between the
affected releases which will depend what to use.
If we can help it, then yes.
That's why I resent QEMU patch keeping 'x-' prefix (with your feedback
included).
2:29 a.m.
New subject: [PATCH 2/2] qemu: Do not Use canonical path for system memory
In commit v6.9.0-rc1~450 I've adapted libvirt to QEMU's deprecation of
-mem-path and -mem-prealloc and switched to memory-backend-* even for
system memory. My claim was that that's what QEMU does under the hood
anyway. And indeed it was: see QEMU commit v5.0.0-rc0~75^2~1^2~76 and
look at function create_default_memdev().
However, then commit v5.0.0-rc1~11^2~3 was merged into QEMU. While it
was fixing a bug, it also changed the create_default_memdev() function
in which it started turning off use of canonical path (by setting
"x-use-canonical-path-for-ramblock-id" attribute to false). This wasn't
documented until QEMU commit XXX. The path affects migration - the same
path has to be used on the source and on the destination. Therefore, if
there is old guest started with '-m X' it has "pc.ram" block which
doesn't use canonical path and thus when migrating to newer QEMU which
uses memory-backend-* we have to turn off the canonical path explicitly.
Otherwise, "/objects/pc.ram" path would be expected by QEMU which
doesn't match the source.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
I'll replace both occurrences of 'QEMU commit XXX' once QEMU patch is
merged.
src/qemu/qemu_command.c | 30 ++++++++++++++++---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
.../hugepages-memaccess3.x86_64-latest.args | 4 +--
4 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f970a3128..b99d4e5faf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2950,7 +2950,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
qemuDomainObjPrivatePtr priv,
const virDomainDef *def,
const virDomainMemoryDef *mem,
- bool force)
+ bool force,
+ bool systemMemory)
{
const char *backendType = "memory-backend-file";
virDomainNumatuneMemMode mode;
@@ -2967,6 +2968,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
bool needHugepage = !!pagesize;
bool useHugepage = !!pagesize;
int discard = mem->discard;
+ bool useCanonicalPath = true;
/* The difference between @needHugepage and @useHugepage is that the latter
* is true whenever huge page is defined for the current memory cell.
@@ -3081,6 +3083,9 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+ if (systemMemory)
+ useCanonicalPath = false;
+
} else if (useHugepage || mem->nvdimmPath || memAccess ||
def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
@@ -3122,10 +3127,27 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+
+ if (systemMemory)
+ useCanonicalPath = false;
+
} else {
backendType = "memory-backend-ram";
}
+ /* This is a terrible hack, but unfortunately there is no better way.
+ * The replacement for '-m X' argument is not simple '-machine
+ * memory-backend' and '-object memory-backend-*,size=X' (which was the
+ * idea). This is because of create_default_memdev() in QEMU sets
+ * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
+ * documented in QEMU in qemu-options.hx under 'memory-backend'.
+ * See QEMU commit XXX.
+ */
+ if (!useCanonicalPath &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID)
&&
+ virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id",
false, NULL) < 0)
+ return -1;
+
if (!priv->memPrealloc &&
virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
return -1;
@@ -3237,7 +3259,7 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
mem.info.alias = alias;
if ((rc = qemuBuildMemoryBackendProps(&props, alias, cfg,
- priv, def, &mem, false)) < 0)
+ priv, def, &mem, false, false)) < 0)
return -1;
if (virQEMUBuildObjectCommandlineFromJSON(buf, props) < 0)
@@ -3266,7 +3288,7 @@ qemuBuildMemoryDimmBackendStr(virBufferPtr buf,
alias = g_strdup_printf("mem%s", mem->info.alias);
if (qemuBuildMemoryBackendProps(&props, alias, cfg,
- priv, def, mem, true) < 0)
+ priv, def, mem, true, false) < 0)
return -1;
if (virQEMUBuildObjectCommandlineFromJSON(buf, props) < 0)
@@ -7065,7 +7087,7 @@ qemuBuildMemCommandLineMemoryDefaultBackend(virCommandPtr cmd,
mem.info.alias = (char *) defaultRAMid;
if (qemuBuildMemoryBackendProps(&props, defaultRAMid, cfg,
- priv, def, &mem, false) < 0)
+ priv, def, &mem, false, true) < 0)
return -1;
if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 3cfe6ff3e9..32cb013172 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -153,7 +153,8 @@ int qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
qemuDomainObjPrivatePtr priv,
const virDomainDef *def,
const virDomainMemoryDef *mem,
- bool force);
+ bool force,
+ bool systemMemory);
char *
qemuBuildMemoryDeviceStr(const virDomainDef *def,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f336a90c8e..18d8cc06f5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2426,7 +2426,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
goto cleanup;
if (qemuBuildMemoryBackendProps(&props, objalias, cfg,
- priv, vm->def, mem, true) < 0)
+ priv, vm->def, mem, true, false) < 0)
goto cleanup;
if (qemuProcessBuildDestroyMemoryPaths(driver, vm, mem, true) < 0)
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
b/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
index 6033950eab..9396441b92 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
@@ -19,8 +19,8 @@
arch-capabilities=on,ssbd=on,xsaves=on,cmp-legacy=on,amd-ssbd=on,virt-ssbd=on,\
rdctl-no=on,skip-l1dfl-vmentry=on,mds-no=on,pschange-mc-no=on \
-m 4096 \
-object memory-backend-file,id=pc.ram,\
-mem-path=/dev/hugepages2M/libvirt/qemu/-1-fedora,share=yes,prealloc=yes,\
-size=4294967296 \
+mem-path=/dev/hugepages2M/libvirt/qemu/-1-fedora,share=yes,\
+x-use-canonical-path-for-ramblock-id=no,prealloc=yes,size=4294967296 \
-overcommit mem-lock=off \
-smp 4,sockets=4,cores=1,threads=1 \
-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
--
2.26.2
1:21 p.m.
New subject: [PATCH 2/2] qemu: Do not Use canonical path for system memory
On Tue, 12 Jan 2021 09:29:50 +0100
Michal Privoznik <mprivozn(a)redhat.com> wrote:
In commit v6.9.0-rc1~450 I've adapted libvirt to QEMU's
deprecation of
-mem-path and -mem-prealloc and switched to memory-backend-* even for
system memory. My claim was that that's what QEMU does under the hood
anyway. And indeed it was: see QEMU commit v5.0.0-rc0~75^2~1^2~76 and
look at function create_default_memdev().
However, then commit v5.0.0-rc1~11^2~3 was merged into QEMU. While it
was fixing a bug, it also changed the create_default_memdev() function
in which it started turning off use of canonical path (by setting
"x-use-canonical-path-for-ramblock-id" attribute to false). This wasn't
documented until QEMU commit XXX. The path affects migration - the same
path has to be used on the source and on the destination. Therefore, if
there is old guest started with '-m X' it has "pc.ram" block which
doesn't use canonical path and thus when migrating to newer QEMU which
uses memory-backend-* we have to turn off the canonical path explicitly.
Otherwise, "/objects/pc.ram" path would be expected by QEMU which
doesn't match the source.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
I'll replace both occurrences of 'QEMU commit XXX' once QEMU patch is
merged.
src/qemu/qemu_command.c | 30 ++++++++++++++++---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
.../hugepages-memaccess3.x86_64-latest.args | 4 +--
4 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f970a3128..b99d4e5faf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2950,7 +2950,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
qemuDomainObjPrivatePtr priv,
const virDomainDef *def,
const virDomainMemoryDef *mem,
- bool force)
+ bool force,
+ bool systemMemory)
{
const char *backendType = "memory-backend-file";
virDomainNumatuneMemMode mode;
@@ -2967,6 +2968,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
bool needHugepage = !!pagesize;
bool useHugepage = !!pagesize;
int discard = mem->discard;
+ bool useCanonicalPath = true;
/* The difference between @needHugepage and @useHugepage is that the latter
* is true whenever huge page is defined for the current memory cell.
@@ -3081,6 +3083,9 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+ if (systemMemory)
+ useCanonicalPath = false;
+
} else if (useHugepage || mem->nvdimmPath || memAccess ||
def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
@@ -3122,10 +3127,27 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+
+ if (systemMemory)
+ useCanonicalPath = false;
+
} else {
backendType = "memory-backend-ram";
}
+ /* This is a terrible hack, but unfortunately there is no better way.
+ * The replacement for '-m X' argument is not simple '-machine
+ * memory-backend' and '-object memory-backend-*,size=X' (which was the
+ * idea). This is because of create_default_memdev() in QEMU sets
+ * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
+ * documented in QEMU in qemu-options.hx under 'memory-backend'.
+ * See QEMU commit XXX.
+ */
+ if (!useCanonicalPath &&
+ virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID) &&
+ virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id",
false, NULL) < 0)
+ return -1;
is it possible to do it only for old machine types <= 4.0, to limit hack exposure?
if (!priv->memPrealloc &&
virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
return -1;
@@ -3237,7 +3259,7 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
mem.info.alias = alias;
if ((rc = qemuBuildMemoryBackendProps(&props, alias, cfg,
- priv, def, &mem, false)) < 0)
+ priv, def, &mem, false, false)) < 0)
return -1;
if (virQEMUBuildObjectCommandlineFromJSON(buf, props) < 0)
@@ -3266,7 +3288,7 @@ qemuBuildMemoryDimmBackendStr(virBufferPtr buf,
alias = g_strdup_printf("mem%s", mem->info.alias);
if (qemuBuildMemoryBackendProps(&props, alias, cfg,
- priv, def, mem, true) < 0)
+ priv, def, mem, true, false) < 0)
return -1;
if (virQEMUBuildObjectCommandlineFromJSON(buf, props) < 0)
@@ -7065,7 +7087,7 @@ qemuBuildMemCommandLineMemoryDefaultBackend(virCommandPtr cmd,
mem.info.alias = (char *) defaultRAMid;
if (qemuBuildMemoryBackendProps(&props, defaultRAMid, cfg,
- priv, def, &mem, false) < 0)
+ priv, def, &mem, false, true) < 0)
return -1;
if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 3cfe6ff3e9..32cb013172 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -153,7 +153,8 @@ int qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
qemuDomainObjPrivatePtr priv,
const virDomainDef *def,
const virDomainMemoryDef *mem,
- bool force);
+ bool force,
+ bool systemMemory);
char *
qemuBuildMemoryDeviceStr(const virDomainDef *def,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f336a90c8e..18d8cc06f5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2426,7 +2426,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
goto cleanup;
if (qemuBuildMemoryBackendProps(&props, objalias, cfg,
- priv, vm->def, mem, true) < 0)
+ priv, vm->def, mem, true, false) < 0)
goto cleanup;
if (qemuProcessBuildDestroyMemoryPaths(driver, vm, mem, true) < 0)
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
b/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
index 6033950eab..9396441b92 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hugepages-memaccess3.x86_64-latest.args
@@ -19,8 +19,8 @@
arch-capabilities=on,ssbd=on,xsaves=on,cmp-legacy=on,amd-ssbd=on,virt-ssbd=on,\
rdctl-no=on,skip-l1dfl-vmentry=on,mds-no=on,pschange-mc-no=on \
-m 4096 \
-object memory-backend-file,id=pc.ram,\
-mem-path=/dev/hugepages2M/libvirt/qemu/-1-fedora,share=yes,prealloc=yes,\
-size=4294967296 \
+mem-path=/dev/hugepages2M/libvirt/qemu/-1-fedora,share=yes,\
+x-use-canonical-path-for-ramblock-id=no,prealloc=yes,size=4294967296 \
-overcommit mem-lock=off \
-smp 4,sockets=4,cores=1,threads=1 \
-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
2:13 p.m.
New subject: [PATCH 2/2] qemu: Do not Use canonical path for system memory
On Tue, Jan 12, 2021 at 20:21:19 +0100, Igor Mammedov wrote:
On Tue, 12 Jan 2021 09:29:50 +0100
Michal Privoznik <mprivozn(a)redhat.com> wrote:
> In commit v6.9.0-rc1~450 I've adapted libvirt to QEMU's deprecation of
> -mem-path and -mem-prealloc and switched to memory-backend-* even for
> system memory. My claim was that that's what QEMU does under the hood
> anyway. And indeed it was: see QEMU commit v5.0.0-rc0~75^2~1^2~76 and
> look at function create_default_memdev().
>
> However, then commit v5.0.0-rc1~11^2~3 was merged into QEMU. While it
> was fixing a bug, it also changed the create_default_memdev() function
> in which it started turning off use of canonical path (by setting
> "x-use-canonical-path-for-ramblock-id" attribute to false). This
wasn't
> documented until QEMU commit XXX. The path affects migration - the same
> path has to be used on the source and on the destination. Therefore, if
> there is old guest started with '-m X' it has "pc.ram" block
which
> doesn't use canonical path and thus when migrating to newer QEMU which
> uses memory-backend-* we have to turn off the canonical path explicitly.
> Otherwise, "/objects/pc.ram" path would be expected by QEMU which
> doesn't match the source.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
>
> I'll replace both occurrences of 'QEMU commit XXX' once QEMU patch is
> merged.
>
> src/qemu/qemu_command.c | 30 ++++++++++++++++---
> src/qemu/qemu_command.h | 3 +-
> src/qemu/qemu_hotplug.c | 2 +-
> .../hugepages-memaccess3.x86_64-latest.args | 4 +--
> 4 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6f970a3128..b99d4e5faf 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
...
> @@ -3122,10 +3127,27 @@
qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
>
> if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
> return -1;
> +
> + if (systemMemory)
> + useCanonicalPath = false;
> +
> } else {
> backendType = "memory-backend-ram";
> }
>
> + /* This is a terrible hack, but unfortunately there is no better way.
> + * The replacement for '-m X' argument is not simple '-machine
> + * memory-backend' and '-object memory-backend-*,size=X' (which was
the
> + * idea). This is because of create_default_memdev() in QEMU sets
> + * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
> + * documented in QEMU in qemu-options.hx under 'memory-backend'.
> + * See QEMU commit XXX.
> + */
> + if (!useCanonicalPath &&
> + virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID) &&
> + virJSONValueObjectAdd(props,
"b:x-use-canonical-path-for-ramblock-id", false, NULL) < 0)
> + return -1;
is it possible to do it only for old machine types <= 4.0, to limit hack exposure?
We consume machine types as opaque strings, we don't parse them and thus
we don't have any ordering on them. Without this telling what <= 4.0
means is impossible. And I don't think we should start doing it, and
especially not for limiting this hack as it would be limiting a hack
with another one.
A reasonable solution would be if we could tell which machine types need
(or perhaps don't need) such treatment by probing QEMU for available
machine types.
Jirka
1:06 a.m.
New subject: [PATCH 2/2] qemu: Do not Use canonical path for system memory
On Tue, Jan 12, 2021 at 21:13:54 +0100, Jiri Denemark wrote:
On Tue, Jan 12, 2021 at 20:21:19 +0100, Igor Mammedov wrote:
> On Tue, 12 Jan 2021 09:29:50 +0100
> Michal Privoznik <mprivozn(a)redhat.com> wrote:
[...]
We consume machine types as opaque strings, we don't parse them and thus
we don't have any ordering on them. Without this telling what <= 4.0
means is impossible. And I don't think we should start doing it, and
especially not for limiting this hack as it would be limiting a hack
with another one.
A reasonable solution would be if we could tell which machine types need
(or perhaps don't need) such treatment by probing QEMU for available
machine types.
Well this has two implications:
1) qemu telling us would be detectable, thus no need to check for x-something
2) if qemu can tell us when to use it, it's very probable that at that
point it can do the correct thing itself, not requiring anything from
us
5:26 a.m.
New subject: [PATCH 2/2] qemu: Do not Use canonical path for system memory
On Wed, Jan 13, 2021 at 08:06:30 +0100, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 21:13:54 +0100, Jiri Denemark wrote:
> On Tue, Jan 12, 2021 at 20:21:19 +0100, Igor Mammedov wrote:
> > On Tue, 12 Jan 2021 09:29:50 +0100
> > Michal Privoznik <mprivozn(a)redhat.com> wrote:
[...]
>
> We consume machine types as opaque strings, we don't parse them and thus
> we don't have any ordering on them. Without this telling what <= 4.0
> means is impossible. And I don't think we should start doing it, and
> especially not for limiting this hack as it would be limiting a hack
> with another one.
>
> A reasonable solution would be if we could tell which machine types need
> (or perhaps don't need) such treatment by probing QEMU for available
> machine types.
Well this has two implications:
1) qemu telling us would be detectable, thus no need to check for x-something
2) if qemu can tell us when to use it, it's very probable that at that
point it can do the correct thing itself, not requiring anything from
us
I didn't really study the details, but I guess QEMU cannot do the right
thing for old machine types because of backward compatibility. While for
new machine types (>4.0) QEMU is automatically doing it right. With this
assumption, new QEMU could report some machine types are OK and thus we
could skip applying the hack for them. But we would still need it
(including the x-... detection) for old machine type and older QEMU
releases which didn't mark any machine type as OK.
Thus we would not get rid of the x-... hack, but we could properly limit
it for old QEMU and old machine types. That is, the additional probing
would be an optimization of the hack which we need anyway I'm afraid.
You can of course ignore all I said if my assumption is wrong :-)
Jirka
1407
days inactive
1410
days old
16 comments
5 participants
participants (5)
-
Daniel P. Berrangé -
Igor Mammedov -
Jiri Denemark -
Michal Privoznik -
Peter Krempa