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

[Max Gurtovoy <mgurtovoy@nvidia.com>]

On 03/03/2023 1:47, Michael S. Tsirkin wrote:
> On Thu, Mar 02, 2023 at 06:40:29PM +0000, Parav Pandit wrote:
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, March 2, 2023 8:05 AM
> >
> > > +When \field{status} is VIRTIO_ADMIN_STATUS_OK, \field{status_qialifier}
> > > +is reserved and set to zero by the device.
> > > +
> > s/status_qialifier/status_qualifier
> > Missed from v10 of Feb.
> >
> > > +When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL, the following table
> > > +describes possible \field{status_qialifier} values:
> > s/status_qialifier/status_qualifier
> >
> > Can you please add other useful error codes in addition to the EINVAL?
> > Few that we are needed EAGAIN, ENOMEM, EBUSY, ENODEV.
>
> We'll add them when there's a use-case.

Can you please add at least the EAGAIN in v11 ?
HW devices my be in a transitional state in which the command can't 
succeed in time T but the same command may succeed if it retried in time T'.

This is very common in linux as well and it's will improve the virtio AQ 
spec.

We as device developers need more that one type of error.

Definition:
VIRTIO_ADMIN_STATUS_EAGAIN - command may succeed if retried


> > And with that, VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER moves under ENODEV error code.
>
>
> No I think that belongs in in EINVAL - command is malformed.
> ENODEV is more "The wrong type of device was given to a function that
> expects a particular sort of device."
>
>
> > > +\begin{tabular}{|l|l|l|}
> > > +\hline
> > > +Status & Name & Description \\
> > > +\hline \hline
> > > +0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command
> > > error: no additional information  \\
> > > +\hline
> > > +0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or
> > > invalid \field{opcode}  \\
> > > +\hline
> > > +0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or
> > > invalid field within \field{command_specific_data}  \\
> > > +\hline
> > > +0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or
> > > invalid \field{group_type} \\
> > > +\hline
> > > +0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER    & unsupported or
> > > invalid \field{group_member_id} \\
> > > +\hline
> > > +0x05-0xFFFF   & -    & reserved for future use \\
> > > +\hline
> > > +\end{tabular}