1. Patchew
  2. Libvirt
  3. View series

[PATCH] docs: Mention vhostuser for queues and queue_size

Han Han posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230720062212.192059-1-hhan@redhat.com
There is a newer version of this series
docs/formatdomain.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] docs: Mention vhostuser for queues and queue_size
Posted by Han Han 10 months ago 
These two attributes are supported for vhost-user-blk as well.

Signed-off-by: Han Han <hhan@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` )
    -  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
Re: [PATCH] docs: Mention vhostuser for queues and queue_size
Posted by Michal Prívozník 10 months ago 
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 <hhan@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.

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
Posted by Han Han 10 months ago 
On Thu, Jul 20, 2023 at 2:56 PM Michal Prívozník <mprivozn@redhat.com>
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 <hhan@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.

> 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
Posted by Michal Prívozník 10 months ago 
On 7/20/23 10:53, Han Han wrote:
> 
> 
> On Thu, Jul 20, 2023 at 2:56 PM Michal Prívozník <mprivozn@redhat.com
> <mailto:mprivozn@redhat.com>> 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 <hhan@redhat.com <mailto:hhan@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.

Michal
Re: [PATCH] docs: Mention vhostuser for queues and queue_size
Posted by Han Han 9 months, 4 weeks ago 
On Fri, Jul 21, 2023 at 8:18 PM Michal Prívozník <mprivozn@redhat.com>
wrote:

> On 7/20/23 10:53, Han Han wrote:
> >
> >
> > On Thu, Jul 20, 2023 at 2:56 PM Michal Prívozník <mprivozn@redhat.com
> > <mailto:mprivozn@redhat.com>> 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 <hhan@redhat.com <mailto:hhan@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
>
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.