> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Friday, February 18, 2022 10:06 PM
>
>
> > > and to be defined in future. While making the whole PRE_COPY feature
> > > optional eliminates the concern from mlx5, this is still a complicated arc
> > > to implement and seems prudent to leave it closed until a proper use
> case
> >
> > Can you shed some light on the complexity here?
>
> It is with the data_fd, once a driver enters STOP_COPY it should stuff
> its final state into the data_fd. If this is aborted back to PRE_COPY
> then the data_fd needs to return to streaming changes. Managing this
> transition is not trivial - it is something that has to be signaled to
> the receiver.
>
> There is also something of a race here where the data_fd can reach
> end-of-stream and then the user can do STOP_COPY->PRE_COPY and
> continue stuffing data. This makes the construction of the data stream
> framing "interesting" as there is no longer a possible in-band end of
> stream marker. See the other discussion about async operation why this
> is not ideal.
>
> Basically, it is behavior current qemu doesn't trigger that requires
> significant complexity and testing in any driver to support
> properly. No driver proposed
Make sense.
>
> > > @@ -959,6 +1007,8 @@ struct vfio_device_feature_mig_state {
> > > * above FSM arcs. As there are multiple paths through the FSM arcs the
> > > path
> > > * should be selected based on the following rules:
> > > * - Select the shortest path.
> > > + * - The path cannot have saving group states as interior arcs, only
> > > + * starting/end states.
> >
> > what about PRECOPY->PRECOPY_P2P->STOP_COPY? In this case
> > PRECOPY_P2P is used as interior arc.
>
> It isn't an interior arc because there are only two arcs :) But yes,
> it is bit unclear.
>
> > and if we disallow a non-saving-group state as interior arc when both
> > start and end states are saving-group states (e.g.
> > STOP_COPY->STOP->RUNNING_P2P->PRE_COPY_P2P as I asked in
> > the start) then it might be another rule to be specified...
>
> This isn't a shortest path.
it is the shortest path when STOP_COPY->PRE_COPY_P2P base arc is
not supported. I guess your earlier explanation about data_fd
should be the 3rd rule for why that combination arc is not allowed
in FSM.
>
> > > @@ -972,6 +1022,9 @@ struct vfio_device_feature_mig_state {
> > > * support them. The user can disocver if these states are supported by
> using
> > > * VFIO_DEVICE_FEATURE_MIGRATION. By using combination transitions
> the
> > > user can
> > > * avoid knowing about these optional states if the kernel driver supports
> > > them.
> > > + *
> > > + * Arcs touching PRE_COPY and PRE_COPY_P2P are removed if support
> for
> > > PRE_COPY
> > > + * is not present.
> >
> > why adding this sentence particularly for PRE_COPY? Isn't it already
> > explained by last paragraph for optional states?
>
> Well, I thought it was clarifying about how the optionality is
> constructed.
The last paragraph already says:
+ * The optional states cannot be used with SET_STATE if the device does not
+ * support them. The user can disocver if these states are supported by using
+ * VFIO_DEVICE_FEATURE_MIGRATION. By using combination transitions the user can
+ * avoid knowing about these optional states if the kernel driver supports them.
>
> > > + * Drivers should attempt to return estimates so that initial_bytes +
> > > + * dirty_bytes matches the amount of data an immediate transition to
> > > STOP_COPY
> > > + * will require to be streamed.
> >
> > I didn't understand this requirement. In an immediate transition to
> > STOP_COPY I expect the amount of data covers the entire device
> > state, i.e. initial_bytes. dirty_bytes are dynamic and iteratively returned
> > then why we need set some expectation on the sum of
> > initial+round1_dity+round2_dirty+...
>
> "will require to be streamed" means additional data from this point
> forward, not including anything already sent.
>
> It turns into the estimate of how long STOP_COPY will take.
>
I still didn't get the 'match' part. Why should the amount of data which
has already been sent match the additional data to be sent in STOP_COPY?
Thanks
Keivn
