Re: [PATCH] docs: Mention vhostuser for queues and queue_size

[Han Han]

On Fri, Jul 21, 2023 at 8:18 PM Michal Prívozník 
wrote:

> On 7/20/23 10:53, Han Han wrote:
> >
> >
> > On Thu, Jul 20, 2023 at 2:56 PM Michal Prívozník  > > wrote:
> >
> > On 7/20/23 08:22, Han Han wrote:
> > > These two attributes are supported for vhost-user-blk as well.
> > >
> > > Signed-off-by: Han Han mailto:h...@redhat.com>>
> > > ---
> > >  docs/formatdomain.rst | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > > index 4af0b82569..447ab32c01 100644
> > > --- a/docs/formatdomain.rst
> > > +++ b/docs/formatdomain.rst
> > > @@ -3275,9 +3275,9 @@ paravirtualized driver is specified via the
> > ``disk`` element.
> > >"virtio" ``bus`` and "pci" or "ccw" ``address`` types.
> > :since:`Since 1.2.8
> > >(QEMU 2.1)`
> > > -  The optional ``queues`` attribute specifies the number of
> > virt queues for
> > > -  virtio-blk. ( :since:`Since 3.9.0` )
> > > +  virtio-blk or vhost-user-blk. ( :since:`Since 3.9.0` )
> >
> > This doesn't feel right. The vhost-user-blk disk was introduced
> fairly
> > recently and 3.9.0 is just ancient. Digging into commits,
> vhost-user-blk
> > disk was introduced in v7.1.0 and I didn't check whether it supported
> > the attribute from the very beginning.
> >
> > Thank you for pointing this out.  It looks libvirt allows the user to
> > set the queues for
> > all the bus=='virtio':
> >
> https://gitlab.com/libvirt/libvirt/-/blob/v7.1.0/src/conf/domain_validate.c#L465
> <
> https://gitlab.com/libvirt/libvirt/-/blob/v7.1.0/src/conf/domain_validate.c#L465
> >
> >
> > So vhost-user-blk could use the queues at the beginning.
>
> Okay, can you post v2 then please? I'll review it.
>
> Yeah. V2 link:
https://listman.redhat.com/archives/libvir-list/2023-July/240841.html

> Michal
>
>

Re: [PATCH] docs: Mention vhostuser for queues and queue_size

[Michal Prívozník]

On 7/20/23 10:53, Han Han wrote:
> 
> 
> On Thu, Jul 20, 2023 at 2:56 PM Michal Prívozník  > wrote:
> 
> On 7/20/23 08:22, Han Han wrote:
> > These two attributes are supported for vhost-user-blk as well.
> >
> > Signed-off-by: Han Han mailto:h...@redhat.com>>
> > ---
> >  docs/formatdomain.rst | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 4af0b82569..447ab32c01 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -3275,9 +3275,9 @@ paravirtualized driver is specified via the
> ``disk`` element.
> >        "virtio" ``bus`` and "pci" or "ccw" ``address`` types.
> :since:`Since 1.2.8
> >        (QEMU 2.1)`
> >     -  The optional ``queues`` attribute specifies the number of
> virt queues for
> > -      virtio-blk. ( :since:`Since 3.9.0` )
> > +      virtio-blk or vhost-user-blk. ( :since:`Since 3.9.0` )
> 
> This doesn't feel right. The vhost-user-blk disk was introduced fairly
> recently and 3.9.0 is just ancient. Digging into commits, vhost-user-blk
> disk was introduced in v7.1.0 and I didn't check whether it supported
> the attribute from the very beginning.
> 
> Thank you for pointing this out.  It looks libvirt allows the user to
> set the queues for
> all the bus=='virtio': 
> https://gitlab.com/libvirt/libvirt/-/blob/v7.1.0/src/conf/domain_validate.c#L465
>  
> 
> 
> So vhost-user-blk could use the queues at the beginning.

Okay, can you post v2 then please? I'll review it.

Michal

Re: [PATCH] docs: Mention vhostuser for queues and queue_size

[Han Han]

On Thu, Jul 20, 2023 at 2:56 PM Michal Prívozník 
wrote:

> On 7/20/23 08:22, Han Han wrote:
> > These two attributes are supported for vhost-user-blk as well.
> >
> > Signed-off-by: Han Han 
> > ---
> >  docs/formatdomain.rst | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 4af0b82569..447ab32c01 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -3275,9 +3275,9 @@ paravirtualized driver is specified via the
> ``disk`` element.
> >"virtio" ``bus`` and "pci" or "ccw" ``address`` types.
> :since:`Since 1.2.8
> >(QEMU 2.1)`
> > -  The optional ``queues`` attribute specifies the number of virt
> queues for
> > -  virtio-blk. ( :since:`Since 3.9.0` )
> > +  virtio-blk or vhost-user-blk. ( :since:`Since 3.9.0` )
>
> This doesn't feel right. The vhost-user-blk disk was introduced fairly
> recently and 3.9.0 is just ancient. Digging into commits, vhost-user-blk
> disk was introduced in v7.1.0 and I didn't check whether it supported
> the attribute from the very beginning.
>
> Thank you for pointing this out.  It looks libvirt allows the user to set
the queues for
all the bus=='virtio':
https://gitlab.com/libvirt/libvirt/-/blob/v7.1.0/src/conf/domain_validate.c#L465

So vhost-user-blk could use the queues at the beginning.

> I think if we want to document the attribute then we should probably do
> it like this:
>
> The optional ``queues`` attribute specifies the number of virt queues
> for virtio-blk ( :sinnce:`Since 3.9.0) or vhost-user-blk ( :since:`Since
> X.Y.Z` ).
>
> > -  The optional ``queue_size`` attribute specifies the size of each
> virt
> > -  queue for virtio-blk. ( :since:`Since 7.8.0` )
> > +  queue for virtio-blk or vhost-user-blk. ( :since:`Since 7.8.0` )
> > -  For virtio disks, `Virtio-related options`_ can also
> >be set. ( :since:`Since 3.5.0` )
> > -  The optional ``metadata_cache`` subelement controls aspects
> related to the
>
> I have not checked this later hunk.
>
> Michal
>
>

Re: [PATCH] docs: Mention vhostuser for queues and queue_size

[Michal Prívozník]

On 7/20/23 08:22, Han Han wrote:
> These two attributes are supported for vhost-user-blk as well.
> 
> Signed-off-by: Han Han 
> ---
>  docs/formatdomain.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 4af0b82569..447ab32c01 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3275,9 +3275,9 @@ paravirtualized driver is specified via the ``disk`` 
> element.
>"virtio" ``bus`` and "pci" or "ccw" ``address`` types. :since:`Since 
> 1.2.8
>(QEMU 2.1)`
> -  The optional ``queues`` attribute specifies the number of virt queues 
> for
> -  virtio-blk. ( :since:`Since 3.9.0` )
> +  virtio-blk or vhost-user-blk. ( :since:`Since 3.9.0` )

This doesn't feel right. The vhost-user-blk disk was introduced fairly
recently and 3.9.0 is just ancient. Digging into commits, vhost-user-blk
disk was introduced in v7.1.0 and I didn't check whether it supported
the attribute from the very beginning.

I think if we want to document the attribute then we should probably do
it like this:

The optional ``queues`` attribute specifies the number of virt queues
for virtio-blk ( :sinnce:`Since 3.9.0) or vhost-user-blk ( :since:`Since
X.Y.Z` ).

> -  The optional ``queue_size`` attribute specifies the size of each virt
> -  queue for virtio-blk. ( :since:`Since 7.8.0` )
> +  queue for virtio-blk or vhost-user-blk. ( :since:`Since 7.8.0` )
> -  For virtio disks, `Virtio-related options`_ can also
>be set. ( :since:`Since 3.5.0` )
> -  The optional ``metadata_cache`` subelement controls aspects related to 
> the

I have not checked this later hunk.

Michal

[PATCH] docs: Mention vhostuser for queues and queue_size

[Han Han]

These two attributes are supported for vhost-user-blk as well.

Signed-off-by: Han Han 
---
 docs/formatdomain.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 4af0b82569..447ab32c01 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3275,9 +3275,9 @@ paravirtualized driver is specified via the ``disk`` 
element.
   "virtio" ``bus`` and "pci" or "ccw" ``address`` types. :since:`Since 
1.2.8
   (QEMU 2.1)`
-  The optional ``queues`` attribute specifies the number of virt queues for
-  virtio-blk. ( :since:`Since 3.9.0` )
+  virtio-blk or vhost-user-blk. ( :since:`Since 3.9.0` )
-  The optional ``queue_size`` attribute specifies the size of each virt
-  queue for virtio-blk. ( :since:`Since 7.8.0` )
+  queue for virtio-blk or vhost-user-blk. ( :since:`Since 7.8.0` )
-  For virtio disks, `Virtio-related options`_ can also
   be set. ( :since:`Since 3.5.0` )
-  The optional ``metadata_cache`` subelement controls aspects related to 
the
-- 
2.41.0