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

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Mar 06, 2023 at 05:37:09PM +0200, Max Gurtovoy wrote:
> 
> 
> On 06/03/2023 14:41, Jiri Pirko wrote:
> > Thu, Mar 02, 2023 at 02:05:06PM CET, mst@redhat.com wrote:
> > > The admin virtqueues will be the first interface to issue admin commands.
> > > 
> > > Currently virtio specification defines control virtqueue to manipulate
> > > features and configuration of the device it operates on. However,
> > > control virtqueue commands are device type specific, which makes it very
> > > difficult to extend for device agnostic commands.
> > > 
> > > To support this requirement in a more generic way, this patch introduces
> > > a new admin virtqueue interface.
> > > 
> > > We also support more than one admin virtqueue, for QoS and
> > > scalability requirements.
> > > 
> > > Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > admin.tex   | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > content.tex |  7 +++--
> > > 2 files changed, 79 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/admin.tex b/admin.tex
> > > index 7e28b77..3ffac2e 100644
> > > --- a/admin.tex
> > > +++ b/admin.tex
> > > @@ -155,3 +155,77 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
> > > \field{command_specific_data} and \field{command_specific_result}
> > > depends on these structures and is described separately or is
> > > implicit in the structure description.
> > > +
> > > +\section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> > > +
> > > +An administration virtqueue of an owner device is used to submit
> > > +group administration commands. An owner device can have more
> > 
> > I admit I'm confused. You introduce a concept of admin virtqueue, which
> > sounds quite generic to me, usable in future by much more things than
> > "device groups", correct?
> > 
> > If yes, here you say "group administration commands" which contradics
> > that idea. On multiple places the text this patchset introduces
> > this very muych tights to groups. Like in struct virtio_admin_cmd
> > which contains fields very specific to groups.
> > 
> > If no, isn't it a mistake as the admin queue should be here to
> > handle more than just group commands?
> 
> This was my original idea to have various different types of admin
> operations allowed by an AQ. Not only group management.
> Somehow this was changed down the road when MST took ownership :)
> Maybe it's fine as an initial start but this shouldn't restrict the general
> idea.
> 
> I had in mind things like general health commands, log commands, fw upgrade
>

I'm not sure what if anything unifies these though.  I looked at
specific proposals on the table and this covered them, and
scope was already too broad and was not converging because of this.
People are not good at predicting the future, so I confidently
predict that we'll make some changes down the road ;)


-- 
MST