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 <jferlan@xxxxxxxxxx> --- 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list