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

Re: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"



On 11/29/2011 10:46 PM, Stefan Berger wrote:
On 11/29/2011 10:03 AM, Prerna Saxena wrote:
From: Michael Ellerman<michael 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 ellerman id au>
Signed-off-by: Prerna Saxena<prerna 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.

Will do.

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.


--
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India


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