Hi, On Monday, June 24, 2024 5:06:42 PM GMT+5:30 Eugenio Perez Martin wrote: > [...] > > > > /* Shadow virtqueue to relay notifications */ > > > > typedef struct VhostShadowVirtqueue { > > > > > > > > + /* Virtio queue shadowing */ > > > > + VirtQueue *vq; > > > > + > > > > + /* Virtio device */ > > > > + VirtIODevice *vdev; > > > > + > > > > + /* SVQ vring descriptors state */ > > > > + SVQDescState *desc_state; > > > > + > > > > + /* > > > > + * Backup next field for each descriptor so we can recover > > > > securely, > > > > not + * needing to trust the device access. > > > > + */ > > > > + uint16_t *desc_next; > > > > + > > > > + /* Next free descriptor */ > > > > + uint16_t free_head; > > > > + > > > > + /* Size of SVQ vring free descriptors */ > > > > + uint16_t num_free; > > > > + > > > > > > Why the reorder of all of the previous members? > > > > I was thinking of placing all the members that are common to packed and > > split vring above the union while leaving the remaining members below the > > union. I did this based on our discussion here [1]. I don't think this > > reordering serves any purpose implementation-wise. Please let me know if > > I should revert this change. > > I'm not against the change, but removing superfluous changes helps the > reviewing. If you send the reordering, please use a separate patch so > it's easier to review.
Understood, I'll keep this in mind when sending patches. Thanks, Sahil