Re: [PATCH v3 19/24] vfio/migration: Add x-migration-multifd-transfer VFIO property

[Cédric Le Goater]

On 11/29/24 18:15, Maciej S. Szmigiero wrote:
> On 29.11.2024 15:11, Cédric Le Goater wrote:
> > On 11/17/24 20:20, Maciej S. Szmigiero wrote:
> > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > >
> > > This property allows configuring at runtime whether to transfer the
> > > particular device state via multifd channels when live migrating that
> > > device.
> > >
> > > It defaults to AUTO, which means that VFIO device state transfer via
> > > multifd channels is attempted in configurations that otherwise support it.
> > >
> > > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> > > ---
> > >   hw/core/machine.c             | 1 +
> > >   hw/vfio/pci.c                 | 9 +++++++++
> > >   include/hw/vfio/vfio-common.h | 1 +
> > >   3 files changed, 11 insertions(+)
> > >
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index ed8d39fd769f..fda0f8280edd 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -39,6 +39,7 @@
> > >   GlobalProperty hw_compat_9_1[] = {
> > >       { TYPE_PCI_DEVICE, "x-pcie-ext-tag", "false" },
> > >       { "migration", "send-switchover-start", "off"},
> > > +    { "vfio-pci", "x-migration-multifd-transfer", "off" },
> >
> > Could you please move the compat changes into their own patch ?
> > It's easier for backports
> >
> > >   };
> > >   const size_t hw_compat_9_1_len = G_N_ELEMENTS(hw_compat_9_1);
> > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > > index 14bcc725c301..9d547cb5cdff 100644
> > > --- a/hw/vfio/pci.c
> > > +++ b/hw/vfio/pci.c
> > > @@ -3354,6 +3354,8 @@ static void vfio_instance_init(Object *obj)
> > >       pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> > >   }
> > > +static PropertyInfo qdev_prop_on_off_auto_mutable;
> > > +
> > >   static Property vfio_pci_dev_properties[] = {
> > >       DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
> > >       DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
> > > @@ -3378,6 +3380,10 @@ static Property vfio_pci_dev_properties[] = {
> > >                       VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
> > >       DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
> > >                               vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
> > > +    DEFINE_PROP("x-migration-multifd-transfer", VFIOPCIDevice,
> > > +                vbasedev.migration_multifd_transfer,
> > > +                qdev_prop_on_off_auto_mutable, OnOffAuto,
> > > +                .set_default = true, .defval.i = ON_OFF_AUTO_AUTO),
> >
> > What are you trying to do that DEFINE_PROP_ON_OFF_AUTO() can not satisfy ?
>
> DEFINE_PROP_ON_OFF_AUTO() property isn't runtime-mutable so using it
> would mean that the source VM would need to already decide at startup
> time whether it wants to do a multifd device state transfer.
>
> Source VM can run for a long time before being migrated so it is
> desirable to have a fallback mechanism to the old way of transferring
> VFIO device state if it turns to be necessary for some reason.
>
> After all, ordinary migration parameters can be adjusted at the run time
> too.

I see. I don't think it works this way. Anyhow, it won't compile anymore
with upstream so this part needs to be reworked. Let's keep it in mind
and make it simpler first. That is to rely on values of
vfio_multifd_transfer_supported() and "x-migration-multifd-transfer"

Thanks,

C.