10:44 a.m.
Andrea Bolognani (3):
tests: Improve target index validation coverage
conf: Move target index validation
conf: Use the correct limit for the number of PHBs
src/conf/domain_conf.c | 40 +++++++++++++---------
...ml2argv-pseries-phb-invalid-target-index-1.xml} | 4 ---
...ml2argv-pseries-phb-invalid-target-index-2.xml} | 6 +---
...ml2argv-pseries-phb-invalid-target-index-3.xml} | 11 +++---
tests/qemuxml2argvtest.c | 4 ++-
5 files changed, 32 insertions(+), 33 deletions(-)
copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-1.xml} (79%)
copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-2.xml} (68%)
rename tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-3.xml} (57%)
--
2.13.5
10:44 a.m.
New subject: [libvirt] [PATCH v2 1/3] tests: Improve target index validation coverage
Split one of the existing tests to ensure both configuration
errors it contained cause a failure, and introduce a new
test case.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
...ml => qemuxml2argv-pseries-phb-invalid-target-index-1.xml} | 4 ----
...ml => qemuxml2argv-pseries-phb-invalid-target-index-2.xml} | 6 +-----
...ml => qemuxml2argv-pseries-phb-invalid-target-index-3.xml} | 11 ++++-------
tests/qemuxml2argvtest.c | 4 +++-
4 files changed, 8 insertions(+), 17 deletions(-)
copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-1.xml} (79%)
copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-2.xml} (68%)
rename tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-3.xml} (57%)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-1.xml
similarity index 79%
copy from tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
copy to tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-1.xml
index 48e3b1f80..4de341978 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-1.xml
@@ -12,10 +12,6 @@
<controller type='pci' index='0' model='pci-root'>
<target index='1'/>
</controller>
- <!-- Other PHBs can't have target index 0 -->
- <controller type='pci' index='1' model='pci-root'>
- <target index='0'/>
- </controller>
<controller type='usb' model='none'/>
<memballoon model='none'/>
</devices>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-2.xml
similarity index 68%
copy from tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
copy to tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-2.xml
index 48e3b1f80..5e4aa2635 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-2.xml
@@ -8,11 +8,7 @@
</os>
<devices>
<emulator>/usr/bin/qemu-system-ppc64</emulator>
- <!-- The default PHB (controller index 0) must have target index 0 -->
- <controller type='pci' index='0' model='pci-root'>
- <target index='1'/>
- </controller>
- <!-- Other PHBs can't have target index 0 -->
+ <!-- PHBs other than the default one can't have target index 0 -->
<controller type='pci' index='1' model='pci-root'>
<target index='0'/>
</controller>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
similarity index 57%
rename from tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
rename to tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
index 48e3b1f80..864c5d875 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-wrong-target-index.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
@@ -8,13 +8,10 @@
</os>
<devices>
<emulator>/usr/bin/qemu-system-ppc64</emulator>
- <!-- The default PHB (controller index 0) must have target index 0 -->
- <controller type='pci' index='0' model='pci-root'>
- <target index='1'/>
- </controller>
- <!-- Other PHBs can't have target index 0 -->
- <controller type='pci' index='1' model='pci-root'>
- <target index='0'/>
+ <!-- QEMU only supports 32 PHBs with target index in the range 0-31,
+ so attempting to use target index 32 should fail -->
+ <controller type='pci' model='pci-root'>
+ <target index='32'/>
</controller>
<controller type='usb' model='none'/>
<memballoon model='none'/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 77cfe8d28..5cdbc78eb 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1764,13 +1764,15 @@ mymain(void)
DO_TEST("pseries-phb-default-missing",
QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
- DO_TEST_PARSE_ERROR("pseries-phb-wrong-target-index", NONE);
DO_TEST("pseries-phb-numa-node",
QEMU_CAPS_NUMA,
QEMU_CAPS_OBJECT_MEMORY_RAM,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE);
DO_TEST_PARSE_ERROR("pseries-default-phb-numa-node", NONE);
+ DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-1", NONE);
+ DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-2", NONE);
+ DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-3", NONE);
DO_TEST("pseries-many-devices",
QEMU_CAPS_NODEFCONFIG,
--
2.13.5
10:44 a.m.
New subject: [libvirt] [PATCH v2 2/3] conf: Move target index validation
Validation should happen after parsing, so the proper
location for it is virDomainControllerDefValidate()
rather than virDomainControllerDefParseXML().
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3db56ffb7..86afeaa3f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4995,12 +4995,34 @@ static int
virDomainControllerDefValidate(const virDomainControllerDef *controller)
{
if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+ const virDomainPCIControllerOpts *opts = &controller->opts.pciopts;
+
if (controller->idx > 255) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller index %d too high, maximum is
255"),
controller->idx);
return -1;
}
+
+ /* Only validate the target index if it's been set */
+ if (opts->targetIndex != -1) {
+
+ if (opts->targetIndex < 0 || opts->targetIndex > 31) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("PCI controller target index '%d' out of
"
+ "range - must be 0-31"),
+ opts->targetIndex);
+ return -1;
+ }
+
+ if ((controller->idx == 0 && opts->targetIndex != 0) ||
+ (controller->idx != 0 && opts->targetIndex == 0)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Only the PCI controller with index 0 can "
+ "have target index 0, and vice versa"));
+ return -1;
+ }
+ }
}
return 0;
}
@@ -9322,27 +9344,13 @@ virDomainControllerDefParseXML(xmlNodePtr node,
}
if (targetIndex) {
if (virStrToLong_i(targetIndex, NULL, 0,
- &def->opts.pciopts.targetIndex) < 0) {
+ &def->opts.pciopts.targetIndex) < 0 ||
+ def->opts.pciopts.targetIndex == -1) {
virReportError(VIR_ERR_XML_ERROR,
_("Invalid target index '%s' in PCI
controller"),
targetIndex);
goto error;
}
- if (def->opts.pciopts.targetIndex < 0 ||
- def->opts.pciopts.targetIndex > 31) {
- virReportError(VIR_ERR_XML_ERROR,
- _("PCI controller target index '%s' out of
"
- "range - must be 0-31"),
- targetIndex);
- goto error;
- }
- if ((def->idx == 0 && def->opts.pciopts.targetIndex != 0) ||
- (def->idx != 0 && def->opts.pciopts.targetIndex == 0)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Only the PCI controller with index 0 can "
- "have target index 0, and vice versa"));
- goto error;
- }
}
if (numaNode >= 0) {
if (def->idx == 0) {
--
2.13.5
10:44 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Use the correct limit for the number of PHBs
I mistakenly thought pSeries guests supported 32 PHBs,
but it turns out they only support 31. Validate the
target index accordingly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1479647
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 4 ++--
.../qemuxml2argv-pseries-phb-invalid-target-index-3.xml | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 86afeaa3f..41a2a7bc8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5007,10 +5007,10 @@ virDomainControllerDefValidate(const virDomainControllerDef
*controller)
/* Only validate the target index if it's been set */
if (opts->targetIndex != -1) {
- if (opts->targetIndex < 0 || opts->targetIndex > 31) {
+ if (opts->targetIndex < 0 || opts->targetIndex > 30) {
virReportError(VIR_ERR_XML_ERROR,
_("PCI controller target index '%d' out of
"
- "range - must be 0-31"),
+ "range - must be 0-30"),
opts->targetIndex);
return -1;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
index 864c5d875..3d99da499 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-invalid-target-index-3.xml
@@ -8,10 +8,10 @@
</os>
<devices>
<emulator>/usr/bin/qemu-system-ppc64</emulator>
- <!-- QEMU only supports 32 PHBs with target index in the range 0-31,
- so attempting to use target index 32 should fail -->
+ <!-- QEMU only supports 31 PHBs with target index in the range 0-30,
+ so attempting to use target index 31 should fail -->
<controller type='pci' model='pci-root'>
- <target index='32'/>
+ <target index='31'/>
</controller>
<controller type='usb' model='none'/>
<memballoon model='none'/>
--
2.13.5
7:42 a.m.
On Thu, Aug 17, 2017 at 05:44:20PM +0200, Andrea Bolognani wrote:
Andrea Bolognani (3):
tests: Improve target index validation coverage
conf: Move target index validation
conf: Use the correct limit for the number of PHBs
src/conf/domain_conf.c | 40 +++++++++++++---------
...ml2argv-pseries-phb-invalid-target-index-1.xml} | 4 ---
...ml2argv-pseries-phb-invalid-target-index-2.xml} | 6 +---
...ml2argv-pseries-phb-invalid-target-index-3.xml} | 11 +++---
tests/qemuxml2argvtest.c | 4 ++-
5 files changed, 32 insertions(+), 33 deletions(-)
copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-1.xml} (79%)
copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-2.xml} (68%)
rename tests/qemuxml2argvdata/{qemuxml2argv-pseries-phb-wrong-target-index.xml =>
qemuxml2argv-pseries-phb-invalid-target-index-3.xml} (57%)
ACK series
Jan
2654
days inactive
2655
days old
4 comments
2 participants
participants (2)
-
Andrea Bolognani -
Ján Tomko