4:33 a.m.
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(a)bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei(a)bytedance.com>
Signed-off-by: zhangruien <zhangruien(a)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)
2:55 a.m.
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(a)bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei(a)bytedance.com>
Signed-off-by: zhangruien <zhangruien(a)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)
3:20 a.m.
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 :|
3:56 a.m.
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.
10:13 p.m.
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(a)bytedance.com>
> Signed-off-by: zhenwei pi <pizhenwei(a)bytedance.com>
> Signed-off-by: zhangruien <zhangruien(a)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)
>
1:53 a.m.
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(a)bytedance.com>
> Signed-off-by: zhenwei pi <pizhenwei(a)bytedance.com>
> Signed-off-by: zhangruien <zhangruien(a)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)
>
1305
days inactive
1307
days old
5 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Peter Krempa -
Zhiyong Ye