Re: [Qemu-devel] [libvirt][PATCH v2 1/2] add pci-bridge controller type

[Doug Goldstein]

On Mon, Jan 7, 2013 at 7:58 PM, liguang <address@hidden> wrote:
> Signed-off-by: liguang <address@hidden>
> ---
>  src/conf/device_conf.c |   12 +++++++++++-
>  src/conf/device_conf.h |    1 +
>  src/conf/domain_conf.c |    5 ++++-
>  src/conf/domain_conf.h |    1 +
>  4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 7b97f45..1c06ed0 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -51,16 +51,18 @@ int
>  virDevicePCIAddressParseXML(xmlNodePtr node,
>                              virDevicePCIAddressPtr addr)
>  {
> -    char *domain, *slot, *bus, *function, *multi;
> +    char *domain, *slot, *bus, *function, *multi, *bridge;
>      int ret = -1;
>
>      memset(addr, 0, sizeof(*addr));
> +    addr->bridge = -1;
>
>      domain   = virXMLPropString(node, "domain");
>      bus      = virXMLPropString(node, "bus");
>      slot     = virXMLPropString(node, "slot");
>      function = virXMLPropString(node, "function");
>      multi    = virXMLPropString(node, "multifunction");
> +    bridge   = virXMLPropString(node, "bridge");
>
>      if (domain &&
>          virStrToLong_ui(domain, NULL, 0, &addr->domain) < 0) {
> @@ -98,6 +100,14 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
>          goto cleanup;
>
>      }
> +
> +    if (bridge &&
> +        virStrToLong_i(bridge, NULL, 0, &addr->bridge) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("pci-bridge number must be >= 0 "));
> +        goto cleanup;
> +    }

This check isn't correct for the error message. The check is saying
that we weren't able to parse out the value specified (look at the
checks earlier in the function). The subsequent checks (where this
code is added) checks for the validity of the values and use
VIR_ERR_CONFIG_UNSUPPORTED.

You're also failing to free bridge in the cleanup section.

> +
>      if (!virDevicePCIAddressIsValid(addr)) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("Insufficient specification for PCI address"));
> diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> index 5318738..7ac3461 100644
> --- a/src/conf/device_conf.h
> +++ b/src/conf/device_conf.h
> @@ -48,6 +48,7 @@ struct _virDevicePCIAddress {
>      unsigned int slot;
>      unsigned int function;
>      int          multi;  /* enum virDomainDeviceAddressPciMulti */
> +    int          bridge; /* for pci-bridge */
>  };
>
>  int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr);
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6a7646e..8ebe77d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -264,7 +264,8 @@ VIR_ENUM_IMPL(virDomainController, 
> VIR_DOMAIN_CONTROLLER_TYPE_LAST,
>                "sata",
>                "virtio-serial",
>                "ccid",
> -              "usb")
> +              "usb",
> +              "pci-bridge")
>
>  VIR_ENUM_IMPL(virDomainControllerModelSCSI, 
> VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
>                "auto",
> @@ -4479,6 +4480,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>          goto error;
>
>      switch (def->type) {
> +    case VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE:
> +        break;
>      case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: {
>          char *ports = virXMLPropString(node, "ports");
>          if (ports) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 5062e07..56e5a40 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -652,6 +652,7 @@ enum virDomainControllerType {
>      VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
>      VIR_DOMAIN_CONTROLLER_TYPE_CCID,
>      VIR_DOMAIN_CONTROLLER_TYPE_USB,
> +    VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE,
>
>      VIR_DOMAIN_CONTROLLER_TYPE_LAST
>  };
> --
> 1.7.2.5
>
>

Looks like:

int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr)

Needs to be updated as well part of this series to allow bus to not be
0 anymore.

This change also needs an update to the XML schemas in
docs/schemas/basictypes.rng

-- 
Doug Goldstein