Re: [PATCH for-8.2 6/6] vfio/migration: Allow migration of multiple P2P supporting devices

[Avihai Horon]

On 21/07/2023 15:09, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> On 7/16/23 10:15, Avihai Horon wrote:
> > Now that P2P support has been added to VFIO migration, allow migration
> > of multiple devices if all of them support P2P migration.
> >
> > Single device migration is allowed regardless of P2P migration support.
> >
> > Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > ---
> >   hw/vfio/common.c | 26 ++++++++++++++++++--------
> >   1 file changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index 7c3d636025..753b320739 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -363,21 +363,31 @@ bool vfio_mig_active(void)
> >
> >   static Error *multiple_devices_migration_blocker;
> >
> > -static unsigned int vfio_migratable_device_num(void)
> > +/*
> > + * Multiple devices migration is allowed only if all devices support 
> > P2P
> > + * migration. Single device migration is allowed regardless of P2P 
> > migration
> > + * support.
> > + */
> > +static bool vfio_should_block_multiple_devices_migration(void)
>
> Could we revert the logic and call the routine :
>
>   vfio_multiple_devices_migration_is_supported()
>
> I think it would be clearer in the callers.  This is minor.
Yes, of course, I will change that.

Thanks!

> >   {
> >       VFIOGroup *group;
> >       VFIODevice *vbasedev;
> >       unsigned int device_num = 0;
> > +    bool all_support_p2p = true;
> >
> >       QLIST_FOREACH(group, &vfio_group_list, next) {
> >           QLIST_FOREACH(vbasedev, &group->device_list, next) {
> >               if (vbasedev->migration) {
> >                   device_num++;
> > +
> > +                if (!(vbasedev->migration->mig_flags & 
> > VFIO_MIGRATION_P2P)) {
> > +                    all_support_p2p = false;
> > +                }
> >               }
> >           }
> >       }
> >
> > -    return device_num;
> > +    return !all_support_p2p && device_num > 1;
> >   }
> >
> >   int vfio_block_multiple_devices_migration(VFIODevice *vbasedev, 
> > Error **errp)
> > @@ -385,19 +395,19 @@ int 
> > vfio_block_multiple_devices_migration(VFIODevice *vbasedev, Error 
> > **errp)
> >       int ret;
> >
> >       if (multiple_devices_migration_blocker ||
> > -        vfio_migratable_device_num() <= 1) {
> > +        !vfio_should_block_multiple_devices_migration()) {
> >           return 0;
> >       }
> >
> >       if (vbasedev->enable_migration == ON_OFF_AUTO_ON) {
> > -        error_setg(errp, "Migration is currently not supported with 
> > multiple "
> > -                         "VFIO devices");
> > +        error_setg(errp, "Multiple VFIO devices migration is 
> > supported only if "
> > +                         "all of them support P2P migration");
> >           return -EINVAL;
> >       }
> >
> >       error_setg(&multiple_devices_migration_blocker,
> > -               "Migration is currently not supported with multiple "
> > -               "VFIO devices");
> > +               "Multiple VFIO devices migration is supported only if 
> > all of "
> > +               "them support P2P migration");
> >       ret = migrate_add_blocker(multiple_devices_migration_blocker, 
> > errp);
> >       if (ret < 0) {
> >           error_free(multiple_devices_migration_blocker);
> > @@ -410,7 +420,7 @@ int 
> > vfio_block_multiple_devices_migration(VFIODevice *vbasedev, Error 
> > **errp)
> >   void vfio_unblock_multiple_devices_migration(void)
> >   {
> >       if (!multiple_devices_migration_blocker ||
> > -        vfio_migratable_device_num() > 1) {
> > +        vfio_should_block_multiple_devices_migration()) {
> >           return;
> >       }