public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] device-parts: editorial: Fix metadata type name
@ 2024-09-07 16:52 Parav Pandit
  2024-09-09 14:18 ` Matias Ezequiel Vara Larsen
  0 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2024-09-07 16:52 UTC (permalink / raw)
  To: virtio-comment, mst, cohuck; +Cc: shahafs, Parav Pandit

The description is for the type VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
however, it referred to a non-existent definition. Correct it.

Branch: virtio-1.4
Fixes: aa4f6f069ab3 ("admin: Add admin commands for device parts")
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 admin-cmds-device-parts.tex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/admin-cmds-device-parts.tex b/admin-cmds-device-parts.tex
index 27d8251..3a16692 100644
--- a/admin-cmds-device-parts.tex
+++ b/admin-cmds-device-parts.tex
@@ -152,10 +152,10 @@ \subsubsection{Device parts}\label{sec:Basic Facilities of a Virtio Device / Dev
 device responds with \field{hdr_list_count.count}. The
 \field{hdr_list_count.count} indicates an count of
 \field{struct virtio_dev_part_hdr} metadata entries that the device can
-provide when the \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
+provide when the \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
 in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET command.
 
-When \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
+When \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
 the device responds with \field{hdr_list}. \field{hdr_list}
 indicates the device parts metadata.
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-07 16:52 [PATCH] device-parts: editorial: Fix metadata type name Parav Pandit
@ 2024-09-09 14:18 ` Matias Ezequiel Vara Larsen
  2024-09-10 16:46   ` Parav Pandit
  0 siblings, 1 reply; 10+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2024-09-09 14:18 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, mst, cohuck, shahafs

On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> The description is for the type VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> however, it referred to a non-existent definition. Correct it.
> 
> Branch: virtio-1.4
> Fixes: aa4f6f069ab3 ("admin: Add admin commands for device parts")
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
>  admin-cmds-device-parts.tex | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/admin-cmds-device-parts.tex b/admin-cmds-device-parts.tex
> index 27d8251..3a16692 100644
> --- a/admin-cmds-device-parts.tex
> +++ b/admin-cmds-device-parts.tex
> @@ -152,10 +152,10 @@ \subsubsection{Device parts}\label{sec:Basic Facilities of a Virtio Device / Dev
>  device responds with \field{hdr_list_count.count}. The
>  \field{hdr_list_count.count} indicates an count of
>  \field{struct virtio_dev_part_hdr} metadata entries that the device can
> -provide when the \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> +provide when the \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
>  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET command.
>  
> -When \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> +When \field{type} is set to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
>  the device responds with \field{hdr_list}. \field{hdr_list}
>  indicates the device parts metadata.
>  
> -- 
> 2.34.1
> 
> 
In previous patches, I see the pattern `\field{type} field`. I wonder if
you would like to do the same here for consistency but I do not have a
strong opinion about this. Otherwise,

Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>

Thanks, Matias.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-09 14:18 ` Matias Ezequiel Vara Larsen
@ 2024-09-10 16:46   ` Parav Pandit
  2024-09-10 16:48     ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2024-09-10 16:46 UTC (permalink / raw)
  To: Matias Ezequiel Vara Larsen
  Cc: virtio-comment@lists.linux.dev, mst@redhat.com, cohuck@redhat.com,
	Shahaf Shuler



> From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> Sent: Monday, September 9, 2024 7:48 PM
> 
> On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> > The description is for the type
> > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> > however, it referred to a non-existent definition. Correct it.
> >
> > Branch: virtio-1.4
> > Fixes: aa4f6f069ab3 ("admin: Add admin commands for device parts")
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > ---
> >  admin-cmds-device-parts.tex | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/admin-cmds-device-parts.tex b/admin-cmds-device-parts.tex
> > index 27d8251..3a16692 100644
> > --- a/admin-cmds-device-parts.tex
> > +++ b/admin-cmds-device-parts.tex
> > @@ -152,10 +152,10 @@ \subsubsection{Device parts}\label{sec:Basic
> > Facilities of a Virtio Device / Dev  device responds with
> > \field{hdr_list_count.count}. The  \field{hdr_list_count.count}
> > indicates an count of  \field{struct virtio_dev_part_hdr} metadata
> > entries that the device can -provide when the \field{type} is set to
> > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> > +provide when the \field{type} is set to
> > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
> >  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET
> command.
> >
> > -When \field{type} is set to
> > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> > +When \field{type} is set to
> > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
> >  the device responds with \field{hdr_list}. \field{hdr_list}
> > indicates the device parts metadata.
> >
> > --
> > 2.34.1
> >
> >
> In previous patches, I see the pattern `\field{type} field`. I wonder if you
> would like to do the same here for consistency but I do not have a strong
> opinion about this. Otherwise,
> 
True, maybe I can remove 'field' word from some places in general language improvement separate patch.
Will look.

> Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> 
> Thanks, Matias.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-10 16:46   ` Parav Pandit
@ 2024-09-10 16:48     ` Michael S. Tsirkin
  2024-09-10 16:51       ` Parav Pandit
  0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-10 16:48 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler

On Tue, Sep 10, 2024 at 04:46:34PM +0000, Parav Pandit wrote:
> 
> 
> > From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > Sent: Monday, September 9, 2024 7:48 PM
> > 
> > On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> > > The description is for the type
> > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> > > however, it referred to a non-existent definition. Correct it.
> > >
> > > Branch: virtio-1.4
> > > Fixes: aa4f6f069ab3 ("admin: Add admin commands for device parts")
> > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > ---
> > >  admin-cmds-device-parts.tex | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/admin-cmds-device-parts.tex b/admin-cmds-device-parts.tex
> > > index 27d8251..3a16692 100644
> > > --- a/admin-cmds-device-parts.tex
> > > +++ b/admin-cmds-device-parts.tex
> > > @@ -152,10 +152,10 @@ \subsubsection{Device parts}\label{sec:Basic
> > > Facilities of a Virtio Device / Dev  device responds with
> > > \field{hdr_list_count.count}. The  \field{hdr_list_count.count}
> > > indicates an count of  \field{struct virtio_dev_part_hdr} metadata
> > > entries that the device can -provide when the \field{type} is set to
> > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> > > +provide when the \field{type} is set to
> > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
> > >  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET
> > command.
> > >
> > > -When \field{type} is set to
> > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> > > +When \field{type} is set to
> > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
> > >  the device responds with \field{hdr_list}. \field{hdr_list}
> > > indicates the device parts metadata.
> > >
> > > --
> > > 2.34.1
> > >
> > >
> > In previous patches, I see the pattern `\field{type} field`. I wonder if you
> > would like to do the same here for consistency but I do not have a strong
> > opinion about this. Otherwise,
> > 
> True, maybe I can remove 'field' word from some places in general language improvement separate patch.
> Will look.


It just typesets it as italic ATM. Worth doing to avoid confusion, as some
fields are plain english words.


> > Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > 
> > Thanks, Matias.
> 
> Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-10 16:48     ` Michael S. Tsirkin
@ 2024-09-10 16:51       ` Parav Pandit
  2024-09-10 18:34         ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2024-09-10 16:51 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler



> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, September 10, 2024 10:18 PM
> 
> On Tue, Sep 10, 2024 at 04:46:34PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > Sent: Monday, September 9, 2024 7:48 PM
> > >
> > > On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> > > > The description is for the type
> > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> > > > however, it referred to a non-existent definition. Correct it.
> > > >
> > > > Branch: virtio-1.4
> > > > Fixes: aa4f6f069ab3 ("admin: Add admin commands for device parts")
> > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > ---
> > > >  admin-cmds-device-parts.tex | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/admin-cmds-device-parts.tex
> > > > b/admin-cmds-device-parts.tex index 27d8251..3a16692 100644
> > > > --- a/admin-cmds-device-parts.tex
> > > > +++ b/admin-cmds-device-parts.tex
> > > > @@ -152,10 +152,10 @@ \subsubsection{Device parts}\label{sec:Basic
> > > > Facilities of a Virtio Device / Dev  device responds with
> > > > \field{hdr_list_count.count}. The  \field{hdr_list_count.count}
> > > > indicates an count of  \field{struct virtio_dev_part_hdr} metadata
> > > > entries that the device can -provide when the \field{type} is set
> > > > to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> > > > +provide when the \field{type} is set to
> > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
> > > >  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET
> > > command.
> > > >
> > > > -When \field{type} is set to
> > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> > > > +When \field{type} is set to
> > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
> > > >  the device responds with \field{hdr_list}. \field{hdr_list}
> > > > indicates the device parts metadata.
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > > >
> > > In previous patches, I see the pattern `\field{type} field`. I
> > > wonder if you would like to do the same here for consistency but I
> > > do not have a strong opinion about this. Otherwise,
> > >
> > True, maybe I can remove 'field' word from some places in general
> language improvement separate patch.
> > Will look.
> 
> 
> It just typesets it as italic ATM. Worth doing to avoid confusion, as some
> fields are plain english words.
> 
Do you mean to add word 'field' wherever missing as Matias suggested?

If so, no strong opinion to me, I will run a separate patch for such improvements without fixes tag.

> 
> > > Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > >
> > > Thanks, Matias.
> >
> > Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-10 16:51       ` Parav Pandit
@ 2024-09-10 18:34         ` Michael S. Tsirkin
  2024-09-10 18:54           ` Parav Pandit
  0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-10 18:34 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler

On Tue, Sep 10, 2024 at 04:51:03PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, September 10, 2024 10:18 PM
> > 
> > On Tue, Sep 10, 2024 at 04:46:34PM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > > Sent: Monday, September 9, 2024 7:48 PM
> > > >
> > > > On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> > > > > The description is for the type
> > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> > > > > however, it referred to a non-existent definition. Correct it.
> > > > >
> > > > > Branch: virtio-1.4
> > > > > Fixes: aa4f6f069ab3 ("admin: Add admin commands for device parts")
> > > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > > ---
> > > > >  admin-cmds-device-parts.tex | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/admin-cmds-device-parts.tex
> > > > > b/admin-cmds-device-parts.tex index 27d8251..3a16692 100644
> > > > > --- a/admin-cmds-device-parts.tex
> > > > > +++ b/admin-cmds-device-parts.tex
> > > > > @@ -152,10 +152,10 @@ \subsubsection{Device parts}\label{sec:Basic
> > > > > Facilities of a Virtio Device / Dev  device responds with
> > > > > \field{hdr_list_count.count}. The  \field{hdr_list_count.count}
> > > > > indicates an count of  \field{struct virtio_dev_part_hdr} metadata
> > > > > entries that the device can -provide when the \field{type} is set
> > > > > to VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> > > > > +provide when the \field{type} is set to
> > > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
> > > > >  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET
> > > > command.
> > > > >
> > > > > -When \field{type} is set to
> > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> > > > > +When \field{type} is set to
> > > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
> > > > >  the device responds with \field{hdr_list}. \field{hdr_list}
> > > > > indicates the device parts metadata.
> > > > >
> > > > > --
> > > > > 2.34.1
> > > > >
> > > > >
> > > > In previous patches, I see the pattern `\field{type} field`. I
> > > > wonder if you would like to do the same here for consistency but I
> > > > do not have a strong opinion about this. Otherwise,
> > > >
> > > True, maybe I can remove 'field' word from some places in general
> > language improvement separate patch.
> > > Will look.
> > 
> > 
> > It just typesets it as italic ATM. Worth doing to avoid confusion, as some
> > fields are plain english words.
> > 
> Do you mean to add word 'field' wherever missing as Matias suggested?


If it's a field in a struct, it should be in italics, yes. I didn't get which field
Matias refers to, exactly.

> If so, no strong opinion to me, I will run a separate patch for such improvements without fixes tag.
> 
> > 
> > > > Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > >
> > > > Thanks, Matias.
> > >
> > > Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-10 18:34         ` Michael S. Tsirkin
@ 2024-09-10 18:54           ` Parav Pandit
  2024-09-10 20:21             ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2024-09-10 18:54 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler



> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, September 11, 2024 12:05 AM
> 
> On Tue, Sep 10, 2024 at 04:51:03PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Tuesday, September 10, 2024 10:18 PM
> > >
> > > On Tue, Sep 10, 2024 at 04:46:34PM +0000, Parav Pandit wrote:
> > > >
> > > >
> > > > > From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > > > Sent: Monday, September 9, 2024 7:48 PM
> > > > >
> > > > > On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> > > > > > The description is for the type
> > > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> > > > > > however, it referred to a non-existent definition. Correct it.
> > > > > >
> > > > > > Branch: virtio-1.4
> > > > > > Fixes: aa4f6f069ab3 ("admin: Add admin commands for device
> > > > > > parts")
> > > > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > > > ---
> > > > > >  admin-cmds-device-parts.tex | 4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/admin-cmds-device-parts.tex
> > > > > > b/admin-cmds-device-parts.tex index 27d8251..3a16692 100644
> > > > > > --- a/admin-cmds-device-parts.tex
> > > > > > +++ b/admin-cmds-device-parts.tex
> > > > > > @@ -152,10 +152,10 @@ \subsubsection{Device
> > > > > > parts}\label{sec:Basic Facilities of a Virtio Device / Dev
> > > > > > device responds with \field{hdr_list_count.count}. The
> > > > > > \field{hdr_list_count.count} indicates an count of
> > > > > > \field{struct virtio_dev_part_hdr} metadata entries that the
> > > > > > device can -provide when the \field{type} is set to
> > > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> > > > > > +provide when the \field{type} is set to
> > > > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
> > > > > >  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET
> > > > > command.
> > > > > >
> > > > > > -When \field{type} is set to
> > > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> > > > > > +When \field{type} is set to
> > > > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
> > > > > >  the device responds with \field{hdr_list}. \field{hdr_list}
> > > > > > indicates the device parts metadata.
> > > > > >
> > > > > > --
> > > > > > 2.34.1
> > > > > >
> > > > > >
> > > > > In previous patches, I see the pattern `\field{type} field`. I
> > > > > wonder if you would like to do the same here for consistency but
> > > > > I do not have a strong opinion about this. Otherwise,
> > > > >
> > > > True, maybe I can remove 'field' word from some places in general
> > > language improvement separate patch.
> > > > Will look.
> > >
> > >
> > > It just typesets it as italic ATM. Worth doing to avoid confusion,
> > > as some fields are plain english words.
> > >
> > Do you mean to add word 'field' wherever missing as Matias suggested?
> 
> 
> If it's a field in a struct, it should be in italics, yes. 
It is already in italics.

The question was : should be written as,

Option_A: \field{type} field indicates so-n-so...
or
Option_B: \field{type} indicates so-n-soo..

> I didn't get which field
> Matias refers to, exactly.
> 
> > If so, no strong opinion to me, I will run a separate patch for such
> improvements without fixes tag.
> >
> > >
> > > > > Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > > >
> > > > > Thanks, Matias.
> > > >
> > > > Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-10 18:54           ` Parav Pandit
@ 2024-09-10 20:21             ` Michael S. Tsirkin
  2024-09-11  3:19               ` Parav Pandit
  0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-10 20:21 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler

On Tue, Sep 10, 2024 at 06:54:38PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, September 11, 2024 12:05 AM
> > 
> > On Tue, Sep 10, 2024 at 04:51:03PM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Tuesday, September 10, 2024 10:18 PM
> > > >
> > > > On Tue, Sep 10, 2024 at 04:46:34PM +0000, Parav Pandit wrote:
> > > > >
> > > > >
> > > > > > From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > > > > Sent: Monday, September 9, 2024 7:48 PM
> > > > > >
> > > > > > On Sat, Sep 07, 2024 at 07:52:18PM +0300, Parav Pandit wrote:
> > > > > > > The description is for the type
> > > > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST;
> > > > > > > however, it referred to a non-existent definition. Correct it.
> > > > > > >
> > > > > > > Branch: virtio-1.4
> > > > > > > Fixes: aa4f6f069ab3 ("admin: Add admin commands for device
> > > > > > > parts")
> > > > > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > > > > ---
> > > > > > >  admin-cmds-device-parts.tex | 4 ++--
> > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/admin-cmds-device-parts.tex
> > > > > > > b/admin-cmds-device-parts.tex index 27d8251..3a16692 100644
> > > > > > > --- a/admin-cmds-device-parts.tex
> > > > > > > +++ b/admin-cmds-device-parts.tex
> > > > > > > @@ -152,10 +152,10 @@ \subsubsection{Device
> > > > > > > parts}\label{sec:Basic Facilities of a Virtio Device / Dev
> > > > > > > device responds with \field{hdr_list_count.count}. The
> > > > > > > \field{hdr_list_count.count} indicates an count of
> > > > > > > \field{struct virtio_dev_part_hdr} metadata entries that the
> > > > > > > device can -provide when the \field{type} is set to
> > > > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR
> > > > > > > +provide when the \field{type} is set to
> > > > > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST
> > > > > > >  in a subsequent VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_GET
> > > > > > command.
> > > > > > >
> > > > > > > -When \field{type} is set to
> > > > > > > VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_HDR,
> > > > > > > +When \field{type} is set to
> > > > > > > +VIRTIO_ADMIN_CMD_DEV_PARTS_METADATA_TYPE_LIST,
> > > > > > >  the device responds with \field{hdr_list}. \field{hdr_list}
> > > > > > > indicates the device parts metadata.
> > > > > > >
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> > > > > > >
> > > > > > In previous patches, I see the pattern `\field{type} field`. I
> > > > > > wonder if you would like to do the same here for consistency but
> > > > > > I do not have a strong opinion about this. Otherwise,
> > > > > >
> > > > > True, maybe I can remove 'field' word from some places in general
> > > > language improvement separate patch.
> > > > > Will look.
> > > >
> > > >
> > > > It just typesets it as italic ATM. Worth doing to avoid confusion,
> > > > as some fields are plain english words.
> > > >
> > > Do you mean to add word 'field' wherever missing as Matias suggested?
> > 
> > 
> > If it's a field in a struct, it should be in italics, yes. 
> It is already in italics.
> 
> The question was : should be written as,
> 
> Option_A: \field{type} field indicates so-n-so...
> or
> Option_B: \field{type} indicates so-n-soo..

Ah.

I don't really feel adding "field" adds much, because of
italics and because one can see it's a field from the name.
So, brevity wins.


> > I didn't get which field
> > Matias refers to, exactly.
> > 
> > > If so, no strong opinion to me, I will run a separate patch for such
> > improvements without fixes tag.
> > >
> > > >
> > > > > > Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > > > > >
> > > > > > Thanks, Matias.
> > > > >
> > > > > Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-10 20:21             ` Michael S. Tsirkin
@ 2024-09-11  3:19               ` Parav Pandit
  2024-09-11 10:01                 ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2024-09-11  3:19 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler



> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, September 11, 2024 1:51 AM

[..]
> > It is already in italics.
> >
> > The question was : should be written as,
> >
> > Option_A: \field{type} field indicates so-n-so...
> > or
> > Option_B: \field{type} indicates so-n-soo..
> 
> Ah.
> 
> I don't really feel adding "field" adds much, because of italics and because
> one can see it's a field from the name.
> So, brevity wins.

Ok. Got it.
The current patch is discussion does not have word 'field'.
So current patch is good as-is.
Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] device-parts: editorial: Fix metadata type name
  2024-09-11  3:19               ` Parav Pandit
@ 2024-09-11 10:01                 ` Michael S. Tsirkin
  0 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-11 10:01 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Matias Ezequiel Vara Larsen, virtio-comment@lists.linux.dev,
	cohuck@redhat.com, Shahaf Shuler

On Wed, Sep 11, 2024 at 03:19:45AM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, September 11, 2024 1:51 AM
> 
> [..]
> > > It is already in italics.
> > >
> > > The question was : should be written as,
> > >
> > > Option_A: \field{type} field indicates so-n-so...
> > > or
> > > Option_B: \field{type} indicates so-n-soo..
> > 
> > Ah.
> > 
> > I don't really feel adding "field" adds much, because of italics and because
> > one can see it's a field from the name.
> > So, brevity wins.
> 
> Ok. Got it.
> The current patch is discussion does not have word 'field'.
> So current patch is good as-is.
> Thanks.

Agreed.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-09-11 10:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-07 16:52 [PATCH] device-parts: editorial: Fix metadata type name Parav Pandit
2024-09-09 14:18 ` Matias Ezequiel Vara Larsen
2024-09-10 16:46   ` Parav Pandit
2024-09-10 16:48     ` Michael S. Tsirkin
2024-09-10 16:51       ` Parav Pandit
2024-09-10 18:34         ` Michael S. Tsirkin
2024-09-10 18:54           ` Parav Pandit
2024-09-10 20:21             ` Michael S. Tsirkin
2024-09-11  3:19               ` Parav Pandit
2024-09-11 10:01                 ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox