On 3/22/24 14:37, Fiona Ebner wrote:
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.csa...@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'.


yeah sorry, i added that but overlooked it when adding the hunks...

AFAIR i intended to use the mdev property to safeguard the mapping
like we do with the iommu groups, so when it changes, warn the user that
the mapping changed (using it with mdevs wouldn't work anyway if that
was set but the device wouldn't provide one)

aside from that, yes it currently only has effects on the gui,
do we maybe want to make that stricter? (would be a breaking change imho)

+       '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 => {},
      };
  }



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

Reply via email to