[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.

I don't think you need the first occurence of the word "will" here -
documentation should always read as if this is the way things are now,
it's the way things have always been, and the way they always will be in
the future (except the "Since x.y.z" tags).


Aside from this, I'm still uncomfortable about the lack of new tests
here, because my own experience is that new code is alsmot always
incorrect until it's been run through some tests. But since you're
remedying this problem in a later patch in the series, I'm okay with it.


Reviewed-by: Laine Stump <laine laine org>


> +      </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
>       */
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]