On 29.06.2012 17:02, Viktor Mihajlovski wrote:
This is in preparation of the enablement of s390 guests with virtio
devices.
The assignment of device addresses happens in different places, i.e. the
qemu driver and process modules as well as in the unit tests in slightly
different flavors. Currently, these are PPC spapr-vio and PCI
devices, virtio-s390 (not PCI based) will follow.
By optionally passing to qemuDomainAssignAddresses the domain
object and the capabilities it is now possible to call the function
from most of the places (except for hotplug) where address assignment
is done.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 41 ++++++++++++++++++++++++++++++++---------
src/qemu/qemu_command.h | 6 ++++--
src/qemu/qemu_driver.c | 14 +++++++-------
src/qemu/qemu_process.c | 42 ++++--------------------------------------
4 files changed, 47 insertions(+), 56 deletions(-)
Nice cleanup.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6549f57..5edf915 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -942,16 +942,22 @@ cleanup:
int
-qemuDomainAssignPCIAddresses(virDomainDefPtr def)
+qemuDomainAssignPCIAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+ virDomainObjPtr obj)
Even though this is still less than 80 characters, I think we could have each argument per
separate line.
{
int ret = -1;
- virBitmapPtr qemuCaps = NULL;
+ virBitmapPtr localCaps = NULL;
qemuDomainPCIAddressSetPtr addrs = NULL;
+ qemuDomainObjPrivatePtr priv = NULL;
- if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch,
- NULL,
- &qemuCaps) < 0)
- goto cleanup;
+ if (!qemuCaps) {
+ /* need to get information from real environment */
+ if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch,
+ NULL,
+ &localCaps) < 0)
+ goto cleanup;
+ qemuCaps = localCaps;
+ }
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (!(addrs = qemuDomainPCIAddressSetCreate(def)))
@@ -961,16 +967,33 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def)
goto cleanup;
}
+ if (obj && obj->privateData) {
+ priv = obj->privateData;
+ if (addrs) {
+ /* if this is the live domain object, we persist the PCI addresses*/
+ if (priv->pciaddrs) {
+ qemuDomainPCIAddressSetFree(priv->pciaddrs);
+ priv->pciaddrs = NULL;
+ }
qemuDomainPCIAddressSetFree() handles passed NULL, so this check is redundant.
+ priv->persistentAddrs = 1;
+ priv->pciaddrs = addrs;
+ addrs = NULL;
+ } else {
+ priv->persistentAddrs = 0;
+ }
+ }
+
ret = 0;
cleanup:
- qemuCapsFree(qemuCaps);
+ qemuCapsFree(localCaps);
qemuDomainPCIAddressSetFree(addrs);
return ret;
}
-int qemuDomainAssignAddresses(virDomainDefPtr def)
+int qemuDomainAssignAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+ virDomainObjPtr obj)
Again. I'd prefer to have each argument on a separate line...
{
int rc;
@@ -978,7 +1001,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def)
if (rc)
return rc;
- return qemuDomainAssignPCIAddresses(def);
+ return qemuDomainAssignPCIAddresses(def, qemuCaps, obj);
}
static void
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1eafeb3..dd104d6 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -175,10 +175,12 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
virDomainChrSourceDefPtr *monConfig,
bool *monJSON);
-int qemuDomainAssignAddresses(virDomainDefPtr def);
+int qemuDomainAssignAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+ virDomainObjPtr);
int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def);
-int qemuDomainAssignPCIAddresses(virDomainDefPtr def);
+int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+ virDomainObjPtr obj);
qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def);
... and same here.
int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr
addrs,
int slot, int function);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2f93404..ef9983c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1404,7 +1404,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const
char *xml,
if (qemudCanonicalizeMachine(driver, def) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def) < 0)
+ if (qemuDomainAssignAddresses(def, NULL, NULL) < 0)
goto cleanup;
if (!(vm = virDomainAssignDef(driver->caps,
@@ -5070,7 +5070,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const
char *xml) {
if (qemudCanonicalizeMachine(driver, def) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def) < 0)
+ if (qemuDomainAssignAddresses(def, NULL, NULL) < 0)
goto cleanup;
if (!(vm = virDomainAssignDef(driver->caps,
@@ -5548,7 +5548,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
if (virDomainDefAddImplicitControllers(vmdef) < 0)
return -1;
- if (qemuDomainAssignAddresses(vmdef) < 0)
+ if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0)
return -1;
break;
@@ -5566,7 +5566,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
return -1;
}
dev->data.net = NULL;
- if (qemuDomainAssignAddresses(vmdef) < 0)
+ if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0)
return -1;
break;
@@ -5582,7 +5582,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
return -1;
}
dev->data.hostdev = NULL;
- if (qemuDomainAssignAddresses(vmdef) < 0)
+ if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0)
return -1;
break;
@@ -5736,7 +5736,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
vmdef->nets[pos] = net;
dev->data.net = NULL;
- if (qemuDomainAssignAddresses(vmdef) < 0)
+ if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0)
return -1;
break;
@@ -11734,7 +11734,7 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn,
if (qemudCanonicalizeMachine(driver, def) < 0)
goto cleanup;
- if (qemuDomainAssignAddresses(def) < 0)
+ if (qemuDomainAssignAddresses(def, NULL, NULL) < 0)
goto cleanup;
if (!(vm = virDomainAssignDef(driver->caps,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c5140c3..bf32487 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3090,13 +3090,8 @@ qemuProcessReconnect(void *opaque)
goto endjob;
}
- if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- priv->persistentAddrs = 1;
-
- if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(obj->def)) ||
- qemuAssignDevicePCISlots(obj->def, priv->pciaddrs) < 0)
- goto error;
- }
+ if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
+ qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj);
if (virSecurityManagerReserveLabel(driver->securityManager, obj->def,
obj->pid) < 0)
goto error;
@@ -3560,22 +3555,7 @@ int qemuProcessStart(virConnectPtr conn,
*/
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
VIR_DEBUG("Assigning domain PCI addresses");
- /* Populate cache with current addresses */
- if (priv->pciaddrs) {
- qemuDomainPCIAddressSetFree(priv->pciaddrs);
- priv->pciaddrs = NULL;
- }
- if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def)))
- goto cleanup;
-
-
- /* Assign any remaining addresses */
- if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0)
- goto cleanup;
-
- priv->persistentAddrs = 1;
- } else {
- priv->persistentAddrs = 0;
+ qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm);
}
VIR_DEBUG("Building emulator command line");
@@ -4257,21 +4237,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
*/
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
VIR_DEBUG("Assigning domain PCI addresses");
- /* Populate cache with current addresses */
- if (priv->pciaddrs) {
- qemuDomainPCIAddressSetFree(priv->pciaddrs);
- priv->pciaddrs = NULL;
- }
- if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def)))
- goto cleanup;
-
- /* Assign any remaining addresses */
- if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0)
- goto cleanup;
-
- priv->persistentAddrs = 1;
- } else {
- priv->persistentAddrs = 0;
+ qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm);
}
if ((timestamp = virTimeStringNow()) == NULL) {
ACK with this squashed in:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a6fc9ca..bcc2192 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -942,7 +942,8 @@ cleanup:
int
-qemuDomainAssignPCIAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+qemuDomainAssignPCIAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
virDomainObjPtr obj)
{
int ret = -1;
@@ -971,10 +972,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virBitmapPtr
qemuCaps,
priv = obj->privateData;
if (addrs) {
/* if this is the live domain object, we persist the PCI addresses*/
- if (priv->pciaddrs) {
- qemuDomainPCIAddressSetFree(priv->pciaddrs);
- priv->pciaddrs = NULL;
- }
+ qemuDomainPCIAddressSetFree(priv->pciaddrs);
priv->persistentAddrs = 1;
priv->pciaddrs = addrs;
addrs = NULL;
@@ -992,7 +990,8 @@ cleanup:
return ret;
}
-int qemuDomainAssignAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+int qemuDomainAssignAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
virDomainObjPtr obj)
{
int rc;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index dd104d6..ddf426f 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -175,11 +175,13 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
virDomainChrSourceDefPtr *monConfig,
bool *monJSON);
-int qemuDomainAssignAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+int qemuDomainAssignAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
virDomainObjPtr);
int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def);
-int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virBitmapPtr qemuCaps,
+int qemuDomainAssignPCIAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
virDomainObjPtr obj);
qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def);
int qemuDomainPCIAddressReserveFunction(qemuDomainPCIAddressSetPtr addrs,
Michal