[libvirt] [PATCH v2 0/3] conf: Use the correct limit for the number of PHBs
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
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@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
Validation should happen after parsing, so the proper location for it is virDomainControllerDefValidate() rather than virDomainControllerDefParseXML(). Signed-off-by: Andrea Bolognani <abologna@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
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@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
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
participants (2)
-
Andrea Bolognani -
Ján Tomko