* [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