[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v5 3/7] Introduce new destination type for admin commands
On Mon, May 16, 2022 at 09:21:11PM +0000, Parav Pandit wrote: > > > From: Michael S. Tsirkin <mst@redhat.com> > > Sent: Sunday, May 15, 2022 11:09 AM > > > > > --- > > > admin.tex | 18 ++++++++++++++---- > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > > diff --git a/admin.tex b/admin.tex > > > index 6725daa..f816c3b 100644 > > > --- a/admin.tex > > > +++ b/admin.tex > > > @@ -11,11 +11,13 @@ \section{Administration command > > set}\label{sec:Basic Facilities of a Virtio Devi > > > le16 command; > > > /* > > > * 0 - self > > > - * 1 - 65535 are reserved > > > + * 1 - other virtio device (identified by vdev_id) in the same device > > group > > > + * 2 - 65535 are reserved > > > */ > > > le16 dst_type; > > > + le64 vdev_id; > > > > Alignment problems. Proposal: > > > > vdev_id > > dst_type > > command > > I remember that this has come up internal review as well. > Though it is certainly good to naturally align, I don't think we have alignment problem spec wise based on below snippet of the spec [1]. > It was kind of counter intuitive to see vdev_id before seeing what the actual command is. > That way current layout made more sense. vdev_id and dst_type tell you where to send the command. They do not depend on the command. In fact vdev_id -> dst_id might make sense. > > There was some internal version or external, do not recall, where vdev_id was optional or was union. > > But I think if we always going have vdev_id, it can be naturally aligned like your above proposal. > > [1] "Structure Specifications > Many device and driver in-memory structure layouts are documented using the C struct syntax. All structures > are assumed to be without additional padding. To stress this, cases where common C compilers are known > to insert extra padding within structures are tagged using the GNU C __attribute__((packed)) syntax." These are pathological cases due to legacy. Better avoided. -- MST
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]