[PATCH] qemu: add support for max-ram-below-4g option
Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory> ... </domain> Signed-off-by: Zhiyong Ye <yezhiyong@bytedance.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Signed-off-by: zhangruien <zhangruien@bytedance.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/qemu/qemu_command.c | 4 ++++ 3 files changed, 26 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a72d58f488..c211a69ed1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4720,6 +4720,19 @@ virDomainDefPostParseMemory(virDomainDef *def, return -1; } + if (def->mem.max_ram_below_4g && + def->mem.max_ram_below_4g < (1ULL << 10)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary is too small, " + "BIOS may not work with less than 1MiB")); + return -1; + } else if (def->mem.max_ram_below_4g > (1ULL << 22)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary must be " + "less than or equal to 4GiB")); + return -1; + } + return 0; } @@ -19786,6 +19799,10 @@ virDomainDefParseMemory(virDomainDef *def, &def->mem.max_memory, false, false) < 0) goto error; + if (virDomainParseMemory("./memory[1]/@below4g", "./memory[1]/@unit", ctxt, + &def->mem.max_ram_below_4g, false, true) < 0) + goto error; + if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Failed to parse memory slot count")); @@ -28844,6 +28861,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", virTristateSwitchTypeToString(def->mem.dump_core)); + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(buf, " below4g='%llu'", def->mem.max_ram_below_4g); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", virDomainDefGetMemoryTotal(def)); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4838687edf..a939d43e93 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2597,6 +2597,9 @@ struct _virDomainMemtune { unsigned long long max_memory; /* in kibibytes */ unsigned int memory_slots; /* maximum count of RAM memory slots */ + /* maximum memory below the 4GiB boundary (32bit boundary) */ + unsigned long long max_ram_below_4g; /* in kibibytes */ + bool nosharepages; bool locked; int dump_core; /* enum virTristateSwitch */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index be93182092..c69ad781e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6961,6 +6961,10 @@ qemuBuildMachineCommandLine(virCommand *cmd, cfg->dumpGuestCore ? "on" : "off"); } + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(&buf, ",max-ram-below-4g=%llu", + def->mem.max_ram_below_4g * 1024); + if (def->mem.nosharepages) virBufferAddLit(&buf, ",mem-merge=off"); -- 2.24.3 (Apple Git-128)
On Sun, Apr 25, 2021 at 17:33:31 +0800, Zhiyong Ye wrote:
Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory>
This illustrates that sharing the 'unit' argument between 'below4g' and the actual memory size is wrong (pointed out also below). In case the user modifies the unit but doesn't change below4g value it may be misrepresented and since below4g is new some tools might not actually know how to handle that. below4g must either be by default in kiB, or have an own unit (which will probably be ugly).
... </domain>
Signed-off-by: Zhiyong Ye <yezhiyong@bytedance.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Signed-off-by: zhangruien <zhangruien@bytedance.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/qemu/qemu_command.c | 4 ++++
Missing docs (doc/formatdomain.rst) and test changes. Also we usually don't mix conf and qemu changes. You need at least a test case for qemuxml2argvtest and qemuxml2xmltest. The conf change is also missing a check in the ABI stability checking code.
3 files changed, 26 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a72d58f488..c211a69ed1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4720,6 +4720,19 @@ virDomainDefPostParseMemory(virDomainDef *def, return -1; }
+ if (def->mem.max_ram_below_4g && + def->mem.max_ram_below_4g < (1ULL << 10)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary is too small, " + "BIOS may not work with less than 1MiB"));
"may not work" seems like you picked an arbitrary limit, and the same way it may not work with more than 1MiB. In cases when there isn't a strict technical limit, but rather "it usually doesn't work" like this the check is almost pointless.
+ return -1; + } else if (def->mem.max_ram_below_4g > (1ULL << 22)) {
we usually prefer to describe the limit as 4 * 1024 * 1024
+ virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary must be " + "less than or equal to 4GiB"));
don't break up error messages into multiple lines
+ return -1;
This belongs to conf/domain_validate.c You'll need to add a new function e.g. 'virDomainDefValidateMemory' call it from virDomainDefValidateInternal and add those checks there.
+ } + return 0; }
@@ -19786,6 +19799,10 @@ virDomainDefParseMemory(virDomainDef *def, &def->mem.max_memory, false, false) < 0) goto error;
+ if (virDomainParseMemory("./memory[1]/@below4g", "./memory[1]/@unit", ctxt, + &def->mem.max_ram_below_4g, false, true) < 0)
This uses the 'unit' which is used for memory for the 'below4g' attribute.
+ goto error; + if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Failed to parse memory slot count")); @@ -28844,6 +28861,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", virTristateSwitchTypeToString(def->mem.dump_core)); + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(buf, " below4g='%llu'", def->mem.max_ram_below_4g); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", virDomainDefGetMemoryTotal(def));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4838687edf..a939d43e93 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2597,6 +2597,9 @@ struct _virDomainMemtune { unsigned long long max_memory; /* in kibibytes */ unsigned int memory_slots; /* maximum count of RAM memory slots */
+ /* maximum memory below the 4GiB boundary (32bit boundary) */ + unsigned long long max_ram_below_4g; /* in kibibytes */ + bool nosharepages; bool locked; int dump_core; /* enum virTristateSwitch */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index be93182092..c69ad781e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6961,6 +6961,10 @@ qemuBuildMachineCommandLine(virCommand *cmd, cfg->dumpGuestCore ? "on" : "off"); }
+ if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(&buf, ",max-ram-below-4g=%llu", + def->mem.max_ram_below_4g * 1024); + if (def->mem.nosharepages) virBufferAddLit(&buf, ",mem-merge=off");
-- 2.24.3 (Apple Git-128)
On Mon, Apr 26, 2021 at 09:55:05AM +0200, Peter Krempa wrote:
On Sun, Apr 25, 2021 at 17:33:31 +0800, Zhiyong Ye wrote:
Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory>
This illustrates that sharing the 'unit' argument between 'below4g' and the actual memory size is wrong (pointed out also below). In case the user modifies the unit but doesn't change below4g value it may be misrepresented and since below4g is new some tools might not actually know how to handle that.
below4g must either be by default in kiB, or have an own unit (which will probably be ugly).
Why would we want to pick different units for the two values. Feels like it is overkill, and the same units value just applies to everything on this element. 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 :|
On Mon, Apr 26, 2021 at 09:20:53 +0100, Daniel P. Berrangé wrote:
On Mon, Apr 26, 2021 at 09:55:05AM +0200, Peter Krempa wrote:
On Sun, Apr 25, 2021 at 17:33:31 +0800, Zhiyong Ye wrote:
Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory>
This illustrates that sharing the 'unit' argument between 'below4g' and the actual memory size is wrong (pointed out also below). In case the user modifies the unit but doesn't change below4g value it may be misrepresented and since below4g is new some tools might not actually know how to handle that.
below4g must either be by default in kiB, or have an own unit (which will probably be ugly).
Why would we want to pick different units for the two values. Feels like it is overkill, and the same units value just applies to everything on this element.
It could create problems in an unlikely scenario such as a management app modifying the XML it got from libvirt, where it changes the unit but doesn't know about 'below4g'. Similarly it's unfriendly to users. Libvirt always formats back the value in kiB. If you want to change the memory size using user-friendly values but don't want to change 'below4g' you'd have to modify it as well. Both are minor, but annoying once you have to deal with it.
On 4/26/21 3:55 PM, Peter Krempa wrote:
Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory> This illustrates that sharing the 'unit' argument between 'below4g' and
On Sun, Apr 25, 2021 at 17:33:31 +0800, Zhiyong Ye wrote: the actual memory size is wrong (pointed out also below). In case the user modifies the unit but doesn't change below4g value it may be misrepresented and since below4g is new some tools might not actually know how to handle that.
below4g must either be by default in kiB, or have an own unit (which will probably be ugly).
So, how should we place 'blow4g'? I have summarized that we may have two ways: The first way is to put 'below4g' in the existing element, as Daniel suggested, that 'below4g' does not have its own 'unit' and let it default to KiB, e.g, <memory unit = "MiB" below4g = "2048"> 4096 </memory> The second way is to add a new element and 'below4g' has its own 'unit', such as:<memory unit = 'MiB'> 4096 </memory> <below4gMemory unit = 'MiB'> 2048 </below4gMemory>
... </domain>
Signed-off-by: Zhiyong Ye <yezhiyong@bytedance.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Signed-off-by: zhangruien <zhangruien@bytedance.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/qemu/qemu_command.c | 4 ++++ Missing docs (doc/formatdomain.rst) and test changes. Also we usually don't mix conf and qemu changes.
You need at least a test case for qemuxml2argvtest and qemuxml2xmltest.
The conf change is also missing a check in the ABI stability checking code.
3 files changed, 26 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a72d58f488..c211a69ed1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4720,6 +4720,19 @@ virDomainDefPostParseMemory(virDomainDef *def, return -1; }
+ if (def->mem.max_ram_below_4g && + def->mem.max_ram_below_4g < (1ULL << 10)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary is too small, " + "BIOS may not work with less than 1MiB")); "may not work" seems like you picked an arbitrary limit, and the same way it may not work with more than 1MiB. In cases when there isn't a strict technical limit, but rather "it usually doesn't work" like this the check is almost pointless.
This is seen from QEMU code. When it is less than 1MiB, QEMU cannot create a new virtual machine.
+ return -1; + } else if (def->mem.max_ram_below_4g > (1ULL << 22)) { we usually prefer to describe the limit as 4 * 1024 * 1024
+ virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary must be " + "less than or equal to 4GiB")); don't break up error messages into multiple lines
+ return -1; This belongs to conf/domain_validate.c
You'll need to add a new function e.g. 'virDomainDefValidateMemory' call it from virDomainDefValidateInternal and add those checks there.
+ } + return 0; }
@@ -19786,6 +19799,10 @@ virDomainDefParseMemory(virDomainDef *def, &def->mem.max_memory, false, false) < 0) goto error;
+ if (virDomainParseMemory("./memory[1]/@below4g", "./memory[1]/@unit", ctxt, + &def->mem.max_ram_below_4g, false, true) < 0) This uses the 'unit' which is used for memory for the 'below4g' attribute.
+ goto error; + if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Failed to parse memory slot count")); @@ -28844,6 +28861,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", virTristateSwitchTypeToString(def->mem.dump_core)); + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(buf, " below4g='%llu'", def->mem.max_ram_below_4g); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", virDomainDefGetMemoryTotal(def));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4838687edf..a939d43e93 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2597,6 +2597,9 @@ struct _virDomainMemtune { unsigned long long max_memory; /* in kibibytes */ unsigned int memory_slots; /* maximum count of RAM memory slots */
+ /* maximum memory below the 4GiB boundary (32bit boundary) */ + unsigned long long max_ram_below_4g; /* in kibibytes */ + bool nosharepages; bool locked; int dump_core; /* enum virTristateSwitch */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index be93182092..c69ad781e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6961,6 +6961,10 @@ qemuBuildMachineCommandLine(virCommand *cmd, cfg->dumpGuestCore ? "on" : "off"); }
+ if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(&buf, ",max-ram-below-4g=%llu", + def->mem.max_ram_below_4g * 1024); + if (def->mem.nosharepages) virBufferAddLit(&buf, ",mem-merge=off");
-- 2.24.3 (Apple Git-128)
I'm sorry that the format of the last email seems to be problematic, so I sent it again. On 4/26/21 3:55 PM, Peter Krempa wrote:
On Sun, Apr 25, 2021 at 17:33:31 +0800, Zhiyong Ye wrote:
Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory>
This illustrates that sharing the 'unit' argument between 'below4g' and the actual memory size is wrong (pointed out also below). In case the user modifies the unit but doesn't change below4g value it may be misrepresented and since below4g is new some tools might not actually know how to handle that.
below4g must either be by default in kiB, or have an own unit (which will probably be ugly).
So, how should we place 'blow4g'? I have summarized that we may have two ways: The first way is to put 'below4g' in the existing element, as Daniel suggested, that 'below4g' does not have its own 'unit' and let it default to KiB, e.g, <memory unit="MiB" below4g="2048"> 4096 </memory> The second way is to add a new element and 'below4g' has its own 'unit', such as: <memory unit='MiB'> 4096 </memory> <below4gMemory unit='MiB'> 2048 </below4gMemory>
... </domain>
Signed-off-by: Zhiyong Ye <yezhiyong@bytedance.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Signed-off-by: zhangruien <zhangruien@bytedance.com> --- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/qemu/qemu_command.c | 4 ++++
Missing docs (doc/formatdomain.rst) and test changes. Also we usually don't mix conf and qemu changes.
You need at least a test case for qemuxml2argvtest and qemuxml2xmltest.
The conf change is also missing a check in the ABI stability checking code.
3 files changed, 26 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a72d58f488..c211a69ed1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4720,6 +4720,19 @@ virDomainDefPostParseMemory(virDomainDef *def, return -1; }
+ if (def->mem.max_ram_below_4g && + def->mem.max_ram_below_4g < (1ULL << 10)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary is too small, " + "BIOS may not work with less than 1MiB"));
"may not work" seems like you picked an arbitrary limit, and the same way it may not work with more than 1MiB. In cases when there isn't a strict technical limit, but rather "it usually doesn't work" like this the check is almost pointless.
This is seen from QEMU code. When it is less than 1MiB, QEMU cannot create a new virtual machine.
+ return -1; + } else if (def->mem.max_ram_below_4g > (1ULL << 22)) {
we usually prefer to describe the limit as 4 * 1024 * 1024
+ virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary must be " + "less than or equal to 4GiB"));
don't break up error messages into multiple lines
+ return -1;
This belongs to conf/domain_validate.c
You'll need to add a new function e.g. 'virDomainDefValidateMemory' call it from virDomainDefValidateInternal and add those checks there.
+ } + return 0; }
@@ -19786,6 +19799,10 @@ virDomainDefParseMemory(virDomainDef *def, &def->mem.max_memory, false, false) < 0) goto error;
+ if (virDomainParseMemory("./memory[1]/@below4g", "./memory[1]/@unit", ctxt, + &def->mem.max_ram_below_4g, false, true) < 0)
This uses the 'unit' which is used for memory for the 'below4g' attribute.
+ goto error; + if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Failed to parse memory slot count")); @@ -28844,6 +28861,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", virTristateSwitchTypeToString(def->mem.dump_core)); + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(buf, " below4g='%llu'", def->mem.max_ram_below_4g); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", virDomainDefGetMemoryTotal(def));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4838687edf..a939d43e93 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2597,6 +2597,9 @@ struct _virDomainMemtune { unsigned long long max_memory; /* in kibibytes */ unsigned int memory_slots; /* maximum count of RAM memory slots */
+ /* maximum memory below the 4GiB boundary (32bit boundary) */ + unsigned long long max_ram_below_4g; /* in kibibytes */ + bool nosharepages; bool locked; int dump_core; /* enum virTristateSwitch */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index be93182092..c69ad781e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6961,6 +6961,10 @@ qemuBuildMachineCommandLine(virCommand *cmd, cfg->dumpGuestCore ? "on" : "off"); }
+ if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(&buf, ",max-ram-below-4g=%llu", + def->mem.max_ram_below_4g * 1024); + if (def->mem.nosharepages) virBufferAddLit(&buf, ",mem-merge=off");
-- 2.24.3 (Apple Git-128)
participants (3)
-
Daniel P. Berrangé -
Peter Krempa -
Zhiyong Ye