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




Reply via email to