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

Re: [libvirt] [PATCH v2 09/11] qemu: build command line for pci-bridge device



On 04/17/2013 03:00 PM, Ján Tomko wrote:
> From: liguang <lig fnst cn fujitsu com>
>
> ---
>  src/qemu/qemu_capabilities.c |  3 +++
>  src/qemu/qemu_capabilities.h |  1 +
>  src/qemu/qemu_command.c      | 44 ++++++++++++++++++++++++++++++++++++++++----
>  tests/qemuhelptest.c         | 21 ++++++++++++++-------
>  4 files changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index d10c8aa..4a86d81 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "machine-usb-opt",
>                "tpm-passthrough",
>                "tpm-tis",
> +
> +              "pci-bridge", /* 140 */
>      );
>  
>  struct _virQEMUCaps {
> @@ -1346,6 +1348,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>      { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG },
>      { "rng-random", QEMU_CAPS_OBJECT_RNG_RANDOM },
>      { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD },
> +    { "pci-bridge", QEMU_CAPS_DEVICE_PCI_BRIDGE },
>  };
>  
>  static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 4e76799..9c11157 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -178,6 +178,7 @@ enum virQEMUCapsFlags {
>      QEMU_CAPS_MACHINE_USB_OPT    = 137, /* -machine xxx,usb=on/off */
>      QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */
>      QEMU_CAPS_DEVICE_TPM_TIS     = 139, /* -device tpm_tis */
> +    QEMU_CAPS_DEVICE_PCI_BRIDGE  = 140, /* -device pci-bridge */
>  
>      QEMU_CAPS_LAST,                   /* this must always be the last item */
>  };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1b9d940..ce86eea 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1957,10 +1957,21 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>           * When QEMU grows support for > 1 PCI domain, then pci.0 change
>           * to pciNN.0  where NN is the domain number
>           */
> -        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS))
> -            virBufferAsprintf(buf, ",bus=pci.0");
> -        else
> -            virBufferAsprintf(buf, ",bus=pci");
> +        if (info->addr.pci.bus != 0) {
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
> +                virBufferAsprintf(buf, ",bus=pci.%u", info->addr.pci.bus);
> +            } else {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("Multiple PCI buses are not supported "
> +                                 "with this QEMU binary"));
> +                return -1;
> +            }
> +        } else {
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS))
> +                virBufferAsprintf(buf, ",bus=pci.0");
> +            else
> +                virBufferAsprintf(buf, ",bus=pci");
> +        }

Well, really this hunk is supporting *other* devices that plug into a
pci-bridge device, not the pci-bridge device itself. I think this hunk
would be better put into 10/10 instead of here.

(But it *is* commandline changes related to the presence of a
pci-bridge, and one is pointless without the other, so I'm not going to
complain too much if you leave it here. :-))

>          if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
>              virBufferAddLit(buf, ",multifunction=on");
>          else if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF)
> @@ -3606,6 +3617,24 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>  
>          break;
>  
> +    case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> +        switch (def->model) {
> +        case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
> +            if (def->idx == 0) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("PCI bridge index should be > 0"));
> +                goto error;
> +            }
> +            virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=pci.%d",
> +                              def->idx, def->idx);
> +            break;
> +        case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("wrong function called for pci-root"));
> +            return NULL;
> +        }
> +        break;
> +
>      /* We always get an IDE controller, whether we want it or not. */
>      case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
>      default:
> @@ -5792,6 +5821,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>          /* We don't add an explicit IDE or FD controller because the
>           * provided PIIX4 device already includes one. It isn't possible to
>           * remove the PIIX4. */


You should include mention of the pci-root pci controller in that comment.


> +        VIR_DOMAIN_CONTROLLER_TYPE_PCI,
>          VIR_DOMAIN_CONTROLLER_TYPE_USB,
>          VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
>          VIR_DOMAIN_CONTROLLER_TYPE_SATA,
> @@ -6405,6 +6435,12 @@ qemuBuildCommandLine(virConnectPtr conn,
>                      continue;
>                  }
>  
> +                /* Skip pci-root */
> +                if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
> +                    cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
> +                    continue;
> +                }
> +
>                  /* Only recent QEMU implements a SATA (AHCI) controller */
>                  if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
>                      if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) {
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index 7290183..eeba4d4 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -398,7 +398,8 @@ mymain(void)
>              QEMU_CAPS_DEVICE_CIRRUS_VGA,
>              QEMU_CAPS_DEVICE_VMWARE_SVGA,
>              QEMU_CAPS_DEVICE_USB_SERIAL,
> -            QEMU_CAPS_DEVICE_USB_NET);
> +            QEMU_CAPS_DEVICE_USB_NET,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>      DO_TEST("qemu-kvm-0.12.3", 12003, 1, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -507,7 +508,8 @@ mymain(void)
>              QEMU_CAPS_DEVICE_CIRRUS_VGA,
>              QEMU_CAPS_DEVICE_VMWARE_SVGA,
>              QEMU_CAPS_DEVICE_USB_SERIAL,
> -            QEMU_CAPS_DEVICE_USB_NET);
> +            QEMU_CAPS_DEVICE_USB_NET,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>      DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -572,7 +574,8 @@ mymain(void)
>              QEMU_CAPS_DEVICE_CIRRUS_VGA,
>              QEMU_CAPS_DEVICE_VMWARE_SVGA,
>              QEMU_CAPS_DEVICE_USB_SERIAL,
> -            QEMU_CAPS_DEVICE_USB_NET);
> +            QEMU_CAPS_DEVICE_USB_NET,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>      DO_TEST("qemu-kvm-0.12.1.2-rhel62-beta", 12001, 1, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -644,7 +647,8 @@ mymain(void)
>              QEMU_CAPS_VNC,
>              QEMU_CAPS_DEVICE_QXL,
>              QEMU_CAPS_DEVICE_VGA,
> -            QEMU_CAPS_DEVICE_CIRRUS_VGA);
> +            QEMU_CAPS_DEVICE_CIRRUS_VGA,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>      DO_TEST("qemu-1.0", 1000000, 0, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -816,7 +820,8 @@ mymain(void)
>              QEMU_CAPS_DEVICE_USB_SERIAL,
>              QEMU_CAPS_DEVICE_USB_NET,
>              QEMU_CAPS_DTB,
> -            QEMU_CAPS_IPV6_MIGRATION);
> +            QEMU_CAPS_IPV6_MIGRATION,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>      DO_TEST("qemu-1.2.0", 1002000, 0, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -919,7 +924,8 @@ mymain(void)
>              QEMU_CAPS_DEVICE_USB_NET,
>              QEMU_CAPS_DTB,
>              QEMU_CAPS_SCSI_MEGASAS,
> -            QEMU_CAPS_IPV6_MIGRATION);
> +            QEMU_CAPS_IPV6_MIGRATION,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>      DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -1027,7 +1033,8 @@ mymain(void)
>              QEMU_CAPS_DEVICE_USB_NET,
>              QEMU_CAPS_DTB,
>              QEMU_CAPS_SCSI_MEGASAS,
> -            QEMU_CAPS_IPV6_MIGRATION);
> +            QEMU_CAPS_IPV6_MIGRATION,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE);
>  
>      return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }

ACK with the comment changed (and that one hunk moved to 10/10 if it's
not too big a trouble).


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