Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
docs/formatdomain.html.in | 5 +++++
docs/schemas/domaincommon.rng | 5 +++++
src/conf/domain_conf.c | 24 ++++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
4 files changed, 35 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a55a9e1..159c243 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3768,6 +3768,11 @@
libvirt API to attach host devices to the correct
pci-expander-bus when assigning them to the domain).
</dd>
+ <dt><code>index</code></dt>
+ <dd>
+ pci-root controllers for pSeries guests will use this attribute to
+ record the order they will show up in the guest.
+ </dd>
</dl>
<p>
For machine types which provide an implicit PCI bus, the pci-root
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e259e3e..39eff46 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1997,6 +1997,11 @@
</attribute>
</optional>
<optional>
+ <attribute name='index'>
+ <ref name='uint8'/>
+ </attribute>
+ </optional>
+ <optional>
<element name='node'>
<ref name='unsignedInt'/>
</element>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 29268a9..1538747 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1866,6 +1866,7 @@ virDomainControllerDefNew(virDomainControllerType type)
def->opts.pciopts.chassis = -1;
def->opts.pciopts.port = -1;
def->opts.pciopts.busNr = -1;
+ def->opts.pciopts.idx = -1;
def->opts.pciopts.numaNode = -1;
break;
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
@@ -9099,6 +9100,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
goto error;
}
def->idx = idxVal;
+ VIR_FREE(idx);
}
cur = node->children;
@@ -9130,6 +9132,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
chassis = virXMLPropString(cur, "chassis");
port = virXMLPropString(cur, "port");
busNr = virXMLPropString(cur, "busNr");
+ idx = virXMLPropString(cur, "index");
processedTarget = true;
}
}
@@ -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;
+ }
+ }
if (numaNode >= 0)
def->opts.pciopts.numaNode = numaNode;
break;
@@ -21748,6 +21768,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
def->opts.pciopts.chassis != -1 ||
def->opts.pciopts.port != -1 ||
def->opts.pciopts.busNr != -1 ||
+ def->opts.pciopts.idx != -1 ||
def->opts.pciopts.numaNode != -1) {
virBufferAddLit(&childBuf, "<target");
if (def->opts.pciopts.chassisNr != -1)
@@ -21762,6 +21783,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
if (def->opts.pciopts.busNr != -1)
virBufferAsprintf(&childBuf, " busNr='%d'",
def->opts.pciopts.busNr);
+ if (def->opts.pciopts.idx != -1)
+ virBufferAsprintf(&childBuf, " index='%d'",
+ def->opts.pciopts.idx);
if (def->opts.pciopts.numaNode == -1) {
virBufferAddLit(&childBuf, "/>\n");
} else {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6d9ee97..53a10db 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -785,6 +785,7 @@ struct _virDomainPCIControllerOpts {
int chassis;
int port;
int busNr; /* used by pci-expander-bus, -1 == unspecified */
+ int idx; /* used by spapr-pci-host-bridge, -1 == unspecified */
/* numaNode is a *subelement* of target (to match existing
* item in memory target config) -1 == unspecified
*/
--
2.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jun 23, 2017 at 8:33 PM, Andrea Bolognani <abologna@redhat.com>
wrote:
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> docs/formatdomain.html.in | 5 +++++
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/domain_conf.c | 24 ++++++++++++++++++++++++
> src/conf/domain_conf.h | 1 +
> 4 files changed, 35 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index a55a9e1..159c243 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3768,6 +3768,11 @@
> libvirt API to attach host devices to the correct
> pci-expander-bus when assigning them to the domain).
> </dd>
> + <dt><code>index</code></dt>
> + <dd>
> + pci-root controllers for pSeries guests will use this attribute to
> + record the order they will show up in the guest.
> + </dd>
> </dl>
> <p>
> For machine types which provide an implicit PCI bus, the pci-root
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index e259e3e..39eff46 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1997,6 +1997,11 @@
> </attribute>
> </optional>
> <optional>
> + <attribute name='index'>
> + <ref name='uint8'/>
> + </attribute>
> + </optional>
> + <optional>
> <element name='node'>
> <ref name='unsignedInt'/>
> </element>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 29268a9..1538747 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1866,6 +1866,7 @@ virDomainControllerDefNew(virDomainControllerType
> type)
> def->opts.pciopts.chassis = -1;
> def->opts.pciopts.port = -1;
> def->opts.pciopts.busNr = -1;
> + def->opts.pciopts.idx = -1;
> def->opts.pciopts.numaNode = -1;
> break;
> case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> @@ -9099,6 +9100,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> goto error;
> }
> def->idx = idxVal;
> + VIR_FREE(idx);
> }
>
> cur = node->children;
> @@ -9130,6 +9132,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> chassis = virXMLPropString(cur, "chassis");
> port = virXMLPropString(cur, "port");
> busNr = virXMLPropString(cur, "busNr");
> + idx = virXMLPropString(cur, "index");
> processedTarget = true;
> }
> }
> @@ -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.
if (numaNode >= 0)
> def->opts.pciopts.numaNode = numaNode;
> break;
> @@ -21748,6 +21768,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
> def->opts.pciopts.chassis != -1 ||
> def->opts.pciopts.port != -1 ||
> def->opts.pciopts.busNr != -1 ||
> + def->opts.pciopts.idx != -1 ||
> def->opts.pciopts.numaNode != -1) {
> virBufferAddLit(&childBuf, "<target");
> if (def->opts.pciopts.chassisNr != -1)
> @@ -21762,6 +21783,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
> if (def->opts.pciopts.busNr != -1)
> virBufferAsprintf(&childBuf, " busNr='%d'",
> def->opts.pciopts.busNr);
> + if (def->opts.pciopts.idx != -1)
> + virBufferAsprintf(&childBuf, " index='%d'",
> + def->opts.pciopts.idx);
> if (def->opts.pciopts.numaNode == -1) {
> virBufferAddLit(&childBuf, "/>\n");
> } else {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 6d9ee97..53a10db 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -785,6 +785,7 @@ struct _virDomainPCIControllerOpts {
> int chassis;
> int port;
> int busNr; /* used by pci-expander-bus, -1 == unspecified */
> + int idx; /* used by spapr-pci-host-bridge, -1 == unspecified */
> /* numaNode is a *subelement* of target (to match existing
> * item in memory target config) -1 == unspecified
> */
> --
> 2.7.5
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/04/2017 07:32 AM, Andrea Bolognani wrote:
> 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?
Sure, that's fine with me.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 06/23/2017 11:03 AM, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> docs/formatdomain.html.in | 5 +++++
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/domain_conf.c | 24 ++++++++++++++++++++++++
> src/conf/domain_conf.h | 1 +
> 4 files changed, 35 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index a55a9e1..159c243 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3768,6 +3768,11 @@
> libvirt API to attach host devices to the correct
> pci-expander-bus when assigning them to the domain).
> </dd>
> + <dt><code>index</code></dt>
> + <dd>
> + pci-root controllers for pSeries guests will use this attribute to
> + record the order they will show up in the guest.
> + </dd>
> </dl>
> <p>
> For machine types which provide an implicit PCI bus, the pci-root
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index e259e3e..39eff46 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1997,6 +1997,11 @@
> </attribute>
> </optional>
> <optional>
> + <attribute name='index'>
> + <ref name='uint8'/>
> + </attribute>
> + </optional>
> + <optional>
> <element name='node'>
> <ref name='unsignedInt'/>
> </element>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 29268a9..1538747 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1866,6 +1866,7 @@ virDomainControllerDefNew(virDomainControllerType type)
> def->opts.pciopts.chassis = -1;
> def->opts.pciopts.port = -1;
> def->opts.pciopts.busNr = -1;
> + def->opts.pciopts.idx = -1;
> def->opts.pciopts.numaNode = -1;
> break;
> case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> @@ -9099,6 +9100,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> goto error;
> }
> def->idx = idxVal;
> + VIR_FREE(idx);
> }
>
> cur = node->children;
> @@ -9130,6 +9132,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> chassis = virXMLPropString(cur, "chassis");
> port = virXMLPropString(cur, "port");
> busNr = virXMLPropString(cur, "busNr");
> + idx = virXMLPropString(cur, "index");
> processedTarget = true;
> }
> }
> @@ -9348,6 +9351,23 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> goto error;
> }
> }
> + if (idx) {
Oh wait - I thought you were going to rename this to targetIdx (both the
locals and the member of the struct) to avoid confusion.
So make my ACK conditional on that.
> + 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;
> + }
> + }
> if (numaNode >= 0)
> def->opts.pciopts.numaNode = numaNode;
> break;
> @@ -21748,6 +21768,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
> def->opts.pciopts.chassis != -1 ||
> def->opts.pciopts.port != -1 ||
> def->opts.pciopts.busNr != -1 ||
> + def->opts.pciopts.idx != -1 ||
> def->opts.pciopts.numaNode != -1) {
> virBufferAddLit(&childBuf, "<target");
> if (def->opts.pciopts.chassisNr != -1)
> @@ -21762,6 +21783,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
> if (def->opts.pciopts.busNr != -1)
> virBufferAsprintf(&childBuf, " busNr='%d'",
> def->opts.pciopts.busNr);
> + if (def->opts.pciopts.idx != -1)
> + virBufferAsprintf(&childBuf, " index='%d'",
> + def->opts.pciopts.idx);
> if (def->opts.pciopts.numaNode == -1) {
> virBufferAddLit(&childBuf, "/>\n");
> } else {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 6d9ee97..53a10db 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -785,6 +785,7 @@ struct _virDomainPCIControllerOpts {
> int chassis;
> int port;
> int busNr; /* used by pci-expander-bus, -1 == unspecified */
> + int idx; /* used by spapr-pci-host-bridge, -1 == unspecified */
> /* numaNode is a *subelement* of target (to match existing
> * item in memory target config) -1 == unspecified
> */
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Sun, 2017-06-25 at 15:57 -0400, Laine Stump wrote:
> > @@ -9348,6 +9351,23 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> > goto error;
> > }
> > }
> > + if (idx) {
>
> Oh wait - I thought you were going to rename this to targetIdx (both the
> locals and the member of the struct) to avoid confusion.
>
> So make my ACK conditional on that.
Yeah, that's a good idea. Consider it done.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.