[pve-devel] [PATCH docs/qemu-server 0/2] Ballooning/PCI passthrough incompatibility: add warning and docs

[pve-devel] [PATCH docs/qemu-server 0/2] Ballooning/PCI passthrough incompatibility: add warning and docs

[Friedrich Weber]

When using PCI(e) passthrough, enabling ballooning for a VM (as in, setting a
"minimum memory" amount that is smaller than "memory") does not have any
effect, which may be surprising to users [0].

Patch #1 adds a note about this incompatibility to the PCI(e) passthrough and
ballooning sections of the docs.

Patch #2 implements a warning on VM start if PCI(e) passthrough and
ballooning are both enabled.

[0] https://forum.proxmox.com/threads/134202/



docs:

Friedrich Weber (1):
  pci passthrough: mention incompatibility with ballooning

 qm-pci-passthrough.adoc | 10 ++++++++++
 qm.adoc                 |  5 +++++
 2 files changed, 15 insertions(+)


qemu-server:

Friedrich Weber (1):
  vm start: warn if using ballooning and PCI(e) passthrough

 PVE/QemuServer.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)


Summary over all repositories:
  3 files changed, 25 insertions(+), 0 deletions(-)

-- 
murpp v0.4.0

[pve-devel] [PATCH docs 1/2] pci passthrough: mention incompatibility with ballooning

[Friedrich Weber]

When using PCI(e) passthrough, setting a minimum amount of memory does
not have any effect, which may be surprising to users [0]. Add a note
to the PCI(e) passthrough section, and reference it in the ballooning
section.

[0] https://forum.proxmox.com/threads/134202/

Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
 qm-pci-passthrough.adoc | 10 ++++++++++
 qm.adoc                 |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
index 1f68721..406156d 100644
--- a/qm-pci-passthrough.adoc
+++ b/qm-pci-passthrough.adoc
@@ -20,6 +20,16 @@ PCI speeds. Passing through devices as PCIe just sets a flag for the guest to
 tell it that the device is a  PCIe device instead of a "really fast legacy PCI
 device". Some guest applications benefit from this.
 
+xref:qm_ballooning[Automatic memory allocation (ballooning)] is not possible
+when using PCI(e) passthrough. As the PCI device may use DMA (Direct Memory
+Access), QEMU needs to map the complete guest memory on VM startup. Hence, the
+QEMU process will use at least the (maximum) configured amount of VM memory and
+setting a minimum amount does not have any effect. When using PCI(e)
+passthrough, it is recommended to set memory and minimum memory to the same
+amount and keep the balloning device enabled. However, keep in mind that the
+memory consumption reported in the GUI for the VM may be much lower than the
+memory consumption of the QEMU process.
+
 General Requirements
 ~~~~~~~~~~~~~~~~~~~~
 
diff --git a/qm.adoc b/qm.adoc
index b7938d7..cbc5e0d 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -623,6 +623,7 @@ it (like for debugging purposes), simply uncheck *Ballooning Device* or set
 
 in the configuration.
 
+[[qm_ballooning]]
 .Automatic Memory Allocation
 
 // see autoballoon() in pvestatd.pm
@@ -659,6 +660,10 @@ systems.
 When allocating RAM to your VMs, a good rule of thumb is always to leave 1GB
 of RAM available to the host.
 
+When using PCI(e) passthrough, setting a minimum amount of memory does not have
+any effect, see the section on xref:qm_pci_passthrough[PCI(e) passthrough] for
+more details.
+
 
 [[qm_network_device]]
 Network Device
-- 
2.39.2

Re: [pve-devel] [PATCH docs 1/2] pci passthrough: mention incompatibility with ballooning

[Fiona Ebner]

Am 13.11.23 um 18:09 schrieb Friedrich Weber:
>  
> +xref:qm_ballooning[Automatic memory allocation (ballooning)] is not possible
> +when using PCI(e) passthrough. As the PCI device may use DMA (Direct Memory
> +Access), QEMU needs to map the complete guest memory on VM startup. Hence, the
> +QEMU process will use at least the (maximum) configured amount of VM memory and
> +setting a minimum amount does not have any effect. When using PCI(e)
> +passthrough, it is recommended to set memory and minimum memory to the same
> +amount and keep the balloning device enabled. However, keep in mind that the

typo: s/balloning/ballooning/

Is there any advantage to keeping the ballooning device enabled?

> +memory consumption reported in the GUI for the VM may be much lower than the
> +memory consumption of the QEMU process.

Maybe mention what the reported value is?

Re: [pve-devel] [PATCH docs 1/2] pci passthrough: mention incompatibility with ballooning

[Friedrich Weber]

On 14/11/2023 09:30, Fiona Ebner wrote:
> Am 13.11.23 um 18:09 schrieb Friedrich Weber:
>>  
>> +xref:qm_ballooning[Automatic memory allocation (ballooning)] is not possible
>> +when using PCI(e) passthrough. As the PCI device may use DMA (Direct Memory
>> +Access), QEMU needs to map the complete guest memory on VM startup. Hence, the
>> +QEMU process will use at least the (maximum) configured amount of VM memory and
>> +setting a minimum amount does not have any effect. When using PCI(e)
>> +passthrough, it is recommended to set memory and minimum memory to the same
>> +amount and keep the balloning device enabled. However, keep in mind that the
> 
> typo: s/balloning/ballooning/

Oops, thanks.

> Is there any advantage to keeping the ballooning device enabled?
> 
>> +memory consumption reported in the GUI for the VM may be much lower than the
>> +memory consumption of the QEMU process.
> 
> Maybe mention what the reported value is?

In my understanding: If the ballooning device is enabled (and a balloon
driver present in the guest), the VM memory usage numbers are taken as
reported by the balloon driver [1] (I'd say "as seen from within the
guest"?). If the ballooning device is disabled, they are inferred from
the rss and vsize of the QEMU process [2], so I'd say "as seen from the
host".

I guess in the end the user has to decide which perspective they care
about? I'll try to make this clearer in the v2.

[1]
https://git.proxmox.com/?p=qemu-server.git;a=blob;f=PVE/QemuServer.pm;h=c465fb6f64ae30dec5112fc4439f9181c2eba4e9;hb=feb51881d#l3013
[2]
https://git.proxmox.com/?p=qemu-server.git;a=blob;f=PVE/QemuServer.pm;h=c465fb6f64ae30dec5112fc4439f9181c2eba4e9;hb=feb51881d#l2967

Re: [pve-devel] [PATCH docs 1/2] pci passthrough: mention incompatibility with ballooning

[Friedrich Weber]

I took another look at ballooning+PCI passthrough and reporting of
memory usage with Markus. Apparently QEMU does not *always* map the
complete guest memory -- at least it didn't with a passed-through NIC.
So both the warning as well as the docs section may be worded too
strongly ("Ballooning is not possible", "VM will use maximum configured
memory", "QEMU needs to map" ...). I'll have to take another look at
this to see how we can phrase this correctly (and hopefully somewhat
precisely) for v2.

On 14/11/2023 11:20, Friedrich Weber wrote:
> On 14/11/2023 09:30, Fiona Ebner wrote:
>> Am 13.11.23 um 18:09 schrieb Friedrich Weber:
>>>  
>>> +xref:qm_ballooning[Automatic memory allocation (ballooning)] is not possible
>>> +when using PCI(e) passthrough. As the PCI device may use DMA (Direct Memory
>>> +Access), QEMU needs to map the complete guest memory on VM startup. Hence, the
>>> +QEMU process will use at least the (maximum) configured amount of VM memory and
>>> +setting a minimum amount does not have any effect. When using PCI(e)
>>> +passthrough, it is recommended to set memory and minimum memory to the same
>>> +amount and keep the balloning device enabled. However, keep in mind that the
>>
>> typo: s/balloning/ballooning/
> 
> Oops, thanks.
> 
>> Is there any advantage to keeping the ballooning device enabled?
>>
>>> +memory consumption reported in the GUI for the VM may be much lower than the
>>> +memory consumption of the QEMU process.
>>
>> Maybe mention what the reported value is?
> 
> In my understanding: If the ballooning device is enabled (and a balloon
> driver present in the guest), the VM memory usage numbers are taken as
> reported by the balloon driver [1] (I'd say "as seen from within the
> guest"?). If the ballooning device is disabled, they are inferred from
> the rss and vsize of the QEMU process [2], so I'd say "as seen from the
> host".
> 
> I guess in the end the user has to decide which perspective they care
> about? I'll try to make this clearer in the v2.
> 
> [1]
> https://git.proxmox.com/?p=qemu-server.git;a=blob;f=PVE/QemuServer.pm;h=c465fb6f64ae30dec5112fc4439f9181c2eba4e9;hb=feb51881d#l3013
> [2]
> https://git.proxmox.com/?p=qemu-server.git;a=blob;f=PVE/QemuServer.pm;h=c465fb6f64ae30dec5112fc4439f9181c2eba4e9;hb=feb51881d#l2967
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] [PATCH qemu-server 2/2] vm start: warn if using ballooning and PCI(e) passthrough

[Friedrich Weber]

If a VM uses PCI(e) passthrough, ballooning does not work as expected:
The QEMU process will always consume the full memory amount given in
`memory`, even if `balloon` is set to a smaller (non-zero) amount. The
reason is that the PCI device might use DMA, so QEMU needs to map the
complete guest memory on startup. However, users may not be aware of
that (see e.g. [0]).

To make users aware of the limitation, warn on VM start if at least
one PCI device is passed through and ballooning is enabled (and
`balloon` != `memory`).

[0] https://forum.proxmox.com/threads/134202/

Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---

Notes:
    I did not test this on a "real" PCI passthrough setup as I don't have
    one at hand, but Markus tested (an earlier version) of this patch on
    his machine.

 PVE/QemuServer.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index dbcd568..70983a4 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5789,6 +5789,16 @@ sub vm_start_nolock {
 	die $err;
     }
 
+    if (
+	scalar(%$pci_devices)
+	&& defined($conf->{balloon})
+	&& $conf->{balloon} != 0
+	&& $conf->{balloon} != $memory
+    ) {
+	log_warn("Ballooning is not possible when using PCI(e) passthrough, "
+		."VM will use maximum configured memory ($memory MiB).\n");
+    }
+
     PVE::Storage::activate_volumes($storecfg, $vollist);
 
 
-- 
2.39.2

Re: [pve-devel] [PATCH qemu-server 2/2] vm start: warn if using ballooning and PCI(e) passthrough

[Fiona Ebner]

Am 13.11.23 um 18:09 schrieb Friedrich Weber:
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index dbcd568..70983a4 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5789,6 +5789,16 @@ sub vm_start_nolock {
>  	die $err;
>      }
>  
> +    if (
> +	scalar(%$pci_devices)

Style nit: we usually check for scalar(keys ...)

> +	&& defined($conf->{balloon})
> +	&& $conf->{balloon} != 0

It's nice being explicit, but replacing the above two with just
$conf->{balloon}, the whole if expression would fit in one line rather
than six. Either way is fine by me, but the vm_start_nolock function is
already pretty long. Also might make sense to move the PCI stuff into a
helper, if you'd like to give it a shot. But not a requirement for this
patch.

> +	&& $conf->{balloon} != $memory
> +    ) {
> +	log_warn("Ballooning is not possible when using PCI(e) passthrough, "
> +		."VM will use maximum configured memory ($memory MiB).\n");

Trailing newline is not required for log_warn().

Style nit: wrong indentation, we don't usually align text like this. It
should just be one more indentation for the second line.

All just nits, so:
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

Re: [pve-devel] [PATCH qemu-server 2/2] vm start: warn if using ballooning and PCI(e) passthrough

[Friedrich Weber]

Thanks for the review! I'll send a v2.

On 14/11/2023 10:13, Fiona Ebner wrote:
> Am 13.11.23 um 18:09 schrieb Friedrich Weber:
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index dbcd568..70983a4 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -5789,6 +5789,16 @@ sub vm_start_nolock {
>>  	die $err;
>>      }
>>  
>> +    if (
>> +	scalar(%$pci_devices)
> 
> Style nit: we usually check for scalar(keys ...)

Will change this.

>> +	&& defined($conf->{balloon})
>> +	&& $conf->{balloon} != 0
> 
> It's nice being explicit, but replacing the above two with just
> $conf->{balloon}, the whole if expression would fit in one line rather
> than six. Either way is fine by me, but the vm_start_nolock function is
> already pretty long.

I opted for being explicit here, but I see the point that the function
is pretty long already and six lines for a relatively unimportant check
is a bit excessive. :) So I'll change this.

> Also might make sense to move the PCI stuff into a
> helper, if you'd like to give it a shot. But not a requirement for this
> patch.

I'll probably keep it at just adding the warning for this patch series.

> 
>> +	&& $conf->{balloon} != $memory
>> +    ) {
>> +	log_warn("Ballooning is not possible when using PCI(e) passthrough, "
>> +		."VM will use maximum configured memory ($memory MiB).\n");
> 
> Trailing newline is not required for log_warn().
> 
> Style nit: wrong indentation, we don't usually align text like this. It
> should just be one more indentation for the second line.

Good catches, thanks!