On Wed, Mar 02 2022, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: > On Wed, 02 Mar 2022 17:07:21 +0100 > Cornelia Huck <cohuck@xxxxxxxxxx> wrote: > >> On Wed, Mar 02 2022, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: >> >> > On Wed, 2 Mar 2022 10:27:32 -0400 >> > Jason Gunthorpe <jgg@xxxxxxxxxx> 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? > > Note that this function doesn't actually transition the device to > next_fsm, it's only informing the driver what the next state is. > Therefore I think it's reasonable to expect that the caller is never > going to use it's actual internal device state for next_fsm. So I > don't really see a case where we need to worry about preserving > next_fsm in the error condition. Thanks, > > Alex Yeah, I guess any reasonable caller won't try to pass in their internal state. Let's hope that any stupid usuage of that interface is caught during review :)
