virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/1] virtio-blk: Add description for blk_size field
@ 2024-10-09 16:05 Max Gurtovoy
  2024-10-10 10:24 ` Matias Ezequiel Vara Larsen
  0 siblings, 1 reply; 4+ messages in thread
From: Max Gurtovoy @ 2024-10-09 16:05 UTC (permalink / raw)
  To: dverkamp, mst, virtualization, stefanha, virtio-dev
  Cc: oren, parav, nitzanc, benwalker, Max Gurtovoy

This field is only valid when the VIRTIO_BLK_F_BLK_SIZE feature bit is
offered by the device.

The blk_size field actually represents the logical block size of the
device. It is always a power of two and typically ranges from 512 bytes
to larger values such as 4 KB.

Add description for this field to provide clarity on its constraints.

Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 device-types/blk/description.tex | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
index 2712ada..caa5d13 100644
--- a/device-types/blk/description.tex
+++ b/device-types/blk/description.tex
@@ -135,6 +135,9 @@ \subsection{Device configuration layout}\label{sec:Device Types / Block Device /
 present. The availability of the others all depend on various feature
 bits as indicated above.
 
+The field \field{blk_size} exists only if VIRTIO_BLK_F_BLK_SIZE is offered by the device.
+This field reports the block size of the device, expressed in bytes.
+
 The field \field{num_queues} only exists if VIRTIO_BLK_F_MQ is set. This field specifies
 the number of queues.
 
@@ -282,6 +285,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
 
 \drivernormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
 
+Drivers SHOULD negotiate VIRTIO_BLK_F_BLK_SIZE if the feature is offered by the
+device. When negotiated, drivers SHOULD interpret the \field{blk_size} as the
+logical block size.
+
+If the VIRTIO_BLK_F_BLK_SIZE feature is not offered by the device, then drivers
+MAY assume that the logical block size is 512 bytes.
+
 Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of
 sending VIRTIO_BLK_T_FLUSH commands.
 
@@ -319,6 +329,10 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
 
 \devicenormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
 
+Devices SHOULD always offer VIRTIO_BLK_F_BLK_SIZE feature. When this feature is
+offered, devices MUST initialize \field{blk_size} to a power of two greater
+than or equal to 512.
+
 Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it
 if they offer VIRTIO_BLK_F_CONFIG_WCE.
 
@@ -879,6 +893,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
 The length of \field{data} MUST be a multiple of 512 bytes for VIRTIO_BLK_T_IN
 and VIRTIO_BLK_T_OUT requests.
 
+The length of \field{data} SHOULD be a multiple of \field{blk_size} for VIRTIO_BLK_T_IN
+and VIRTIO_BLK_T_OUT requests, when the VIRTIO_BLK_F_BLK_SIZE feature has been
+offered by the device.
+
+The value of \field{sector} (multiplied by 512) SHOULD be a multiple of
+\field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests, when the
+VIRTIO_BLK_F_BLK_SIZE feature has offered by the device.
+
 The length of \field{data} MUST be a multiple of the size of struct
 virtio_blk_discard_write_zeroes for VIRTIO_BLK_T_DISCARD,
 VIRTIO_BLK_T_SECURE_ERASE and VIRTIO_BLK_T_WRITE_ZEROES requests.
@@ -966,6 +988,18 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
 for a write request if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT
 write any data.
 
+A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
+VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the requested
+\field{sector} (multiplied by 512) is not an integer multiple of the device's
+\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
+the device.
+
+A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
+VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the length of the
+requested \field{data} is not an integer multiple of the device's
+\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
+the device.
+
 The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
 discard, secure erase and write zeroes commands if any unknown flag is set.
 Furthermore, the device MUST set the \field{status} byte to
-- 
2.18.1


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

* Re: [PATCH v3 1/1] virtio-blk: Add description for blk_size field
  2024-10-09 16:05 [PATCH v3 1/1] virtio-blk: Add description for blk_size field Max Gurtovoy
@ 2024-10-10 10:24 ` Matias Ezequiel Vara Larsen
  2024-10-10 14:04   ` Max Gurtovoy
  0 siblings, 1 reply; 4+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2024-10-10 10:24 UTC (permalink / raw)
  To: Max Gurtovoy
  Cc: dverkamp, mst, virtualization, stefanha, virtio-dev, oren, parav,
	nitzanc, benwalker

Hello, 

I wrote some minor comments below.

On Wed, Oct 09, 2024 at 07:05:12PM +0300, Max Gurtovoy wrote:
> This field is only valid when the VIRTIO_BLK_F_BLK_SIZE feature bit is
> offered by the device.
> 
> The blk_size field actually represents the logical block size of the
> device. It is always a power of two and typically ranges from 512 bytes
> to larger values such as 4 KB.
> 
> Add description for this field to provide clarity on its constraints.
> 
> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
>  device-types/blk/description.tex | 34 ++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
> index 2712ada..caa5d13 100644
> --- a/device-types/blk/description.tex
> +++ b/device-types/blk/description.tex
> @@ -135,6 +135,9 @@ \subsection{Device configuration layout}\label{sec:Device Types / Block Device /
>  present. The availability of the others all depend on various feature
>  bits as indicated above.
>  
> +The field \field{blk_size} exists only if VIRTIO_BLK_F_BLK_SIZE is offered by the device.
> +This field reports the block size of the device, expressed in bytes.
> +
>  The field \field{num_queues} only exists if VIRTIO_BLK_F_MQ is set. This field specifies
>  the number of queues.
>  
> @@ -282,6 +285,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
>  
>  \drivernormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
>  
> +Drivers SHOULD negotiate VIRTIO_BLK_F_BLK_SIZE if the feature is offered by the
> +device. When negotiated, drivers SHOULD interpret the \field{blk_size} as the
> +logical block size.
> +
> +If the VIRTIO_BLK_F_BLK_SIZE feature is not offered by the device, then drivers
> +MAY assume that the logical block size is 512 bytes.
> +
>  Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of
>  sending VIRTIO_BLK_T_FLUSH commands.
>  
> @@ -319,6 +329,10 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
>  
>  \devicenormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
>  
> +Devices SHOULD always offer VIRTIO_BLK_F_BLK_SIZE feature. When this feature is
> +offered, devices MUST initialize \field{blk_size} to a power of two greater
> +than or equal to 512.
> +
>  Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it
>  if they offer VIRTIO_BLK_F_CONFIG_WCE.
>  
> @@ -879,6 +893,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>  The length of \field{data} MUST be a multiple of 512 bytes for VIRTIO_BLK_T_IN
>  and VIRTIO_BLK_T_OUT requests.
>  
> +The length of \field{data} SHOULD be a multiple of \field{blk_size} for VIRTIO_BLK_T_IN
> +and VIRTIO_BLK_T_OUT requests, when the VIRTIO_BLK_F_BLK_SIZE feature has been
> +offered by the device.
> +

I would rewrite it as:

`When the VIRTIO_BLK_F_BLK_SIZE feature has been offered by the device,
the length of \field{data} SHOULD be a multiple of \field{blk_size} for
VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests`

Just because people tend to read from more important to less important
within a sentence but I do not have an strong opinion about it.
Apologies if I may sound bit picky ;). 

> +The value of \field{sector} (multiplied by 512) SHOULD be a multiple of
> +\field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests, when the
> +VIRTIO_BLK_F_BLK_SIZE feature has offered by the device.
> +

For the same reason, I would rewrite it as:

`When the VIRTIO_BLK_F_BLK_SIZE feature has offered by the device, the
value of \field{sector} (multiplied by 512) SHOULD be a multiple of
\field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests`.

I think I would follow this pattern in the text below too.

>  The length of \field{data} MUST be a multiple of the size of struct
>  virtio_blk_discard_write_zeroes for VIRTIO_BLK_T_DISCARD,
>  VIRTIO_BLK_T_SECURE_ERASE and VIRTIO_BLK_T_WRITE_ZEROES requests.
> @@ -966,6 +988,18 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>  for a write request if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT
>  write any data.
>  
> +A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
> +VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the requested
> +\field{sector} (multiplied by 512) is not an integer multiple of the device's
> +\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
> +the device.
> +
> +A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
> +VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the length of the
> +requested \field{data} is not an integer multiple of the device's
> +\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
> +the device.
> +
>  The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
>  discard, secure erase and write zeroes commands if any unknown flag is set.
>  Furthermore, the device MUST set the \field{status} byte to
> -- 
> 2.18.1
> 
> 


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

* Re: [PATCH v3 1/1] virtio-blk: Add description for blk_size field
  2024-10-10 10:24 ` Matias Ezequiel Vara Larsen
@ 2024-10-10 14:04   ` Max Gurtovoy
  2024-10-10 14:15     ` Michael S. Tsirkin
  0 siblings, 1 reply; 4+ messages in thread
From: Max Gurtovoy @ 2024-10-10 14:04 UTC (permalink / raw)
  To: Matias Ezequiel Vara Larsen
  Cc: dverkamp, mst, virtualization, stefanha, virtio-dev, oren, parav,
	nitzanc, benwalker


On 10/10/2024 13:24, Matias Ezequiel Vara Larsen wrote:
> Hello,
>
> I wrote some minor comments below.
>
> On Wed, Oct 09, 2024 at 07:05:12PM +0300, Max Gurtovoy wrote:
>> This field is only valid when the VIRTIO_BLK_F_BLK_SIZE feature bit is
>> offered by the device.
>>
>> The blk_size field actually represents the logical block size of the
>> device. It is always a power of two and typically ranges from 512 bytes
>> to larger values such as 4 KB.
>>
>> Add description for this field to provide clarity on its constraints.
>>
>> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> ---
>>   device-types/blk/description.tex | 34 ++++++++++++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
>> index 2712ada..caa5d13 100644
>> --- a/device-types/blk/description.tex
>> +++ b/device-types/blk/description.tex
>> @@ -135,6 +135,9 @@ \subsection{Device configuration layout}\label{sec:Device Types / Block Device /
>>   present. The availability of the others all depend on various feature
>>   bits as indicated above.
>>   
>> +The field \field{blk_size} exists only if VIRTIO_BLK_F_BLK_SIZE is offered by the device.
>> +This field reports the block size of the device, expressed in bytes.
>> +
>>   The field \field{num_queues} only exists if VIRTIO_BLK_F_MQ is set. This field specifies
>>   the number of queues.
>>   
>> @@ -282,6 +285,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
>>   
>>   \drivernormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
>>   
>> +Drivers SHOULD negotiate VIRTIO_BLK_F_BLK_SIZE if the feature is offered by the
>> +device. When negotiated, drivers SHOULD interpret the \field{blk_size} as the
>> +logical block size.
>> +
>> +If the VIRTIO_BLK_F_BLK_SIZE feature is not offered by the device, then drivers
>> +MAY assume that the logical block size is 512 bytes.
>> +
>>   Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of
>>   sending VIRTIO_BLK_T_FLUSH commands.
>>   
>> @@ -319,6 +329,10 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
>>   
>>   \devicenormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
>>   
>> +Devices SHOULD always offer VIRTIO_BLK_F_BLK_SIZE feature. When this feature is
>> +offered, devices MUST initialize \field{blk_size} to a power of two greater
>> +than or equal to 512.
>> +
>>   Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it
>>   if they offer VIRTIO_BLK_F_CONFIG_WCE.
>>   
>> @@ -879,6 +893,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>>   The length of \field{data} MUST be a multiple of 512 bytes for VIRTIO_BLK_T_IN
>>   and VIRTIO_BLK_T_OUT requests.
>>   
>> +The length of \field{data} SHOULD be a multiple of \field{blk_size} for VIRTIO_BLK_T_IN
>> +and VIRTIO_BLK_T_OUT requests, when the VIRTIO_BLK_F_BLK_SIZE feature has been
>> +offered by the device.
>> +
> I would rewrite it as:
>
> `When the VIRTIO_BLK_F_BLK_SIZE feature has been offered by the device,
> the length of \field{data} SHOULD be a multiple of \field{blk_size} for
> VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests`
>
> Just because people tend to read from more important to less important
> within a sentence but I do not have an strong opinion about it.
> Apologies if I may sound bit picky ;).

I don't have a strong opinion on it as well.

I'm open to making changes, but I'd appreciate getting input from 
MST/Stefan first.

This way, we can ensure we're aligning with their perspective and avoid 
potential back-and-forth revisions.

>
>> +The value of \field{sector} (multiplied by 512) SHOULD be a multiple of
>> +\field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests, when the
>> +VIRTIO_BLK_F_BLK_SIZE feature has offered by the device.
>> +
> For the same reason, I would rewrite it as:
>
> `When the VIRTIO_BLK_F_BLK_SIZE feature has offered by the device, the
> value of \field{sector} (multiplied by 512) SHOULD be a multiple of
> \field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests`.
>
> I think I would follow this pattern in the text below too.
>
>>   The length of \field{data} MUST be a multiple of the size of struct
>>   virtio_blk_discard_write_zeroes for VIRTIO_BLK_T_DISCARD,
>>   VIRTIO_BLK_T_SECURE_ERASE and VIRTIO_BLK_T_WRITE_ZEROES requests.
>> @@ -966,6 +988,18 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>>   for a write request if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT
>>   write any data.
>>   
>> +A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
>> +VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the requested
>> +\field{sector} (multiplied by 512) is not an integer multiple of the device's
>> +\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
>> +the device.
>> +
>> +A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
>> +VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the length of the
>> +requested \field{data} is not an integer multiple of the device's
>> +\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
>> +the device.
>> +
>>   The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
>>   discard, secure erase and write zeroes commands if any unknown flag is set.
>>   Furthermore, the device MUST set the \field{status} byte to
>> -- 
>> 2.18.1
>>
>>

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

* Re: [PATCH v3 1/1] virtio-blk: Add description for blk_size field
  2024-10-10 14:04   ` Max Gurtovoy
@ 2024-10-10 14:15     ` Michael S. Tsirkin
  0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2024-10-10 14:15 UTC (permalink / raw)
  To: Max Gurtovoy
  Cc: Matias Ezequiel Vara Larsen, dverkamp, virtualization, stefanha,
	virtio-dev, oren, parav, nitzanc, benwalker

On Thu, Oct 10, 2024 at 05:04:55PM +0300, Max Gurtovoy wrote:
> 
> On 10/10/2024 13:24, Matias Ezequiel Vara Larsen wrote:
> > Hello,
> > 
> > I wrote some minor comments below.
> > 
> > On Wed, Oct 09, 2024 at 07:05:12PM +0300, Max Gurtovoy wrote:
> > > This field is only valid when the VIRTIO_BLK_F_BLK_SIZE feature bit is
> > > offered by the device.
> > > 
> > > The blk_size field actually represents the logical block size of the
> > > device. It is always a power of two and typically ranges from 512 bytes
> > > to larger values such as 4 KB.
> > > 
> > > Add description for this field to provide clarity on its constraints.
> > > 
> > > Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> > > ---
> > >   device-types/blk/description.tex | 34 ++++++++++++++++++++++++++++++++
> > >   1 file changed, 34 insertions(+)
> > > 
> > > diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
> > > index 2712ada..caa5d13 100644
> > > --- a/device-types/blk/description.tex
> > > +++ b/device-types/blk/description.tex
> > > @@ -135,6 +135,9 @@ \subsection{Device configuration layout}\label{sec:Device Types / Block Device /
> > >   present. The availability of the others all depend on various feature
> > >   bits as indicated above.
> > > +The field \field{blk_size} exists only if VIRTIO_BLK_F_BLK_SIZE is offered by the device.
> > > +This field reports the block size of the device, expressed in bytes.
> > > +
> > >   The field \field{num_queues} only exists if VIRTIO_BLK_F_MQ is set. This field specifies
> > >   the number of queues.
> > > @@ -282,6 +285,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
> > >   \drivernormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
> > > +Drivers SHOULD negotiate VIRTIO_BLK_F_BLK_SIZE if the feature is offered by the
> > > +device. When negotiated, drivers SHOULD interpret the \field{blk_size} as the
> > > +logical block size.
> > > +
> > > +If the VIRTIO_BLK_F_BLK_SIZE feature is not offered by the device, then drivers
> > > +MAY assume that the logical block size is 512 bytes.
> > > +
> > >   Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of
> > >   sending VIRTIO_BLK_T_FLUSH commands.
> > > @@ -319,6 +329,10 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
> > >   \devicenormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization}
> > > +Devices SHOULD always offer VIRTIO_BLK_F_BLK_SIZE feature. When this feature is
> > > +offered, devices MUST initialize \field{blk_size} to a power of two greater
> > > +than or equal to 512.
> > > +
> > >   Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it
> > >   if they offer VIRTIO_BLK_F_CONFIG_WCE.
> > > @@ -879,6 +893,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> > >   The length of \field{data} MUST be a multiple of 512 bytes for VIRTIO_BLK_T_IN
> > >   and VIRTIO_BLK_T_OUT requests.
> > > +The length of \field{data} SHOULD be a multiple of \field{blk_size} for VIRTIO_BLK_T_IN
> > > +and VIRTIO_BLK_T_OUT requests, when the VIRTIO_BLK_F_BLK_SIZE feature has been
> > > +offered by the device.
> > > +
> > I would rewrite it as:
> > 
> > `When the VIRTIO_BLK_F_BLK_SIZE feature has been offered by the device,
> > the length of \field{data} SHOULD be a multiple of \field{blk_size} for
> > VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests`
> > 
> > Just because people tend to read from more important to less important
> > within a sentence but I do not have an strong opinion about it.
> > Apologies if I may sound bit picky ;).
> 
> I don't have a strong opinion on it as well.
> 
> I'm open to making changes, but I'd appreciate getting input from MST/Stefan
> first.
> 
> This way, we can ensure we're aligning with their perspective and avoid
> potential back-and-forth revisions.

Please put the condition first, then the result, this is standard
english.

You can also combine multiple things then.

Also, it should be "is offered".

> > 
> > > +The value of \field{sector} (multiplied by 512) SHOULD be a multiple of
> > > +\field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests, when the
> > > +VIRTIO_BLK_F_BLK_SIZE feature has offered by the device.
> > > +
> > For the same reason, I would rewrite it as:
> > 
> > `When the VIRTIO_BLK_F_BLK_SIZE feature has offered by the device, the

has offered is agrammatical.

> > value of \field{sector} (multiplied by 512) SHOULD be a multiple of
> > \field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests`.
> > 
> > I think I would follow this pattern in the text below too.
> > 
> > >   The length of \field{data} MUST be a multiple of the size of struct
> > >   virtio_blk_discard_write_zeroes for VIRTIO_BLK_T_DISCARD,
> > >   VIRTIO_BLK_T_SECURE_ERASE and VIRTIO_BLK_T_WRITE_ZEROES requests.
> > > @@ -966,6 +988,18 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> > >   for a write request if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT
> > >   write any data.
> > > +A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
> > > +VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the requested
> > > +\field{sector} (multiplied by 512) is not an integer multiple of the device's
> > > +\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
> > > +the device.
> > > +
> > > +A device MAY set the \field{status} to VIRTIO_BLK_S_IOERR for
> > > +VIRTIO_BLK_T_IN or VIRTIO_BLK_T_OUT requests if the length of the
> > > +requested \field{data} is not an integer multiple of the device's
> > > +\field{blk_size}, when the VIRTIO_BLK_F_BLK_SIZE feature has been offered by
> > > +the device.
> > > +
> > >   The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
> > >   discard, secure erase and write zeroes commands if any unknown flag is set.
> > >   Furthermore, the device MUST set the \field{status} byte to
> > > -- 
> > > 2.18.1
> > > 
> > > 


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

end of thread, other threads:[~2024-10-10 14:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 16:05 [PATCH v3 1/1] virtio-blk: Add description for blk_size field Max Gurtovoy
2024-10-10 10:24 ` Matias Ezequiel Vara Larsen
2024-10-10 14:04   ` Max Gurtovoy
2024-10-10 14:15     ` 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;
as well as URLs for NNTP newsgroup(s).