Re: [PATCH v3 13/26] conf: Parse and format <target index='...'/>

[Andrea Bolognani <abologna@xxxxxxxxxx>]

On Fri, 2017-06-30 at 17:10 +0530, Shivaprasad bhat wrote:
> > @@ -9348,6 +9351,23 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> >                  goto error;
> >              }
> >          }
> > +        if (idx) {
> > +            if (virStrToLong_i(idx, NULL, 0,
> > +                               &def->opts.pciopts.idx) < 0) {
> > +                virReportError(VIR_ERR_XML_ERROR,
> > +                               _("Invalid target index '%s' in PCI controller"),
> > +                               idx);
> > +                goto error;
> > +            }
> > +            if (def->opts.pciopts.idx < 0 ||
> > +                def->opts.pciopts.idx > 31) {
> > +                virReportError(VIR_ERR_XML_ERROR,
> > +                               _("PCI controller target index '%s' out of "
> > +                                 "range - must be 0-31"),
> > +                               idx);
> > +                goto error;
> > +            }
> > +        }
> 
> Better to enforce def->idx == 0 to have def->opts.pciopts.idx as 0 ? That would always assure def->idx == 0 as the implicit PHB.
> 
> Qemu today allows "-device spapr-pci-host-bridge,index=X,id=pci.0" when X != 0, and we will generate it for implcit phb(which we normally dont)
> when def->opts.pciopts.idx != 0 in Patch 18. Not sure what all problems/confusions could come because of this inconsistency.

Agreed.

The attached patch should be squashed into this commit to
prevent such a configuration from being accepted. I'm also
working on a test case for this specific configuration.

Laine, are you okay with me squashing this in and keeping
the Reviewed-by?

-- 
Andrea Bolognani / Red Hat / Virtualization

From 2a796858ba76268e7ed0da6cf5dbd2f69a0e0ef6 Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <abologna@xxxxxxxxxx>
Date: Tue, 4 Jul 2017 08:04:48 +0200
Subject: [PATCH v3 13.5/26] qemu: Only the default PHB can have targetIndex=0

Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
---
 src/conf/domain_conf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c797f9f..8a64fbc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9376,6 +9376,13 @@ virDomainControllerDefParseXML(xmlNodePtr node,
                                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,
+                               _("Only the PCI controller with index 0 can "
+                                 "have target index 0, and vice versa"));
+                goto error;
+            }
         }
         if (numaNode >= 0)
             def->opts.pciopts.numaNode = numaNode;
-- 
2.7.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list