[libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Nov 29 17:16:50 UTC 2011
On 11/29/2011 10:03 AM, Prerna Saxena wrote:
> From: Michael Ellerman<michael at ellerman.id.au>
> Date: Tue, 29 Nov 2011 13:48:09 +0530
> Subject: [PATCH 5/5] Add address-type "spapr-vio" for devices based on
> the same bus.
>
> For QEMU PPC64 we have a machine type ("pseries") which
> has a virtual bus called "spapr-vio". We need to be
> able to create devices on this bus, and as such need a
> way to specify the address for those devices.
>
> This patch adds a new address type "spapr-vio", which achieves this.
>
> The addressing is specified with a "reg" property in the address
> definition. The reg is optional, if it is not specified QEMU will
> auto-assign an address for the device.
>
> Additionally, this patch forces an "spapr-vscsi" controller to be
> selected for a guest running on architecture 'ppc64' and platform
> 'pseries'.
>
> Signed-off-by: Michael Ellerman<michael at ellerman.id.au>
> Signed-off-by: Prerna Saxena<prerna at linux.vnet.ibm.com>
> ---
> src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 9 +++++++++
> src/qemu/qemu_command.c | 18 ++++++++++++++----
> src/qemu/qemu_command.h | 3 ++-
> src/qemu/qemu_hotplug.c | 2 +-
> 5 files changed, 67 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3ea99f7..5c3809e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
> "drive",
> "virtio-serial",
> "ccid",
> - "usb")
> + "usb",
> + "spapr-vio")
>
> VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti,
> VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST,
> @@ -1844,6 +1845,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
> info->addr.usb.port);
> break;
>
> + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
> + if (info->addr.spaprvio.has_reg)
> + virBufferAsprintf(buf, " reg='%#llx'", info->addr.spaprvio.reg);
> + break;
> +
> default:
> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> _("unknown address type '%d'"), info->type);
> @@ -2103,6 +2109,34 @@ cleanup:
> }
>
> static int
> +virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node,
> + virDomainDeviceSpaprVioAddressPtr addr)
> +{
> + char *reg;
> + int ret;
> +
> + memset(addr, 0, sizeof(*addr));
> + addr->has_reg = false;
> +
> + reg = virXMLPropString(node, "reg");
> + if (reg) {
> + if (virStrToLong_ull(reg, NULL, 16,&addr->reg)< 0) {
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cannot parse<address> 'reg' attribute"));
> + ret = -1;
> + goto cleanup;
> + }
> +
> + addr->has_reg = true;
> + }
> +
> + ret = 0;
> +cleanup:
> + VIR_FREE(reg);
> + return ret;
> +}
> +
> +static int
> virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
> virDomainDeviceUSBMasterPtr master)
> {
> @@ -2215,6 +2249,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
> goto cleanup;
> break;
>
> + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
> + if (virDomainDeviceSpaprVioAddressParseXML(address,&info->addr.spaprvio)< 0)
> + goto cleanup;
> + break;
> +
> default:
> /* Should not happen */
> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -3048,6 +3087,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> }
>
> if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE&&
> + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO&&
> def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Controllers must use the 'pci' address type"));
Is this still the proper error message for this type of bus? You may
want to look at def->os.arch / def->os.machine to see what is expected
here in terms of bus and then have a more specific error message.
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4439f55..b1f9260 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -69,6 +69,7 @@ enum virDomainDeviceAddressType {
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL,
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID,
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB,
> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO,
>
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
> };
> @@ -121,6 +122,13 @@ struct _virDomainDeviceUSBAddress {
> char *port;
> };
>
> +typedef struct _virDomainDeviceSpaprVioAddress virDomainDeviceSpaprVioAddress;
> +typedef virDomainDeviceSpaprVioAddress *virDomainDeviceSpaprVioAddressPtr;
> +struct _virDomainDeviceSpaprVioAddress {
> + unsigned long long reg;
> + bool has_reg;
> +};
> +
> enum virDomainControllerMaster {
> VIR_DOMAIN_CONTROLLER_MASTER_NONE,
> VIR_DOMAIN_CONTROLLER_MASTER_USB,
> @@ -145,6 +153,7 @@ struct _virDomainDeviceInfo {
> virDomainDeviceVirtioSerialAddress vioserial;
> virDomainDeviceCcidAddress ccid;
> virDomainDeviceUSBAddress usb;
> + virDomainDeviceSpaprVioAddress spaprvio;
> } addr;
> int mastertype;
> union {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 57b25d6..0d03fbc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1253,6 +1253,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
> def->controllers[i]->idx == 0)
> continue;
>
> + if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
> + continue;
> if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> continue;
> if (qemuDomainPCIAddressSetNextAddr(addrs,&def->controllers[i]->info)< 0)
> @@ -1403,6 +1405,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
> virBufferAsprintf(buf, ",bus=");
> qemuUsbId(buf, info->addr.usb.bus);
> virBufferAsprintf(buf, ".0,port=%s", info->addr.usb.port);
> + } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
> + if (info->addr.spaprvio.has_reg)
> + virBufferAsprintf(buf, ",reg=%#llx", info->addr.spaprvio.reg);
> }
>
> return 0;
> @@ -2087,7 +2092,8 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
> }
>
> char *
> -qemuBuildControllerDevStr(virDomainControllerDefPtr def,
> +qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> + virDomainControllerDefPtr def,
> virBitmapPtr qemuCaps,
> int *nusbcontroller)
> {
> @@ -2095,7 +2101,11 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def,
>
> switch (def->type) {
> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> - virBufferAddLit(&buf, "lsi");
> + if (STREQ(domainDef->os.arch, "ppc64")&& STREQ(domainDef->os.machine, "pseries")) {
> + virBufferAddLit(&buf, "spapr-vscsi");
> + } else {
> + virBufferAddLit(&buf, "lsi");
> + }
> virBufferAsprintf(&buf, ",id=scsi%d", def->idx);
> break;
>
> @@ -4009,7 +4019,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> char *devstr;
>
> virCommandAddArg(cmd, "-device");
> - if (!(devstr = qemuBuildControllerDevStr(cont, qemuCaps, NULL)))
> + if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps, NULL)))
> goto error;
>
> virCommandAddArg(cmd, devstr);
> @@ -4028,7 +4038,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> virCommandAddArg(cmd, "-device");
>
> char *devstr;
> - if (!(devstr = qemuBuildControllerDevStr(def->controllers[i], qemuCaps,
> + if (!(devstr = qemuBuildControllerDevStr(def, def->controllers[i], qemuCaps,
> &usbcontroller)))
> goto error;
>
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 1fe0394..3978b2b 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -95,7 +95,8 @@ char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk,
> char * qemuBuildFSDevStr(virDomainFSDefPtr fs,
> virBitmapPtr qemuCaps);
> /* Current, best practice */
> -char * qemuBuildControllerDevStr(virDomainControllerDefPtr def,
> +char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> + virDomainControllerDefPtr def,
> virBitmapPtr qemuCaps,
> int *nusbcontroller);
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 96c0070..eabfeaa 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -329,7 +329,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
> goto cleanup;
> }
>
> - if (!(devstr = qemuBuildControllerDevStr(controller, priv->qemuCaps, NULL))) {
> + if (!(devstr = qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, NULL))) {
> goto cleanup;
> }
> }
Otherwise looks good to me.
More information about the libvir-list
mailing list