Re: [PATCH V9 mlx5-next 09/15] vfio: Define device migration protocol v2

[Cornelia Huck <cohuck@redhat.com>]

On Wed, Mar 02 2022, Alex Williamson <alex.williamson@redhat.com> wrote:

> On Wed, 2 Mar 2022 10:27:32 -0400
> Jason Gunthorpe <jgg@nvidia.com> wrote:
>
>> On Wed, Mar 02, 2022 at 12:19:20PM +0100, Cornelia Huck wrote:
>> > > +/*
>> > > + * vfio_mig_get_next_state - Compute the next step in the FSM
>> > > + * @cur_fsm - The current state the device is in
>> > > + * @new_fsm - The target state to reach
>> > > + * @next_fsm - Pointer to the next step to get to new_fsm
>> > > + *
>> > > + * Return 0 upon success, otherwise -errno
>> > > + * Upon success the next step in the state progression between cur_fsm and
>> > > + * new_fsm will be set in next_fsm.  
>> > 
>> > What about non-success? Can the caller make any assumption about
>> > next_fsm in that case? Because...  
>> 
>> I checked both mlx5 and acc, both properly ignore the next_fsm value
>> on error. This oddness aros when Alex asked to return an errno instead
>> of the state value.
>
> Right, my assertion was that only the driver itself should be able to
> transition to the ERROR state.  vfio_mig_get_next_state() should never
> advise the driver to go to the error state, it can only report that a
> transition is invalid.  The driver may stay in the current state if an
> error occurs here, which is why we added the ability to get the device
> state.  Thanks,
>
> Alex

So, should the function then write anything to next_fsm if it returns
-errno? (Maybe I'm misunderstanding.) Or should the caller always expect
that something may be written to new_fsm, and simply only look at it if
the function returns success?

(I think that the code as-is is likely ok, I just want to make sure I'm
not missing something. Apologies if that seems nitpicky.)