[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] [PATCH v13 03/11] content: Rename confusing queue_notify_data and vqn names
On Tue, Apr 18 2023, Halil Pasic <pasic@linux.ibm.com> wrote: > On Mon, 17 Apr 2023 20:53:32 +0000 > Parav Pandit <parav@nvidia.com> wrote: > >> > From: Halil Pasic <pasic@linux.ibm.com> >> > Sent: Sunday, April 16, 2023 11:42 PM > [..] >> > >> > > @@ -1053,9 +1059,9 @@ \subsubsection{Available Buffer >> > > Notifications}\label{sec:Virtio Transport Option If >> > VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated: >> > > \begin{itemize} >> > > \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the >> > > driver MUST use the -\field{queue_notify_data} value instead of the virtqueue >> > index. >> > > +\field{queue_notify_id} value instead of the virtqueue index. >> > > \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver >> > > MUST set the -\field{vqn} field to the \field{queue_notify_data} value. >> > > +\field{vq_notify_id} field to the \field{queue_notify_id} value. >> > >> > Seems you did not catch these with the s/_id/_config_data/. By doing the >> > replace one would get queue_notify_config_data, vq_notify_config_data and >> > queue_notify_config_data respectively. >> > >> > But that still does not seem right, and the right answer depends on to what do >> > we rename \field{vqn}. >> > >> vqn -> vq_notif_config_data in the notification structure. > > I think I got what you mean. I intend to double check it > in v14. > >> >> virtio_pci_common_cfg.queue_notify_data -> queue_notify_config_data. >> > > I have no strong opinion about this one. > >> Better to rename >> >> VIRTIO_F_NOTIF_CONFIG_DATA to VIRTIO_F_NOTIFY_CONFIG_DATA. >> Adding "Y" is not going to make it that long. Field names will have some consistency. >> WDYT? > > No strong opinion on this either. I agree with Michael that "NOTIF" > likely stands for "notification", but I also see your point: having > "notify" in the field names and "NOTIF" in the feature name isn't great > either. > > IMHO VIRTIO_F_NOTIFY_CONFIG_DATA should remain aligned with > VIRTIO_F_NOTIFICATION_DATA in one way or another. Thus way out would be > to do something like: > virtio_pci_common_cfg.queue_notify_data -> queue_notif_config_data > > I think as editors Connie and Michael should have the most say in this > though. Using "notif" throughout would probably be most consistent.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]