[libvirt] [PATCH] conf: Use the correct limit for the number of PHBs

Andrea Bolognani posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170817075542.15630-1-abologna@redhat.com
There is a newer version of this series
src/conf/domain_conf.c                                |  4 ++--
src/qemu/qemu_domain_address.c                        |  4 ++--
.../qemuxml2argv-pseries-phb-too-many.xml             | 19 +++++++++++++++++++
tests/qemuxml2argvtest.c                              |  1 +
4 files changed, 24 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml
[libvirt] [PATCH] conf: Use the correct limit for the number of PHBs
Posted by Andrea Bolognani 6 years, 8 months ago
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 ++--
 src/qemu/qemu_domain_address.c                        |  4 ++--
 .../qemuxml2argv-pseries-phb-too-many.xml             | 19 +++++++++++++++++++
 tests/qemuxml2argvtest.c                              |  1 +
 4 files changed, 24 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 281dc68f0..8257f04ab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9326,10 +9326,10 @@ virDomainControllerDefParseXML(xmlNodePtr node,
                 goto error;
             }
             if (def->opts.pciopts.targetIndex < 0 ||
-                def->opts.pciopts.targetIndex > 31) {
+                def->opts.pciopts.targetIndex > 30) {
                 virReportError(VIR_ERR_XML_ERROR,
                                _("PCI controller target index '%s' out of "
-                                 "range - must be 0-31"),
+                                 "range - must be 0-30"),
                                targetIndex);
                 goto error;
             }
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 8066ed123..04438fae9 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2157,9 +2157,9 @@ qemuDomainAddressFindNewTargetIndex(virDomainDefPtr def)
     int targetIndex;
     int ret = -1;
 
-    /* Try all indexes between 1 and 31 - QEMU only supports 32
+    /* Try all indexes between 1 and 30 - QEMU only supports 31
      * PHBs, and 0 is reserved for the default, implicit one */
-    for (targetIndex = 1; targetIndex <= 31; targetIndex++) {
+    for (targetIndex = 1; targetIndex <= 30; targetIndex++) {
         bool found = false;
         size_t i;
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml
new file mode 100644
index 000000000..3d99da499
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml
@@ -0,0 +1,19 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid>
+  <memory unit='KiB'>524288</memory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='ppc64' machine='pseries'>hvm</type>
+  </os>
+  <devices>
+    <emulator>/usr/bin/qemu-system-ppc64</emulator>
+    <!-- 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='31'/>
+    </controller>
+    <controller type='usb' model='none'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 77cfe8d28..47ca90f04 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1761,6 +1761,7 @@ mymain(void)
     DO_TEST("pseries-phb-simple",
             QEMU_CAPS_NODEFCONFIG,
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+    DO_TEST_PARSE_ERROR("pseries-phb-too-many", NONE);
     DO_TEST("pseries-phb-default-missing",
             QEMU_CAPS_NODEFCONFIG,
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Use the correct limit for the number of PHBs
Posted by Peter Krempa 6 years, 8 months ago
On Thu, Aug 17, 2017 at 09:55:42 +0200, Andrea Bolognani wrote:
> 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 ++--
>  src/qemu/qemu_domain_address.c                        |  4 ++--
>  .../qemuxml2argv-pseries-phb-too-many.xml             | 19 +++++++++++++++++++
>  tests/qemuxml2argvtest.c                              |  1 +
>  4 files changed, 24 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-too-many.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 281dc68f0..8257f04ab 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9326,10 +9326,10 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>                  goto error;
>              }
>              if (def->opts.pciopts.targetIndex < 0 ||
> -                def->opts.pciopts.targetIndex > 31) {
> +                def->opts.pciopts.targetIndex > 30) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 _("PCI controller target index '%s' out of "
> -                                 "range - must be 0-31"),
> +                                 "range - must be 0-30"),

NACK, you can't make parser checks more strict once the code is released.
This needs to go to the validation callback.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list