[libvirt] [PATCHv2 5/7] qemu: properly set/use device alias for pci controllers

Doug Goldstein cardoe at gentoo.org
Sun Aug 4 23:53:15 UTC 2013


On Sat, Aug 3, 2013 at 9:01 PM, Laine Stump <laine at laine.org> wrote:
> We had been setting the device alias in the devinceinfo for pci
> controllers to "pci%u", but then hardcoding "pci.%u" when creating the
> device address for other devices using that pci bus. This all worked
> just fine until we encountered the built-in "pcie.0" bus (the PCIe
> root complex) in Q35 machines.
>
> In order to create the correct commandline for this one case, this
> patch:
>
> 1) sets the alias for PCI controllers correctly, to "pci.%u" (or
> "pcie.%u" for the pcie-root controller)
>
> 2) eliminates the hardcoded "pci.%u" for pci controllers when
> generatuing device address strings, and instead uses the controller's
> alias.
>
> 3) plumbs a pointer to the virDomainDef all the way down to
> qemuBuildDeviceAddressStr. This was necessary in order to make the
> aliase of the controller *used by a device* available (previously
> qemuBuildDeviceAddressStr only had the deviceinfo of the device
> itself, *not* of the controller it was connecting to). This made for a
> larger than desired diff, but at least in the future we won't have to
> do it again, since all the information we could possibly ever need for
> future enhancements is in the virDomainDef. (right?)
>
> This should be done for *all* controllers, but for now we just do it
> in the case of PCI controllers, to reduce the likelyhood of
> regression.
> ---
>  src/qemu/qemu_command.c                            | 164 ++++++++++++++-------
>  src/qemu/qemu_command.h                            |  28 ++--
>  src/qemu/qemu_hotplug.c                            |   6 +-
>  tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args |   2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-q35.args       |   2 +-
>  5 files changed, 136 insertions(+), 66 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a6d6819..3353c61 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -860,10 +860,18 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller)
>  {
>      const char *prefix = virDomainControllerTypeToString(controller->type);
>
> -    if (virAsprintf(&controller->info.alias, "%s%d", prefix,
> -                    controller->idx) < 0)
> -        return -1;
> -    return 0;
> +    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
> +        /* only pcie-root uses a different naming convention
> +         * ("pcie.0"), because it is hardcoded that way in qemu. All
> +         * other buses use the consistent "pci.%u".
> +         */
> +        if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
> +            return virAsprintf(&controller->info.alias, "pcie.%d", controller->idx);
> +        else
> +            return virAsprintf(&controller->info.alias, "pci.%d", controller->idx);
> +    }
> +
> +    return virAsprintf(&controller->info.alias, "%s%d", prefix, controller->idx);
>  }
>
>  static ssize_t
> @@ -2781,22 +2789,57 @@ qemuUsbId(virBufferPtr buf, int idx)
>
>  static int
>  qemuBuildDeviceAddressStr(virBufferPtr buf,
> +                          virDomainDefPtr domainDef,
>                            virDomainDeviceInfoPtr info,
>                            virQEMUCapsPtr qemuCaps)
>  {
> +    int ret = -1;
> +    char *devStr = NULL;
> +
>      if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +        const char *contAlias = NULL;
> +        size_t i;
> +
> +        if (!(devStr = qemuDomainPCIAddressAsString(&info->addr.pci)))
> +            goto cleanup;
> +        for (i = 0; i < domainDef->ncontrollers; i++) {
> +            virDomainControllerDefPtr cont = domainDef->controllers[i];
> +
> +            if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
> +                cont->idx == info->addr.pci.bus) {
> +                contAlias = cont->info.alias;
> +                if (!contAlias) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("Device alias was not set for PCI "
> +                                     "controller with index %u required "
> +                                     "for device at address %s"),
> +                                   info->addr.pci.bus, devStr);
> +                    goto cleanup;
> +                }
> +                break;
> +            }
> +        }
> +        if (!contAlias) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not find PCI "
> +                             "controller with index %u required "
> +                             "for device at address %s"),
> +                           info->addr.pci.bus, devStr);
> +            goto cleanup;
> +        }
> +
>          if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) {
>              if (info->addr.pci.function != 0) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("Only PCI device addresses with function=0 "
>                                   "are supported with this QEMU binary"));
> -                return -1;
> +                goto cleanup;
>              }
>              if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("'multifunction=on' is not supported with "
>                                   "this QEMU binary"));
> -                return -1;
> +                goto cleanup;
>              }
>          }
>
> @@ -2810,18 +2853,19 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>           */
>          if (info->addr.pci.bus != 0) {
>              if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
> -                virBufferAsprintf(buf, ",bus=pci.%u", info->addr.pci.bus);
> +                virBufferAsprintf(buf, ",bus=%s", contAlias);
>              } else {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("Multiple PCI buses are not supported "
>                                   "with this QEMU binary"));
> -                return -1;
> +                goto cleanup;
>              }
>          } else {
> -            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS))
> -                virBufferAddLit(buf, ",bus=pci.0");
> -            else
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS)) {
> +                virBufferAsprintf(buf, ",bus=%s", contAlias);
> +            } else {
>                  virBufferAddLit(buf, ",bus=pci");
> +            }
>          }
>          if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
>              virBufferAddLit(buf, ",multifunction=on");
> @@ -2845,7 +2889,10 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>                                info->addr.ccw.devno);
>      }
>
> -    return 0;
> +    ret = 0;
> +cleanup:
> +    VIR_FREE(devStr);
> +    return ret;
>  }
>
>  static int
> @@ -4179,13 +4226,13 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>                                (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN)
>                                ? "on" : "off");
>          }
> -        if (qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCaps) < 0)
> +        if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0)
>              goto error;
>          break;
>      case VIR_DOMAIN_DISK_BUS_USB:
>          virBufferAddLit(&opt, "usb-storage");
>
> -        if (qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCaps) < 0)
> +        if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0)
>              goto error;
>          break;
>      default:
> @@ -4309,7 +4356,8 @@ error:
>
>
>  char *
> -qemuBuildFSDevStr(virDomainFSDefPtr fs,
> +qemuBuildFSDevStr(virDomainDefPtr def,
> +                  virDomainFSDefPtr fs,
>                    virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer opt = VIR_BUFFER_INITIALIZER;
> @@ -4325,7 +4373,7 @@ qemuBuildFSDevStr(virDomainFSDefPtr fs,
>      virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
>      virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst);
>
> -    if (qemuBuildDeviceAddressStr(&opt, &fs->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&opt)) {
> @@ -4545,7 +4593,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>      if (def->queues)
>          virBufferAsprintf(&buf, ",num_queues=%u", def->queues);
>
> -    if (qemuBuildDeviceAddressStr(&buf, &def->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, domainDef, &def->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -4583,7 +4631,8 @@ qemuBuildNicStr(virDomainNetDefPtr net,
>
>
>  char *
> -qemuBuildNicDevStr(virDomainNetDefPtr net,
> +qemuBuildNicDevStr(virDomainDefPtr def,
> +                   virDomainNetDefPtr net,
>                     int vlan,
>                     int bootindex,
>                     virQEMUCapsPtr qemuCaps)
> @@ -4645,7 +4694,7 @@ qemuBuildNicDevStr(virDomainNetDefPtr net,
>      virBufferAsprintf(&buf, ",id=%s", net->info.alias);
>      virBufferAsprintf(&buf, ",mac=%s",
>                        virMacAddrFormat(&net->mac, macaddr));
> -    if (qemuBuildDeviceAddressStr(&buf, &net->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &net->info, qemuCaps) < 0)
>          goto error;
>      if (qemuBuildRomStr(&buf, &net->info, qemuCaps) < 0)
>         goto error;
> @@ -4800,7 +4849,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>
>
>  char *
> -qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev,
> +qemuBuildWatchdogDevStr(virDomainDefPtr def,
> +                        virDomainWatchdogDefPtr dev,
>                          virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -4813,7 +4863,7 @@ qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev,
>      }
>
>      virBufferAsprintf(&buf, "%s,id=%s", model, dev->info.alias);
> -    if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -4830,7 +4880,8 @@ error:
>
>
>  char *
> -qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev,
> +qemuBuildMemballoonDevStr(virDomainDefPtr def,
> +                          virDomainMemballoonDefPtr dev,
>                            virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -4850,7 +4901,7 @@ qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev,
>      }
>
>      virBufferAsprintf(&buf, ",id=%s", dev->info.alias);
> -    if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -4893,7 +4944,8 @@ error:
>  }
>
>  char *
> -qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
> +qemuBuildUSBInputDevStr(virDomainDefPtr def,
> +                        virDomainInputDefPtr dev,
>                          virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -4902,7 +4954,7 @@ qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
>                        dev->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ?
>                        "usb-mouse" : "usb-tablet", dev->info.alias);
>
> -    if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -4919,7 +4971,8 @@ error:
>
>
>  char *
> -qemuBuildSoundDevStr(virDomainSoundDefPtr sound,
> +qemuBuildSoundDevStr(virDomainDefPtr def,
> +                     virDomainSoundDefPtr sound,
>                       virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -4940,7 +4993,7 @@ qemuBuildSoundDevStr(virDomainSoundDefPtr sound,
>          model = "intel-hda";
>
>      virBufferAsprintf(&buf, "%s,id=%s", model, sound->info.alias);
> -    if (qemuBuildDeviceAddressStr(&buf, &sound->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &sound->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -5000,7 +5053,8 @@ error:
>  }
>
>  static char *
> -qemuBuildDeviceVideoStr(virDomainVideoDefPtr video,
> +qemuBuildDeviceVideoStr(virDomainDefPtr def,
> +                        virDomainVideoDefPtr video,
>                          virQEMUCapsPtr qemuCaps,
>                          bool primary)
>  {
> @@ -5054,7 +5108,7 @@ qemuBuildDeviceVideoStr(virDomainVideoDefPtr video,
>          virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024);
>      }
>
> -    if (qemuBuildDeviceAddressStr(&buf, &video->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -5094,7 +5148,9 @@ qemuOpenPCIConfig(virDomainHostdevDefPtr dev)
>  }
>
>  char *
> -qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd,
> +qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
> +                          virDomainHostdevDefPtr dev,
> +                          const char *configfd,
>                            virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -5114,7 +5170,7 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd,
>      virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
>      if (dev->info->bootIndex)
>          virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex);
> -    if (qemuBuildDeviceAddressStr(&buf, dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)
>          goto error;
>      if (qemuBuildRomStr(&buf, dev->info, qemuCaps) < 0)
>         goto error;
> @@ -5220,7 +5276,7 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def,
>          virBufferAsprintf(&buf, ",bootindex=%d", dev->info.bootIndex);
>      }
>
> -    if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -5236,7 +5292,8 @@ error:
>  }
>
>  char *
> -qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
> +qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
> +                          virDomainHostdevDefPtr dev,
>                            virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -5259,7 +5316,7 @@ qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
>      if (dev->info->bootIndex)
>          virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex);
>
> -    if (qemuBuildDeviceAddressStr(&buf, dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -5276,7 +5333,8 @@ error:
>
>
>  char *
> -qemuBuildHubDevStr(virDomainHubDefPtr dev,
> +qemuBuildHubDevStr(virDomainDefPtr def,
> +                   virDomainHubDefPtr dev,
>                     virQEMUCapsPtr qemuCaps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -5296,7 +5354,7 @@ qemuBuildHubDevStr(virDomainHubDefPtr dev,
>
>      virBufferAddLit(&buf, "usb-hub");
>      virBufferAsprintf(&buf, ",id=%s", dev->info.alias);
> -    if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
>          goto error;
>
>      if (virBufferError(&buf)) {
> @@ -5817,6 +5875,7 @@ cleanup:
>
>  static int
>  qemuBuildRNGDeviceArgs(virCommandPtr cmd,
> +                       virDomainDefPtr def,
>                         virDomainRNGDefPtr dev,
>                         virQEMUCapsPtr qemuCaps)
>  {
> @@ -5846,7 +5905,7 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd,
>              virBufferAddLit(&buf, ",period=1000");
>      }
>
> -    if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
> +    if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
>          goto cleanup;
>
>      virCommandAddArg(cmd, "-device");
> @@ -7121,7 +7180,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>          virCommandAddArgList(cmd, "-netdev", host, NULL);
>      }
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> -        if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps)))
> +        if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex, qemuCaps)))
>              goto cleanup;
>          virCommandAddArgList(cmd, "-device", nic, NULL);
>      } else {
> @@ -7869,7 +7928,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>          char *optstr;
>
>          virCommandAddArg(cmd, "-device");
> -        if (!(optstr = qemuBuildHubDevStr(hub, qemuCaps)))
> +        if (!(optstr = qemuBuildHubDevStr(def, hub, qemuCaps)))
>              goto error;
>          virCommandAddArg(cmd, optstr);
>          VIR_FREE(optstr);
> @@ -8090,7 +8149,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>              VIR_FREE(optstr);
>
>              virCommandAddArg(cmd, "-device");
> -            if (!(optstr = qemuBuildFSDevStr(fs, qemuCaps)))
> +            if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps)))
>                  goto error;
>              virCommandAddArg(cmd, optstr);
>              VIR_FREE(optstr);
> @@ -8448,7 +8507,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>              if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>                  char *optstr;
>                  virCommandAddArg(cmd, "-device");
> -                if (!(optstr = qemuBuildUSBInputDevStr(input, qemuCaps)))
> +                if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps)))
>                      goto error;
>                  virCommandAddArg(cmd, optstr);
>                  VIR_FREE(optstr);
> @@ -8505,7 +8564,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>              for (i = 0; i < def->nvideos; i++) {
>                  char *str;
>                  virCommandAddArg(cmd, "-device");
> -                if (!(str = qemuBuildDeviceVideoStr(def->videos[i], qemuCaps, !i)))
> +                if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], qemuCaps, !i)))
>                      goto error;
>
>                  virCommandAddArg(cmd, str);
> @@ -8579,7 +8638,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>
>                          virCommandAddArg(cmd, "-device");
>
> -                        if (!(str = qemuBuildDeviceVideoStr(def->videos[i], qemuCaps, false)))
> +                        if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], qemuCaps, false)))
>                              goto error;
>
>                          virCommandAddArg(cmd, str);
> @@ -8643,7 +8702,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>                      virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
>                  } else {
>                      virCommandAddArg(cmd, "-device");
> -                    if (!(str = qemuBuildSoundDevStr(sound, qemuCaps)))
> +                    if (!(str = qemuBuildSoundDevStr(def, sound, qemuCaps)))
>                          goto error;
>
>                      virCommandAddArg(cmd, str);
> @@ -8719,7 +8778,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>          if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>              virCommandAddArg(cmd, "-device");
>
> -            optstr = qemuBuildWatchdogDevStr(watchdog, qemuCaps);
> +            optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps);
>              if (!optstr)
>                  goto error;
>          } else {
> @@ -8832,7 +8891,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>
>              if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>                  virCommandAddArg(cmd, "-device");
> -                if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev, qemuCaps)))
> +                if (!(devstr = qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps)))
>                      goto error;
>                  virCommandAddArg(cmd, devstr);
>                  VIR_FREE(devstr);
> @@ -8880,7 +8939,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>                      }
>                  }
>                  virCommandAddArg(cmd, "-device");
> -                devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name, qemuCaps);
> +                devstr = qemuBuildPCIHostdevDevStr(def, hostdev, configfd_name, qemuCaps);
>                  VIR_FREE(configfd_name);
>                  if (!devstr)
>                      goto error;
> @@ -9009,7 +9068,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>              char *optstr;
>              virCommandAddArg(cmd, "-device");
>
> -            optstr = qemuBuildMemballoonDevStr(def->memballoon, qemuCaps);
> +            optstr = qemuBuildMemballoonDevStr(def, def->memballoon, qemuCaps);
>              if (!optstr)
>                  goto error;
>              virCommandAddArg(cmd, optstr);
> @@ -9025,7 +9084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>              goto error;
>
>          /* add the device */
> -        if (qemuBuildRNGDeviceArgs(cmd, def->rng, qemuCaps) < 0)
> +        if (qemuBuildRNGDeviceArgs(cmd, def, def->rng, qemuCaps) < 0)
>              goto error;
>      }
>
> @@ -9103,6 +9162,7 @@ error:
>   */
>  static int
>  qemuBuildSerialChrDeviceStr(char **deviceStr,
> +                            virDomainDefPtr def,
>                              virDomainChrDefPtr serial,
>                              virQEMUCapsPtr qemuCaps,
>                              virArch arch,
> @@ -9115,7 +9175,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>              serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
>              virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s",
>                                serial->info.alias);
> -            if (qemuBuildDeviceAddressStr(&cmd, &serial->info, qemuCaps) < 0)
> +            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
>                  goto error;
>          }
>      } else {
> @@ -9137,7 +9197,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>                  goto error;
>              }
>
> -            if (qemuBuildDeviceAddressStr(&cmd, &serial->info, qemuCaps) < 0)
> +            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
>                  goto error;
>          }
>      }
> @@ -9252,7 +9312,7 @@ qemuBuildChrDeviceStr(char **deviceStr,
>
>      switch ((enum virDomainChrDeviceType) chr->deviceType) {
>      case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
> -        ret = qemuBuildSerialChrDeviceStr(deviceStr, chr, qemuCaps,
> +        ret = qemuBuildSerialChrDeviceStr(deviceStr, vmdef, chr, qemuCaps,
>                                            vmdef->os.arch,
>                                            vmdef->os.machine);
>          break;
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index e5111d2..5c5c025 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -98,7 +98,8 @@ char * qemuBuildNicStr(virDomainNetDefPtr net,
>                         int vlan);
>
>  /* Current, best practice */
> -char * qemuBuildNicDevStr(virDomainNetDefPtr net,
> +char * qemuBuildNicDevStr(virDomainDefPtr def,
> +                          virDomainNetDefPtr net,
>                            int vlan,
>                            int bootindex,
>                            virQEMUCapsPtr qemuCaps);
> @@ -119,7 +120,8 @@ char * qemuBuildDriveDevStr(virDomainDefPtr def,
>                              virDomainDiskDefPtr disk,
>                              int bootindex,
>                              virQEMUCapsPtr qemuCaps);
> -char * qemuBuildFSDevStr(virDomainFSDefPtr fs,
> +char * qemuBuildFSDevStr(virDomainDefPtr domainDef,
> +                         virDomainFSDefPtr fs,
>                           virQEMUCapsPtr qemuCaps);
>  /* Current, best practice */
>  char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> @@ -127,22 +129,27 @@ char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>                                   virQEMUCapsPtr qemuCaps,
>                                   int *nusbcontroller);
>
> -char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev,
> +char * qemuBuildWatchdogDevStr(virDomainDefPtr domainDef,
> +                               virDomainWatchdogDefPtr dev,
>                                 virQEMUCapsPtr qemuCaps);
>
> -char * qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev,
> +char * qemuBuildMemballoonDevStr(virDomainDefPtr domainDef,
> +                                 virDomainMemballoonDefPtr dev,
>                                   virQEMUCapsPtr qemuCaps);
>
> -char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
> +char * qemuBuildUSBInputDevStr(virDomainDefPtr domainDef,
> +                               virDomainInputDefPtr dev,
>                                 virQEMUCapsPtr qemuCaps);
>
> -char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound,
> +char * qemuBuildSoundDevStr(virDomainDefPtr domainDef,
> +                            virDomainSoundDefPtr sound,
>                              virQEMUCapsPtr qemuCaps);
>
>  /* Legacy, pre device support */
>  char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev);
>  /* Current, best practice */
> -char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev,
> +char * qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
> +                                 virDomainHostdevDefPtr dev,
>                                   const char *configfd,
>                                   virQEMUCapsPtr qemuCaps);
>
> @@ -151,7 +158,8 @@ int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
>  /* Legacy, pre device support */
>  char * qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev);
>  /* Current, best practice */
> -char * qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
> +char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
> +                                 virDomainHostdevDefPtr dev,
>                                   virQEMUCapsPtr qemuCaps);
>
>  char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
> @@ -162,7 +170,9 @@ char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def,
>                                    virDomainHostdevDefPtr dev,
>                                    virQEMUCapsPtr qemuCaps);
>
> -char * qemuBuildHubDevStr(virDomainHubDefPtr dev, virQEMUCapsPtr qemuCaps);
> +char * qemuBuildHubDevStr(virDomainDefPtr def,
> +                          virDomainHubDefPtr dev,
> +                          virQEMUCapsPtr qemuCaps);
>  char * qemuBuildRedirdevDevStr(virDomainDefPtr def,
>                                 virDomainRedirdevDefPtr dev,
>                                 virQEMUCapsPtr qemuCaps);
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 032de69..7a6946e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -859,7 +859,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>      }
>
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> -        if (!(nicstr = qemuBuildNicDevStr(net, vlan, 0, priv->qemuCaps)))
> +        if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0, priv->qemuCaps)))
>              goto try_remove;
>      } else {
>          if (!(nicstr = qemuBuildNicStr(net, NULL, vlan)))
> @@ -1057,7 +1057,7 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver,
>              goto error;
>          }
>
> -        if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name,
> +        if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, configfd_name,
>                                                   priv->qemuCaps)))
>              goto error;
>
> @@ -1302,7 +1302,7 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>          if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0)
>              goto cleanup;
> -        if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev, priv->qemuCaps)))
> +        if (!(devstr = qemuBuildUSBHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
>              goto cleanup;
>      }
>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> index cecef7b..84428f9 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> @@ -1,5 +1,5 @@
>  LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/libexec/qemu-kvm \
>  -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
>  -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
> --device i82801b11-bridge,id=pci.1,bus=pci.0,addr=0x2 \
> +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \
>  -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
> index 6c24407..5ff4bc7 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
> @@ -1,6 +1,6 @@
>  LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
>  /usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
>  -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
> --device i82801b11-bridge,id=pci.1,bus=pci.0,addr=0x2 \
> +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \
>  -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
>  -vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368
> --
> 1.7.11.7
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Straight forward so ACK. This could probably be re-ordered before 4/7
if you want to merge in the ACK'd patches.

-- 
Doug Goldstein




More information about the libvir-list mailing list