Re: [pve-devel] [PATCH guest-common 1/2] mapping: pci: add 'live-migration-capable' flag to mappings

[Fiona Ebner <f.ebner@proxmox.com>]

Am 18.03.24 um 12:18 schrieb Dominik Csapak:
> so that we can decide in qemu-server to allow live-migration.
> the driver and qemu must be capable of that, and it's the
> admins responsibility to know and configure that
> 

Nit: "The" and "QEMU" should be capitalized like this. "admins" ->
"admin's". Missing period at the end.

> Mark the option as experimental in the description.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Mapping/PCI.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
> index 19ace98..0866175 100644
> --- a/src/PVE/Mapping/PCI.pm
> +++ b/src/PVE/Mapping/PCI.pm
> @@ -100,8 +100,16 @@ my $defaultData = {
>  	    maxLength => 4096,
>  	},
>  	mdev => {
> +	    description => "Marks the device(s) as being capable of providing mediated devices.",
>  	    type => 'boolean',
>  	    optional => 1,
> +	    default => 0,
> +	},

Above should be its own patch. Most likely, I'm missing it, but where
exactly does the 'mdev' property from the mapping have an effect? Just
in the UI? At least telling from 'qm showcmd', the 'mdev' property for a
'hostpciN' VM config entry will not be ignored even if the mapping has
'mdev=0'. And it's also possible to run 'qm set 112 --hostpci0
mapping=bar,mdev=foo' without any warning if the mapping has 'mdev=0'.

> +	'live-migration-capable' => {
> +	    description => "Marks the device(s) as being able to be live-migrated (Experimental).",

The bit about QEMU and the driver needing to support it should be
mentioned here.

> +	    type => 'boolean',
> +	    optional => 1,
> +	    default => 0,
>  	},
>  	map => {
>  	    type => 'array',
> @@ -123,6 +131,7 @@ sub options {
>      return {
>  	description => { optional => 1 },
>  	mdev => { optional => 1 },
> +	'live-migration-capable' => { optional => 1 },
>  	map => {},
>      };
>  }