Re: [PATCH 4/5] virtio-net: Update virtio-net curr_queue_pairs in vdpa backends

[Si-Wei Liu]

On 8/24/2022 11:19 PM, Eugenio Perez Martin wrote:
> On Thu, Aug 25, 2022 at 2:38 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
> > On 8/23/2022 9:27 PM, Jason Wang wrote:
> > > 在 2022/8/20 01:13, Eugenio Pérez 写道:
> > > > It was returned as error before. Instead of it, simply update the
> > > > corresponding field so qemu can send it in the migration data.
> > > >
> > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > ---
> > >
> > > Looks correct.
> > >
> > > Adding Si Wei for double check.
> > Hmmm, I understand why this change is needed for live migration, but
> > this would easily cause userspace out of sync with the kernel for other
> > use cases, such as link down or userspace fallback due to vdpa ioctl
> > error. Yes, these are edge cases.
> The link down case is not possible at this moment because that cvq
> command does not call virtio_net_handle_ctrl_iov.
Right. Though shadow cvq would need to rely on extra ASID support from 
kernel. For the case without shadow cvq we still need to look for an 
alternative mechanism.

> A similar treatment
> than mq would be needed when supported, and the call to
> virtio_net_set_status will be avoided.
So, maybe the seemingly "right" fix for the moment is to prohibit manual 
set_link at all (for vDPA only)? In longer term we'd need to come up 
with appropriate support for applying mq config regardless of asid or 
shadow cvq support.

> I'll double check device initialization ioctl failure with
> n->curr_queue_pairs > 1 in the destination, but I think we should be
> safe.
>
> > Not completely against it, but I
> > wonder if there's a way we can limit the change scope to live migration
> > case only?
> The reason to update the device model is to send the curr_queue_pairs
> to the destination in a backend agnostic way. To send it otherwise
> would limit the live migration possibilities, but sure we can explore
> another way.
A hacky workaround that came off the top of my head was to allow sending 
curr_queue_pairs for the !vm_running case for vdpa. It doesn't look it 
would affect other backend I think. But I agree with Jason, this doesn't 
look decent so I give up on this idea. Hence for this patch,

Acked-by: Si-Wei Liu <si-wei.liu@oracle.com>

> Thanks!