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

[Stefan Hajnoczi <stefanha@redhat.com>]

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.

Please define a unique constant for each error condition that can occur
instead of sharing catch-all errno constants between multiple error
conditions. If a driver wants to squash them together into an errno,
that's fine, but I think doing this at the hardware interface level is
just propagating the mistakes of errnos.

Only status_qualifier is needed and the vague status field can be
dropped. It's not clear to me why adding EAGAIN, ENOMEM, EBUSY, and
ENODEV is useful. They have no meaning to the driver, only the
status_qualifier really indicates what is going on.

I'm sure you guys have discussed this previously, but please provide
rationale in the spec because it looks weird to someone with fresh eyes.

Stefan

Attachment: signature.asc
Description: PGP signature