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?
> > >
> > >
> Device can go to so FATAL state in case a command is stuck and causing
> > >
> internal errors in it.
> > > >
> > > > >
> > > > >
> For your example, stopping a member is possible even it there are some
> > > > >
> errors in the network. You can for example destroy all the connections to
> > > > >
> the remote target and complete all the BIOS with some error.
> > > > >
> > > >
> Forgetting about in-flight requests doesn't necessarily make them go
> > > >
> away. It creates a race between forgotten requests and reconnection. In
> > > >
> the worst case a forgotten write request takes effect after
> > > >
> reconnection, causing data corruption.
> > > >
> > >
> For making it work without data corruption we need a cooperation of the
> > >
> target side for sure. But this is fine since the target in that case is part
> > >
> of the "virtio-blk backend".
> > >
> One solution is that the target can decide it will flush all the requests to
> > >
> the storage device before accepting new connections.
> > >
> >
> This solution shifts the unbounded time from disconnection to
> >
> connection. The Group Member Delete command will complete quickly but a
> >
> subsequent Group Member Create command for the same underlying storage
> >
> device would need to wait until the requests are done.
> > >
> >
> Therefore I think the admin queue must be designed under the assumption
> >
> that some commands take a very long time.
> >
>
> For sure an admin command may take long time. FW upgrade can take 10 minutes
>
> for example.
>
> But each device is free to implement internal logic as he choose.
> >
>
> Same for live migration, when we stop/quiesce a device we must make sure it
>
> doesn't master any DMA operations. Thus, in some implementations we need to
>
> wait for all inflights to end fast. In others, we can invalidate the access
>
> to host/guest memory and wait for completions until the freeze state.
> >
>
> Bottom line, this is device implementation specific consideration.
>
> What I'm asking is that the spec clarifies the command completion order
> semantics (in-order or out-of-order), whether there is a mechanism to
> abort commands, etc.
>
> Device implementers can then take advantage of those aspects to
> implement devices that don't hang (e.g. health monitoring becomes
> unavailable when there is a long running command).
>
> If the spec doesn't cover this, then device implementers will not be
> able to work around it when implementing standard commands like
> create/delete group member.
>
> Does that make sense?
>
> Stefan
It does, to me.
