Re: [virtio-comment] Re: [virtio] Re: [PATCH v10 04/10] admin: introduce virtio admin virtqueues

[Stefan Hajnoczi <stefanha@redhat.com>]

On Wed, Mar 08, 2023 at 12:21:46PM -0500, Michael S. Tsirkin wrote:
> On Wed, Mar 08, 2023 at 12:15:23PM -0500, Stefan Hajnoczi wrote:
> > > > > > Or we could say that admin commands must complete within bounded time,
> > > > > > but I'm not sure that is implementable for some device types like
> > > > > > virtio-blk, virtio-scsi, and virtiofs.
> > > > > 
> > > > > No we can't.
> > > > > Some commands, for example FW upgrade can take 10 minutes and it's perfectly
> > > > > fine. Other commands like setting feature bit will take 1 millisec.
> > > > > Each device implements commands in a different internal logic so we can't
> > > > > expect to complete after X time.
> > > > 
> > > > When I say bounded time, I mean that it finishes in a finite amount of
> > > > time. I'm not saying there is a specific time X that all device
> > > > implementations must satisfy. Unbounded means it might never finish.
> > > 
> > > There might be a chance that any command for any virtio device type will
> > > never finish. Nothing new here in the adminq.
> > > 
> > > what one can do is to set a timeout for himself and if this timeout expire -
> > > check the device status. If it needs_reset - do a reset. if status is ok,
> > > then wait some more time.
> > > After X retries, unmap buffers or reset the adminq.
> > 
> > Michael: What effect does resetting the group owner device have on group
> > member devices?
> 
> virtio level reset? It's a good question. I'd expect them all to be
> reset no?
> 
> > I'm concerned that this approach disrupts all group member devices. For
> > example, you try to add a new device but the command hangs. In order to
> > recover you now have to reset the group owner device and this breaks all
> > the group member devices.
> 
> 
> I agree. How about a VQ level reset though? Seems like exactly
> what's needed here?

Yes, a new virtqueue-level reset feature would take care of this case.

Stefan

Attachment: signature.asc
Description: PGP signature