Re: [libvirt] [PATCH v5 05/10] conf: Adjust the iothreadsched expectations
On Fri, Apr 24, 2015 at 12:05:57 -0400, John Ferlan wrote:
With iothreadid's allowing any 'id' value for an iothread_id, the
iothreadsched code needs a slight adjustment to allow for any
unsigned int value in order to create the bitmap of ids that will
have scheduler adjustments. Adjusted the doc description as well.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in| 16
src/conf/domain_conf.c | 2 +-
.../qemuxml2argv-cputune-iothreadsched-toomuch.xml | 3 ++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7af6bd7..0767a2a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -691,10 +691,18 @@
type (values codebatch/code, codeidle/code,
codefifo/code,
coderr/code) for particular vCPU/IOThread threads (based on
codevcpus/code and codeiothreads/code, leaving out
-codevcpus/code/codeiothreads/code sets the default). For
-real-time schedulers (codefifo/code, coderr/code), priority
must
-be specified as well (and is ignored for non-real-time ones). The
value
-range for the priority depends on the host kernel (usually 1-99).
+codevcpus/code/codeiothreads/code sets the default). Valid
+codevcpus/code values start at 0 through one less than the
+number of vCPU's defined for the domain. Valid codeiothreads/code
+values are described in the codeiothreadids/code
+a href=#elementsIOThreadsAllocationcodedescription/code/a.
+If no codeiothreadids/code are defined, then libvirt numbers
+IOThreads from 1 to the number of codeiothreads/code available
+for the domain. For real-time schedulers (codefifo/code,
+coderr/code), priority must real-time schedulers
+(codefifo/code, coderr/code), priority must be specified as
+well (and is ignored for non-real-time ones). The value range
+for the priority depends on the host kernel (usually 1-99).
span class=sinceSince 1.2.13/span
/dd
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 129908d..9d4c916 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14348,7 +14348,7 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i def-cputune.niothreadsched; i++) {
if (virDomainThreadSchedParse(nodes[i],
- 1, def-iothreads,
+ 1, UINT_MAX,
iothreads,
def-cputune.iothreadsched[i])
0)
goto error;
I think this patch should also add code that checks that the provided
scheduler info is provided only for valid iothread IDs.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 05/10] conf: Adjust the iothreadsched expectations
On 04/27/2015 10:28 AM, Peter Krempa wrote:
On Fri, Apr 24, 2015 at 12:05:57 -0400, John Ferlan wrote:
With iothreadid's allowing any 'id' value for an iothread_id, the
iothreadsched code needs a slight adjustment to allow for any
unsigned int value in order to create the bitmap of ids that will
have scheduler adjustments. Adjusted the doc description as well.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in| 16
src/conf/domain_conf.c | 2 +-
.../qemuxml2argv-cputune-iothreadsched-toomuch.xml | 3 ++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7af6bd7..0767a2a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -691,10 +691,18 @@
type (values codebatch/code, codeidle/code,
codefifo/code,
coderr/code) for particular vCPU/IOThread threads (based on
codevcpus/code and codeiothreads/code, leaving out
-codevcpus/code/codeiothreads/code sets the default). For
-real-time schedulers (codefifo/code, coderr/code), priority
must
-be specified as well (and is ignored for non-real-time ones). The
value
-range for the priority depends on the host kernel (usually 1-99).
+codevcpus/code/codeiothreads/code sets the default). Valid
+codevcpus/code values start at 0 through one less than the
+number of vCPU's defined for the domain. Valid
codeiothreads/code
+values are described in the codeiothreadids/code
+a href=#elementsIOThreadsAllocationcodedescription/code/a.
+If no codeiothreadids/code are defined, then libvirt numbers
+IOThreads from 1 to the number of codeiothreads/code available
+for the domain. For real-time schedulers (codefifo/code,
+coderr/code), priority must real-time schedulers
+(codefifo/code, coderr/code), priority must be specified as
+well (and is ignored for non-real-time ones). The value range
+for the priority depends on the host kernel (usually 1-99).
span class=sinceSince 1.2.13/span
/dd
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 129908d..9d4c916 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14348,7 +14348,7 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i def-cputune.niothreadsched; i++) {
if (virDomainThreadSchedParse(nodes[i],
- 1, def-iothreads,
+ 1, UINT_MAX,
iothreads,
def-cputune.iothreadsched[i])
0)
goto error;
I think this patch should also add code that checks that the provided
scheduler info is provided only for valid iothread IDs.
Yuck... I know you eschew inline diffs, but it's just easier if nothing
else just to make progress:
index b6a8129..7da94bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14348,12 +14348,24 @@ virDomainDefParseXML(xmlDocPtr xml,
def-cputune.niothreadsched = n;
for (i = 0; i def-cputune.niothreadsched; i++) {
+ssize_t pos = -1;
+
if (virDomainThreadSchedParse(nodes[i],
1, UINT_MAX,
iothreads,
def-cputune.iothreadsched[i]) 0)
goto error;
+while ((pos =
virBitmapNextSetBit(def-cputune.iothreadsched[i].ids,
+ pos)) -1) {
+if (!virDomainIOThreadIDFind(def, pos)) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(iothreadsched attribute 'iothreads'
+ uses undefined iothread ids));
+goto error;
+}
+}
+
for (j = 0; j i; j++) {
if (virBitmapOverlaps(def-cputune.iothreadsched[i].ids,
def-cputune.iothreadsched[j].ids)) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 05/10] conf: Adjust the iothreadsched expectations
On Mon, Apr 27, 2015 at 11:27:09 -0400, John Ferlan wrote:
On 04/27/2015 10:28 AM, Peter Krempa wrote:
On Fri, Apr 24, 2015 at 12:05:57 -0400, John Ferlan wrote:
With iothreadid's allowing any 'id' value for an iothread_id, the
iothreadsched code needs a slight adjustment to allow for any
unsigned int value in order to create the bitmap of ids that will
have scheduler adjustments. Adjusted the doc description as well.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in| 16
src/conf/domain_conf.c | 2 +-
.../qemuxml2argv-cputune-iothreadsched-toomuch.xml | 3 ++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7af6bd7..0767a2a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -691,10 +691,18 @@
type (values codebatch/code, codeidle/code,
codefifo/code,
coderr/code) for particular vCPU/IOThread threads (based on
codevcpus/code and codeiothreads/code, leaving out
-codevcpus/code/codeiothreads/code sets the default). For
-real-time schedulers (codefifo/code, coderr/code),
priority must
-be specified as well (and is ignored for non-real-time ones). The
value
-range for the priority depends on the host kernel (usually 1-99).
+codevcpus/code/codeiothreads/code sets the default). Valid
+codevcpus/code values start at 0 through one less than the
+number of vCPU's defined for the domain. Valid
codeiothreads/code
+values are described in the codeiothreadids/code
+a
href=#elementsIOThreadsAllocationcodedescription/code/a.
+If no codeiothreadids/code are defined, then libvirt numbers
+IOThreads from 1 to the number of codeiothreads/code available
+for the domain. For real-time schedulers (codefifo/code,
+coderr/code), priority must real-time schedulers
+(codefifo/code, coderr/code), priority must be specified
as
+well (and is ignored for non-real-time ones). The value range
+for the priority depends on the host kernel (usually 1-99).
span class=sinceSince 1.2.13/span
/dd
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 129908d..9d4c916 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14348,7 +14348,7 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i def-cputune.niothreadsched; i++) {
if (virDomainThreadSchedParse(nodes[i],
- 1, def-iothreads,
+ 1, UINT_MAX,
iothreads,
def-cputune.iothreadsched[i])
0)
goto error;
I think this patch should also add code that checks that the provided
scheduler info is provided only for valid iothread IDs.
Yuck... I know you eschew inline diffs, but it's just easier if nothing
else just to make progress:
I only dislike them when they are multi-hunk or generally too big to
review without compiling the code. This one is okay, small enough to see
what's going on.
index b6a8129..7da94bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14348,12 +14348,24 @@ virDomainDefParseXML(xmlDocPtr xml,
def-cputune.niothreadsched = n;
for (i = 0; i def-cputune.niothreadsched; i++) {
+ssize_t pos = -1;
+
if (virDomainThreadSchedParse(nodes[i],
1, UINT_MAX,
iothreads,
def-cputune.iothreadsched[i])
0)
goto error;
+while ((pos =
virBitmapNextSetBit(def-cputune.iothreadsched[i].ids,
+ pos)) -1) {
+if (!virDomainIOThreadIDFind(def, pos)) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(iothreadsched attribute 'iothreads'
+ uses undefined iothread ids));
+goto error;
Unfortunately we don't have the string containing the bitmap here.
It would make for a nice error message. Not worth changing though.
+}
+}
+
for (j = 0; j i; j++) {
if (virBitmapOverlaps(def-cputune.iothreadsched[i].ids,
def-cputune.iothreadsched[j].ids)) {
ACK with the squash-in.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
