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

Re: [libvirt] [PATCH 5/7] qemu: build command line for pci-bridge device



On 04/03/2013 11:50 AM, Ján Tomko wrote:
> From: liguang <lig fnst cn fujitsu com>
>
> ---
>  src/qemu/qemu_capabilities.c |  2 ++
>  src/qemu/qemu_capabilities.h |  1 +
>  src/qemu/qemu_command.c      | 15 ++++++++++++++-
>  tests/qemuhelptest.c         | 21 ++++++++++++++-------
>  4 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index aa381b4..4377e08 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -216,6 +216,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>  
>                "ipv6-migration", /* 135 */
>                "machine-opt",
> +              "pci-bridge",
>      );
>  
>  struct _virQEMUCaps {
> @@ -1357,6 +1358,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 b2dc588..e3bba52 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -176,6 +176,7 @@ enum virQEMUCapsFlags {
>      QEMU_CAPS_SCSI_MEGASAS       = 134, /* -device megasas */
>      QEMU_CAPS_IPV6_MIGRATION     = 135, /* -incoming [::] */
>      QEMU_CAPS_MACHINE_OPT        = 136, /* -machine xxxx*/
> +    QEMU_CAPS_DEVICE_PCI_BRIDGE  = 137, /* -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 e221c82..7817b13 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1972,7 +1972,9 @@ 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))
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE))
> +            virBufferAsprintf(buf, ",bus=pci.%u", info->addr.pci.bus);
> +        else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS))
>              virBufferAsprintf(buf, ",bus=pci.0");
>          else
>              virBufferAsprintf(buf, ",bus=pci");


The above looks like it would just cover up a configuration problem - if
info->addr.pci.bus is > 0, then QEMU_CAPS_DEVICE_PCI_BRIDGE had better
be true, otherwise it's an error in the config (and I assume that if
DEVICE_PCI_BRIDGE is true, then soe is PCI_MULTIBUS, correct?). In this
case we shouldn't just silently use bus=pci.0 or bus=pci, we should log
an error:

        if (virQEMUCapsGet(qemuCaps, QEMUCAPS_DEVICE_PCI_BRIDGE)) {
            virBufferAsprintf(buf, ",bus=pci.%u", info->addr.pci.bus);
        } else {
            if (info->addr.pci.bus != 0) {
                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                               _("Only PCI device addresses with bus=0 "
                                 "are supported with this QEMU binary"));
                return -1;
            }
            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS))
                virBufferAddLit(buf, ",bus=pci.0");
            else
                virBufferAddLit(buf, ",bus=pci");

        }


> @@ -3576,6 +3578,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>  
>          break;
>  
> +    case VIR_DOMAIN_CONTROLLER_TYPE_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;
> +

Depending on what others think about my comments in 4/7, when we see a
bridge with index='0', instead of erroring out, we may want to:

  1) ignore it if machinetype is pc-*
  2) generate a commandline for an i82801b11-bridge device attached to
pcie.0 if machinetype is q35-*


>      /* We always get an IDE controller, whether we want it or not. */
>      case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
>      default:
> @@ -5674,6 +5686,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. */
> +        VIR_DOMAIN_CONTROLLER_TYPE_PCI_BRIDGE,
>          VIR_DOMAIN_CONTROLLER_TYPE_USB,
>          VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
>          VIR_DOMAIN_CONTROLLER_TYPE_SATA,
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index 43774f4..f0181ce 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -397,7 +397,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,
> @@ -506,7 +507,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,
> @@ -571,7 +573,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,
> @@ -643,7 +646,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,
> @@ -815,7 +819,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,
> @@ -918,7 +923,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,
> @@ -1026,7 +1032,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;
>  }

Otherwise seems fine.


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