RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 1:22 AM
> 
> This is not much of an improvement.
> 
> On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
> > Currently queue_notify_data register indicates the device internal
> > queue notification identifier. This register is meaningful only when
> > feature bit VIRTIO_F_NOTIF_CONFIG_DATA is negotiated.
> >
> > However, above register name often get confusing association with very
> > similar feature bit VIRTIO_F_NOTIFICATION_DATA.
> >
> > When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
> > notification really involves sending additional queue progress related
> > information (not queue identifier).
> >
> > Hence
> > 1. to avoid any misunderstanding and association of queue_notify_data
> > with similar name VIRTIO_F_NOTIFICATION_DATA,
> >
> > and
> > 2. to reflect that queue_notify_data is the actual device internal vq
> > identifier, rename it to queue_notify_id.
> >
> > Reflect vq identifier in the driver notification structure by renaming
> > ambiguous vqn to vq_notify_id.
> >
> > The driver notification section assumes that queue notification
> > contains vq index always. CONFIG_DATA feature bit introduction missed
> > to update the driver notification section. Hence, correct it.
> >
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> >
> > ---
> > Some side notes:
> > renaming vqn to vqnd is even more confusing because data is really the
> > queue identifier.
> 
> Clear to whom?  Why do you think so? Marvell who pushed this feature said
> they stick some kind of constant value there which matches what their
> hardware expects. Sounds like a valid way to use this.
Spec currently has described it as "identifier".
Snippet:
"for example an internal virtqueue identifier,
or an internal offset related to the virtqueue number"

> So no, not an identifier, and in any case "vq identifier" is a really general and
> useful term, I would rather not burn it up on a baroque feature that almost no
> one sets - for almost everyone else this is simply vq index.
> 
> If you really insist on renaming this away from "vqn", maybe
> vq_index_config_data will do, and we can add a comment /* Either vq index or
> vq config data, previously named vqn */
> 
Vq_index_config_data also aligns with the crazy feature bit name.
So I will use that.