RE: [PATCH v10 03/10] admin: introduce group administration commands

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Sunday, February 12, 2023 4:50 AM
> 
> On Sat, Feb 11, 2023 at 06:50:37PM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, February 9, 2023 7:14 AM
> >
> > [..]
> >
> > > +Group administration commands can be issued through an owner device
> > > +to control member devices of a group.
> > can be issued by the owner device to control ...
> 
> Well it's clearly the driver that issues the commands. Don't see how
> can a device issue them. If instead of "through an owner device"
> you prefer a more verbose "the driver of an owner device" then
> I can live with that. Let me know.

"through" wording is convoluted and it hints that someone else is preparing the command and its tunneled through the admin vq.
Which is clearly not the scope here.

How about below simple statements like below?

Group administrative commands control the member devices.
The driver sends group administrative commands to the owner device.

> > General spec structure is describing the "rules" as device and driver side
> requirements.
> > Can you please move above rule paragraph belongs to device and driver
> requirements section?
> 
> Generally we have a bit of duplication. A high level description
> showing how things work together. Then per-device/driver conforumance
> statements. So I think this should stay but I will look at adding
> conformance clauses too.
Ok. that will be good.
Yes. General description is good without the "rule" wording so that we have clear separation of conformance and description.

> > I am not sure it should be intentional and mention of Linux.
> > Any OS would need multiple error values to know the error cause.
> > Linux doesn't have name "OK" either for well-known value of 0.
> >
> > For example
> > $ errno 22
> > EINVAL 22 Invalid argument
> >
> > $ errno 0
> > This has not output for it and doesn't exist in errno.h.
> 
> No name OK true, but it does use 0 to signal success sometimes:
Yes, but the wording is "success" and not "ok". :)

> 
> https://man7.org/linux/man-pages/man3/errno.3.html
> 
> For some system calls and library functions (e.g.,
>        getpriority(2)), -1 is a valid return on success.  In such cases,
>        a successful return can be distinguished from an error return by
>        setting errno to zero before the call, and then, if the call
>        returns a status that indicates that an error may have occurred,
>        checking to see if errno has a nonzero value.
> 
> 
> 
> > Description is already good enough to describe what they are.
> > Can we please drop Linux wording?
> 
> But why should we? 
Because of following reasons.

1. If we start putting 22 with Linux annotations, in few weeks virtio Net device section will be full of annotation of ethtool, tc, ip config and more for Linux developers.
For example, the current two interrupt moderation patches need to write ethtool options details to match to following this Linux example here.
It is better to avoid such things and keep spec OS agnostics, even though it is addressing and fitting into the Linux use cases.

2. Virtio error code to Linux error code switch-case is simple routine to have.

3. Every time virtio spec to refer to errno.h for finding right value is opposite of what spec may want to achieve.
If an error code doesn't match to errno.h, now spec developers and reviewers to review what is not defined by errno.h to find as unique value.

3.1 And if that is taken in future by errno.h for something else?

All of this is not worth a simple switch case statement to deal with.