Re: [pve-devel] [PATCH qemu-server v3 07/10] pci: set 'enable-migration' to on for live-migration marked mapped devices

2024-06-05 Thread Fiona Ebner 
Am 05.06.24 um 10:51 schrieb Dominik Csapak:
> On 5/31/24 14:56, Fiona Ebner wrote:
>> Am 19.04.24 um 14:45 schrieb Dominik Csapak:
>>> the default is 'auto', but for those which are marked as capable for
>>> live migration, we want to explicitly enable that, so we get an early
>>> error on start if the driver does not support that.
>>>
>>> Signed-off-by: Dominik Csapak 
>>> ---
>>>   PVE/QemuServer/PCI.pm | 9 -
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
>>> index 6ba43ee8..df2ea3eb 100644
>>> --- a/PVE/QemuServer/PCI.pm
>>> +++ b/PVE/QemuServer/PCI.pm
>>> @@ -435,8 +435,11 @@ sub parse_hostpci {
>>>   my $devices = PVE::Mapping::PCI::get_node_mapping($config,
>>> $mapping, $node);
>>>   die "PCI device mapping not found for '$mapping'\n" if
>>> !$devices || !scalar($devices->@*);
>>>   +    my $cfg = $config->{ids}->{$mapping};
>>> +    $res->{'live-migration-capable'} = 1 if
>>> $cfg->{'live-migration-capable'};
>>> +
>>>   for my $device ($devices->@*) {
>>> -    eval { PVE::Mapping::PCI::assert_valid($mapping, $device,
>>> $config->{ids}->{$mapping}) };
>>> +    eval { PVE::Mapping::PCI::assert_valid($mapping, $device,
>>> $cfg) };
>>
>>
>> This hunk belongs in patch 03/10)
> 
> why though?
> 
> in 03/10 i adapt to the new $cfg parameter to check the mdev parameter,
> and here i refactor it because we have to access it twice
> (for the live-migration-capable flag)
> 
> i mean i can pull it out into the $cfg before, but the the live-migration
> part does IMO not belong in 03/10
> 

Oh, sorry, I somehow thought this was a fix-up for the assert_valid()
call. You're right, is fine as it is.


___
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 v3 07/10] pci: set 'enable-migration' to on for live-migration marked mapped devices

2024-06-05 Thread Dominik Csapak 

On 5/31/24 14:56, Fiona Ebner wrote:

Am 19.04.24 um 14:45 schrieb Dominik Csapak:

the default is 'auto', but for those which are marked as capable for
live migration, we want to explicitly enable that, so we get an early
error on start if the driver does not support that.

Signed-off-by: Dominik Csapak 
---
  PVE/QemuServer/PCI.pm | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 6ba43ee8..df2ea3eb 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -435,8 +435,11 @@ sub parse_hostpci {
my $devices = PVE::Mapping::PCI::get_node_mapping($config, $mapping, 
$node);
die "PCI device mapping not found for '$mapping'\n" if !$devices || 
!scalar($devices->@*);
  
+	my $cfg = $config->{ids}->{$mapping};

+   $res->{'live-migration-capable'} = 1 if 
$cfg->{'live-migration-capable'};
+
for my $device ($devices->@*) {
-   eval { PVE::Mapping::PCI::assert_valid($mapping, $device, 
$config->{ids}->{$mapping}) };
+   eval { PVE::Mapping::PCI::assert_valid($mapping, $device, $cfg) };



This hunk belongs in patch 03/10)


why though?

in 03/10 i adapt to the new $cfg parameter to check the mdev parameter,
and here i refactor it because we have to access it twice
(for the live-migration-capable flag)

i mean i can pull it out into the $cfg before, but the the live-migration
part does IMO not belong in 03/10




die "PCI device mapping invalid (hardware probably changed): $@\n" 
if $@;
push $alternatives->@*, [split(/;/, $device->{path})];
}
@@ -635,6 +638,10 @@ sub print_hostpci_devices {
$devicestr .= ",host=$pcidevice->{id}";
}
  
+	if ($d->{'live-migration-capable'}) {

+   $devicestr .= ",enable-migration=on"
+   }
+
my $mf_addr = $multifunction ? ".$j" : '';
$devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
  




___
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 v3 07/10] pci: set 'enable-migration' to on for live-migration marked mapped devices

2024-05-31 Thread Fiona Ebner 
Am 19.04.24 um 14:45 schrieb Dominik Csapak:
> the default is 'auto', but for those which are marked as capable for
> live migration, we want to explicitly enable that, so we get an early
> error on start if the driver does not support that.
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/QemuServer/PCI.pm | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index 6ba43ee8..df2ea3eb 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -435,8 +435,11 @@ sub parse_hostpci {
>   my $devices = PVE::Mapping::PCI::get_node_mapping($config, $mapping, 
> $node);
>   die "PCI device mapping not found for '$mapping'\n" if !$devices || 
> !scalar($devices->@*);
>  
> + my $cfg = $config->{ids}->{$mapping};
> + $res->{'live-migration-capable'} = 1 if 
> $cfg->{'live-migration-capable'};
> +
>   for my $device ($devices->@*) {
> - eval { PVE::Mapping::PCI::assert_valid($mapping, $device, 
> $config->{ids}->{$mapping}) };
> + eval { PVE::Mapping::PCI::assert_valid($mapping, $device, $cfg) };


This hunk belongs in patch 03/10

>   die "PCI device mapping invalid (hardware probably changed): $@\n" 
> if $@;
>   push $alternatives->@*, [split(/;/, $device->{path})];
>   }
> @@ -635,6 +638,10 @@ sub print_hostpci_devices {
>   $devicestr .= ",host=$pcidevice->{id}";
>   }
>  
> + if ($d->{'live-migration-capable'}) {
> + $devicestr .= ",enable-migration=on"
> + }
> +
>   my $mf_addr = $multifunction ? ".$j" : '';
>   $devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
>  


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

[pve-devel] [PATCH qemu-server v3 07/10] pci: set 'enable-migration' to on for live-migration marked mapped devices

2024-04-19 Thread Dominik Csapak 
the default is 'auto', but for those which are marked as capable for
live migration, we want to explicitly enable that, so we get an early
error on start if the driver does not support that.

Signed-off-by: Dominik Csapak 
---
 PVE/QemuServer/PCI.pm | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index 6ba43ee8..df2ea3eb 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -435,8 +435,11 @@ sub parse_hostpci {
my $devices = PVE::Mapping::PCI::get_node_mapping($config, $mapping, 
$node);
die "PCI device mapping not found for '$mapping'\n" if !$devices || 
!scalar($devices->@*);
 
+   my $cfg = $config->{ids}->{$mapping};
+   $res->{'live-migration-capable'} = 1 if 
$cfg->{'live-migration-capable'};
+
for my $device ($devices->@*) {
-   eval { PVE::Mapping::PCI::assert_valid($mapping, $device, 
$config->{ids}->{$mapping}) };
+   eval { PVE::Mapping::PCI::assert_valid($mapping, $device, $cfg) };
die "PCI device mapping invalid (hardware probably changed): $@\n" 
if $@;
push $alternatives->@*, [split(/;/, $device->{path})];
}
@@ -635,6 +638,10 @@ sub print_hostpci_devices {
$devicestr .= ",host=$pcidevice->{id}";
}
 
+   if ($d->{'live-migration-capable'}) {
+   $devicestr .= ",enable-migration=on"
+   }
+
my $mf_addr = $multifunction ? ".$j" : '';
$devicestr .= ",id=${id}${mf_addr}${pciaddr}${mf_addr}";
 
-- 
2.39.2



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