Re: [libvirt] [PATCH v2 1/4] domain_conf: Introduce iothreads XML
On Tue, Aug 26, 2014 at 11:15 PM, John Ferlan wrote: > +The content of this optional element defines the number > +of IOThreads to be assigned to the domain for use by > +virtio-blk-pci target storage devices. There should be > +only 1 or 2 IOThreads per host CPU and 1 IOThread per > +supported device. Fine for now but in the future devices may support multiple IOThreads assigned. This is necessary for multiqueue block layer support, where a storage controller can submit I/O from multiple host CPUs at the same time. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] domain_conf: Introduce iothreads XML
On 08/26/2014 06:15 PM, John Ferlan wrote:
> Introduce XML to allowing adding iothreads to the domain. These can be
> used by virtio-blk-pci devices in order to assign a specific thread to
> handle the workload for the device. The iothreads are the official
> implementation of the virtio-blk Data Plane that's been in tech preview
> for QEMU.
>
> Signed-off-by: John Ferlan
> ---
> docs/formatdomain.html.in | 26 ++
> docs/schemas/domaincommon.rng | 6 ++
> src/conf/domain_conf.c| 20
> src/conf/domain_conf.h| 2 ++
> 4 files changed, 54 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index de4e4eb..b584a08 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -470,6 +470,32 @@
>
>
>
> +IOThreads Allocation
> +
> +IOThreads are a QEMU feature that will allow supported disk
> +devices to be configured to use a dedicated event loop thread
> +to handle block I/O requests.
> +Since 1.2.8 (QEMU only)
> +
Changed the text to:
IOThreads are dedicated event loop threads for supported disk
devices to perform block I/O requests in order to improve
scalability especially on an SMP host/guest with many LUNs.
Which probably is the best description I could find from my bz.
<...snip...>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index dd512ca..81a3fdf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11957,6 +11957,15 @@ virDomainDefParseXML(xmlDocPtr xml,
> }
> }
>
> +/* Optional - iothreads */
> +tmp = virXPathString("string(./iothreads[1])", ctxt);
> +if (tmp && virStrToLong_uip(tmp, NULL, 0, &def->iothreads) < 0) {
And yes s/10/0 was done here as well - so much for cut-n-paste :-)
<...snip...>
Tks,
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] domain_conf: Introduce iothreads XML
On Tue, Aug 26, 2014 at 06:15:46PM -0400, John Ferlan wrote:
Introduce XML to allowing adding iothreads to the domain. These can be
used by virtio-blk-pci devices in order to assign a specific thread to
handle the workload for the device. The iothreads are the official
implementation of the virtio-blk Data Plane that's been in tech preview
for QEMU.
Signed-off-by: John Ferlan
---
docs/formatdomain.html.in | 26 ++
docs/schemas/domaincommon.rng | 6 ++
src/conf/domain_conf.c| 20
src/conf/domain_conf.h| 2 ++
4 files changed, 54 insertions(+)
I forgot two things in the review.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dd512ca..81a3fdf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11957,6 +11957,15 @@ virDomainDefParseXML(xmlDocPtr xml,
}
}
+/* Optional - iothreads */
+tmp = virXPathString("string(./iothreads[1])", ctxt);
+if (tmp && virStrToLong_uip(tmp, NULL, 0, &def->iothreads) < 0) {
One of the things is that you use '0' here (as I suggested, sorry for
that), which would not that big of a deal, it would even be awesome,
but unfortunately we use '10' everywhere in the XML parsing code :(
Even when parsing the iothread="" in 3/4 you are using '10', so this
should be changed to '10'.
The second thing is that there might be a qemuxml2xmltest case for the
qemuxml2argv-iothreads.xml (which can be added in this patch), but
that's just a bike-shedding.
The ACK still stands, of course.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] domain_conf: Introduce iothreads XML
On Tue, Aug 26, 2014 at 06:15:46PM -0400, John Ferlan wrote: Introduce XML to allowing adding iothreads to the domain. These can be used by virtio-blk-pci devices in order to assign a specific thread to handle the workload for the device. The iothreads are the official implementation of the virtio-blk Data Plane that's been in tech preview for QEMU. Signed-off-by: John Ferlan --- docs/formatdomain.html.in | 26 ++ docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c| 20 src/conf/domain_conf.h| 2 ++ 4 files changed, 54 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de4e4eb..b584a08 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -470,6 +470,32 @@ +IOThreads Allocation + +IOThreads are a QEMU feature that will allow supported disk s/QEMU feature that will allow/feature that allow/ because you say it's QEMU only three lines down the page. TBH I'm not sure it should be "allow" (plural "IOThreads") or "allow" (singular "feature"), but that I leave to you since I'm not a native speaker. ACK with that line fixed up. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
