[pve-devel] [PATCH qemu-server] fix #5543: pci: don't use pci devices when starting templates

[pve-devel] [PATCH qemu-server] fix #5543: pci: don't use pci devices when starting templates

[Dominik Csapak]

templates can only be started in context of a pbs backup, and there
we don't need or want to use the configured pci devices, since
they cannot be started normally anyway.

Without this, users can get into a situation where the template
cannot be backed up when the pci device is not available, even
if it's actually not needed.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm     | 3 +++
 PVE/QemuServer/PCI.pm | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7815b608..c1cc27f3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6137,6 +6137,9 @@ sub get_vm_volumes {
 sub cleanup_pci_devices {
     my ($vmid, $conf) = @_;
 
+    # templates don't use pci devices
+    return if $conf->{template};
+
     foreach my $key (keys %$conf) {
 	next if $key !~ m/^hostpci(\d+)$/;
 	my $hostpciindex = $1;
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 1673041b..4aa4a52c 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -576,9 +576,13 @@ sub print_hostpci_devices {
     my $kvm_off = 0;
     my $gpu_passthrough = 0;
     my $legacy_igd = 0;
+    my $pci_devices = {};
+
+    # templates can only start for backup where we don't want pci devices, so return early
+    return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) if $conf->{template};
 
     my $pciaddr;
-    my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
+    $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
 
     for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
 	my $id = "hostpci$i";
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server] fix #5543: pci: don't use pci devices when starting templates

[Fiona Ebner]

I'd be in favor of fixing this together with
https://bugzilla.proxmox.com/show_bug.cgi?id=3352
and use a minimal QEMU commandline that only attaches the block devices
and nothing else. I guess the addition for the PCI cleanup is still
needed, but getting the minimal commandline is probably best done as a
new helper. What do you think?

Am 19.06.24 um 08:37 schrieb Dominik Csapak:
> templates can only be started in context of a pbs backup, and there
> we don't need or want to use the configured pci devices, since
> they cannot be started normally anyway.
> 
> Without this, users can get into a situation where the template
> cannot be backed up when the pci device is not available, even
> if it's actually not needed.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/QemuServer.pm     | 3 +++
>  PVE/QemuServer/PCI.pm | 6 +++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 7815b608..c1cc27f3 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6137,6 +6137,9 @@ sub get_vm_volumes {
>  sub cleanup_pci_devices {
>      my ($vmid, $conf) = @_;
>  
> +    # templates don't use pci devices
> +    return if $conf->{template};
> +
>      foreach my $key (keys %$conf) {
>  	next if $key !~ m/^hostpci(\d+)$/;
>  	my $hostpciindex = $1;
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 1673041b..4aa4a52c 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -576,9 +576,13 @@ sub print_hostpci_devices {
>      my $kvm_off = 0;
>      my $gpu_passthrough = 0;
>      my $legacy_igd = 0;
> +    my $pci_devices = {};
> +
> +    # templates can only start for backup where we don't want pci devices, so return early
> +    return ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) if $conf->{template};
>  
>      my $pciaddr;
> -    my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
> +    $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
>  
>      for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
>  	my $id = "hostpci$i";


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server] fix #5543: pci: don't use pci devices when starting templates

[Fiona Ebner]

Am 19.06.24 um 10:14 schrieb Fiona Ebner:
> I'd be in favor of fixing this together with
> https://bugzilla.proxmox.com/show_bug.cgi?id=3352
> and use a minimal QEMU commandline that only attaches the block devices
> and nothing else. I guess the addition for the PCI cleanup is still
> needed, but getting the minimal commandline is probably best done as a
> new helper. What do you think?
> 

Another way would be to reduce the config we pass to config_to_command
to only the block devices and 1 CPU, etc.. Maybe that's more
straight-forward than a new helper.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server] fix #5543: pci: don't use pci devices when starting templates

[Dominik Csapak]

On 6/19/24 10:17, Fiona Ebner wrote:
> Am 19.06.24 um 10:14 schrieb Fiona Ebner:
>> I'd be in favor of fixing this together with
>> https://bugzilla.proxmox.com/show_bug.cgi?id=3352
>> and use a minimal QEMU commandline that only attaches the block devices
>> and nothing else. I guess the addition for the PCI cleanup is still
>> needed, but getting the minimal commandline is probably best done as a
>> new helper. What do you think?
>>
> 
> Another way would be to reduce the config we pass to config_to_command
> to only the block devices and 1 CPU, etc.. Maybe that's more
> straight-forward than a new helper.


yeah, some combined fix for that would be good, did not have that bug on my
radar.

I agree that 'minifying' the config is probably the way to go here,
but as you said, the cleanup should also be touched so it does not
throw warnings for not available devices etc.

I think we could have a set of keys as a whitelist to copy, and leave all
others default, i think for starters we should whitelist:

* all disk configs (excl. cdroms)
* the kvm setting
* anything else ?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server] fix #5543: pci: don't use pci devices when starting templates

[Fiona Ebner]

Am 19.06.24 um 10:28 schrieb Dominik Csapak:
> On 6/19/24 10:17, Fiona Ebner wrote:
>> Am 19.06.24 um 10:14 schrieb Fiona Ebner:
>>> I'd be in favor of fixing this together with
>>> https://bugzilla.proxmox.com/show_bug.cgi?id=3352
>>> and use a minimal QEMU commandline that only attaches the block devices
>>> and nothing else. I guess the addition for the PCI cleanup is still
>>> needed, but getting the minimal commandline is probably best done as a
>>> new helper. What do you think?
>>>
>>
>> Another way would be to reduce the config we pass to config_to_command
>> to only the block devices and 1 CPU, etc.. Maybe that's more
>> straight-forward than a new helper.
> 
> 
> yeah, some combined fix for that would be good, did not have that bug on my
> radar.
> 
> I agree that 'minifying' the config is probably the way to go here,
> but as you said, the cleanup should also be touched so it does not
> throw warnings for not available devices etc.
> 
> I think we could have a set of keys as a whitelist to copy, and leave all
> others default, i think for starters we should whitelist:
> 
> * all disk configs (excl. cdroms)
> * the kvm setting
> * anything else ?
>

I don't think anything other than the disks (including TPM+EFI of
course) is needed for backup. I mean, we could also disable KVM: no vCPU
will run, but QEMU will complain if you try to start a template with KVM
enabled on a node that does not support hardware virtualization (there
probably are not many of those around, but it's not impossible).


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel