diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9b3f9ee269..e737e39d38 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3041,8 +3041,11 @@
attribute index which is the decimal integer
describing in which order the bus controller is encountered (for
use in controller attributes of
- <address> elements). Some controller types
- have additional attributes that control specific features, such as:
+ <address> elements).
+ Since 1.3.5 the index is optional; if
+ not specified, it will be auto-assigned to be the lowest unused
+ index for the given controller type. Some controller types have
+ additional attributes that control specific features, such as:
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index bbe676147e..d14c1c5ecf 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1704,9 +1704,11 @@
-
-
-
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 50d1d94935..568c699888 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1647,9 +1647,11 @@ virDomainControllerDefNew(virDomainControllerType type)
return NULL;
def->type = type;
- def->model = -1;
/* initialize anything that has a non-0 default */
+ def->model = -1;
+ def->idx = -1;
+
switch ((virDomainControllerType) def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
def->opts.vioserial.ports = -1;
@@ -3585,7 +3587,7 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr def)
for (i = 0; i < def->ncontrollers; i++) {
cont = def->controllers[i];
- if ((int) cont->idx > max_idx[cont->type])
+ if (cont->idx > max_idx[cont->type])
max_idx[cont->type] = cont->idx;
}
@@ -4254,6 +4256,84 @@ virDomainDefRemoveOfflineVcpuPin(virDomainDefPtr def)
}
+static void
+virDomainAssignControllerIndexes(virDomainDefPtr def)
+{
+ /* the index attribute of a controller is optional in the XML, but
+ * is required to be valid at any time after parse. When no index
+ * is provided for a controller, assign one automatically by
+ * looking at what indexes are already used for that controller
+ * type in the domain - the unindexed controller gets the lowest
+ * unused index.
+ */
+ size_t outer;
+
+ for (outer = 0; outer < def->ncontrollers; outer++) {
+ virDomainControllerDefPtr cont = def->controllers[outer];
+ virDomainControllerDefPtr prev = NULL;
+ size_t inner;
+
+ if (cont->idx != -1)
+ continue;
+
+ if (outer > 0 && IS_USB2_CONTROLLER(cont)) {
+ /* USB2 controllers are the only exception to the simple
+ * "assign the lowest unused index". A group of USB2
+ * "companions" should all be at the same index as other
+ * USB2 controllers in the group, but only do this
+ * automatically if it appears to be the intent. To prove
+ * intent: the USB controller on the list just prior to
+ * this one must also be a USB2 controller, and there must
+ * not yet be a controller with the exact same model of
+ * this one and the same index as the previously added
+ * controller (e.g., if this controller is a UHCI1, then
+ * the previous controller must be an EHCI1 or a UHCI[23],
+ * and there must not already be a UHCI1 controller with
+ * the same index as the previous controller). If all of
+ * these are satisfied, set this controller to the same
+ * index as the previous controller.
+ */
+ int prevIdx;
+
+ prevIdx = outer - 1;
+ while (prevIdx >= 0 &&
+ def->controllers[prevIdx]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
+ prevIdx--;
+ if (prevIdx >= 0)
+ prev = def->controllers[prevIdx];
+ /* if the last USB controller isn't USB2, that breaks
+ * the chain, so we need a new index for this new
+ * controller
+ */
+ if (prev && !IS_USB2_CONTROLLER(prev))
+ prev = NULL;
+
+ /* if prev != NULL, we've found a potential index to
+ * use. Make sure this index isn't already used by an
+ * existing USB2 controller of the same model as the new
+ * one.
+ */
+ for (inner = 0; prev && inner < def->ncontrollers; inner++) {
+ if (def->controllers[inner]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ def->controllers[inner]->idx == prev->idx &&
+ def->controllers[inner]->model == cont->model) {
+ /* we already have a controller of this model with
+ * the proposed index, so we need to move to a new
+ * index for this controller
+ */
+ prev = NULL;
+ }
+ }
+ if (prev)
+ cont->idx = prev->idx;
+ }
+ /* if none of the above applied, prev will be NULL */
+ if (!prev)
+ cont->idx = virDomainControllerFindUnusedIndex(def, cont->type);
+ }
+}
+
+
#define UNSUPPORTED(FEATURE) (!((FEATURE) & xmlopt->config.features))
/**
* virDomainDefPostParseCheckFeatures:
@@ -4422,6 +4502,11 @@ virDomainDefPostParse(virDomainDefPtr def,
.parseFlags = parseFlags,
};
+ /* this must be done before the hypervisor-specific callback,
+ * in case presence of a controller at a specific index is checked
+ */
+ virDomainAssignControllerIndexes(def);
+
/* call the domain config callback */
if (xmlopt->config.domainPostParseCallback) {
ret = xmlopt->config.domainPostParseCallback(def, caps, parseFlags,
@@ -7908,16 +7993,6 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (!(def = virDomainControllerDefNew(type)))
goto error;
- idx = virXMLPropString(node, "index");
- if (idx) {
- if (virStrToLong_ui(idx, NULL, 10, &def->idx) < 0 ||
- def->idx > INT_MAX) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Cannot parse controller index %s"), idx);
- goto error;
- }
- }
-
model = virXMLPropString(node, "model");
if (model) {
if ((def->model = virDomainControllerModelTypeFromString(def, model)) < 0) {
@@ -7927,6 +8002,18 @@ virDomainControllerDefParseXML(xmlNodePtr node,
}
}
+ idx = virXMLPropString(node, "index");
+ if (idx) {
+ unsigned int idxVal;
+ if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 ||
+ idxVal > INT_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse controller index %s"), idx);
+ goto error;
+ }
+ def->idx = idxVal;
+ }
+
cur = node->children;
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
@@ -8075,7 +8162,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
"have an address"));
goto error;
}
- if (def->idx != 0) {
+ if (def->idx > 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("pci-root and pcie-root controllers "
"should have index 0"));
@@ -13672,6 +13759,14 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def,
for (idx = (def->ncontrollers - 1); idx >= 0; idx--) {
current = def->controllers[idx];
if (current->type == controller->type) {
+ if (controller->idx == -1) {
+ /* If the new controller doesn't have an index set
+ * yet, put it just past this controller, which until
+ * now was the last controller of this type.
+ */
+ insertAt = idx + 1;
+ break;
+ }
if (current->idx > controller->idx) {
/* If bus matches and current controller is after
* new controller, then new controller should go here
@@ -16238,6 +16333,7 @@ virDomainDefParseXML(xmlDocPtr xml,
virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i],
ctxt,
flags);
+
if (!controller)
goto error;
@@ -19552,7 +19648,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
}
virBufferAsprintf(buf,
- "idx);
if (model)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0d97003a91..b1953b3143 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -733,7 +733,7 @@ struct _virDomainUSBControllerOpts {
/* Stores the virtual disk controller configuration */
struct _virDomainControllerDef {
int type;
- unsigned int idx;
+ int idx;
int model; /* -1 == undef */
unsigned int queues;
unsigned int cmd_per_lun;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 40ef046a93..10d3e3dfe7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1,7 +1,7 @@
/*
* qemu_driver.c: core driver methods for managing qemu guests
*
- * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -7827,7 +7827,8 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
case VIR_DOMAIN_DEVICE_CONTROLLER:
controller = dev->data.controller;
- if (virDomainControllerFind(vmdef, controller->type,
+ if (controller->idx != -1 &&
+ virDomainControllerFind(vmdef, controller->type,
controller->idx) >= 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Target already exists"));
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 41a0e4f9b8..6ce0a84cfa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -425,6 +425,14 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
return -1;
}
+ /* default idx would normally be set by virDomainDefPostParse(),
+ * which isn't called in the case of live attach of a single
+ * device.
+ */
+ if (controller->idx == -1)
+ controller->idx = virDomainControllerFindUnusedIndex(vm->def,
+ controller->type);
+
if (virDomainControllerFind(vm->def, controller->type, controller->idx) >= 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("target %s:%d already exists"),
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args b/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args
new file mode 100644
index 0000000000..bbf8f474fd
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args
@@ -0,0 +1,53 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm \
+-name q35-test \
+-S \
+-M q35 \
+-m 2048 \
+-smp 2 \
+-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
+-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \
+-device x3130-upstream,id=pci.4,bus=pci.3,addr=0x0 \
+-device xio3130-downstream,port=0x0,chassis=5,id=pci.5,bus=pci.4,addr=0x0 \
+-device xio3130-downstream,port=0x1,chassis=6,id=pci.6,bus=pci.4,addr=0x1 \
+-device xio3130-downstream,port=0x2,chassis=7,id=pci.7,bus=pci.4,addr=0x2 \
+-device xio3130-downstream,port=0x3,chassis=8,id=pci.8,bus=pci.4,addr=0x3 \
+-device xio3130-downstream,port=0x4,chassis=9,id=pci.9,bus=pci.4,addr=0x4 \
+-device xio3130-downstream,port=0x5,chassis=10,id=pci.10,bus=pci.4,addr=0x5 \
+-device xio3130-downstream,port=0x6,chassis=11,id=pci.11,bus=pci.4,addr=0x6 \
+-device xio3130-downstream,port=0x7,chassis=12,id=pci.12,bus=pci.4,addr=0x7 \
+-device xio3130-downstream,port=0x8,chassis=13,id=pci.13,bus=pci.4,addr=0x8 \
+-device xio3130-downstream,port=0x9,chassis=14,id=pci.14,bus=pci.4,addr=0x9 \
+-device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7 \
+-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,\
+addr=0x1d \
+-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 \
+-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 \
+-device ich9-usb-ehci1,id=usb1,bus=pcie.0,addr=0x1a.0x7 \
+-device ich9-usb-uhci1,masterbus=usb1.0,firstport=0,bus=pcie.0,multifunction=on,\
+addr=0x1a \
+-device ich9-usb-uhci1,masterbus=usb2.0,firstport=0,bus=pci.2,multifunction=on,\
+addr=0x1 \
+-device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.2,addr=0x1.0x1 \
+-device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.2,addr=0x1.0x2 \
+-device ich9-usb-ehci1,id=usb2,bus=pci.2,addr=0x1.0x7 \
+-device nec-usb-xhci,id=usb3,bus=pci.2,addr=0x2 \
+-device ich9-usb-uhci1,masterbus=usb4.0,firstport=0,bus=pci.2,multifunction=on,\
+addr=0x3 \
+-device ich9-usb-uhci2,masterbus=usb4.0,firstport=2,bus=pci.2,addr=0x3.0x1 \
+-device ich9-usb-uhci3,masterbus=usb4.0,firstport=4,bus=pci.2,addr=0x3.0x2 \
+-device ich9-usb-ehci1,id=usb4,bus=pci.2,addr=0x3.0x7 \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-sata0-0-0 \
+-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-autoindex.xml b/tests/qemuxml2argvdata/qemuxml2argv-autoindex.xml
new file mode 100644
index 0000000000..1fb8c372c0
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-autoindex.xml
@@ -0,0 +1,54 @@
+
+ q35-test
+ 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774
+ 2097152
+ 2097152
+ 2
+
+ hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+ /usr/libexec/qemu-kvm
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0b26026b5c..db42b7bd71 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1574,6 +1574,15 @@ mymain(void)
QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST("autoindex",
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM,
+ QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1,
+ QEMU_CAPS_NEC_USB_XHCI);
DO_TEST_PARSE_ERROR("q35-wrong-root",
QEMU_CAPS_DEVICE_PCI_BRIDGE,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml
new file mode 100644
index 0000000000..086a1cf5a4
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml
@@ -0,0 +1,153 @@
+
+ q35-test
+ 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774
+ 2097152
+ 2097152
+ 2
+
+ hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+ /usr/libexec/qemu-kvm
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e23831b3b4..e9d1f93836 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -664,7 +664,15 @@ mymain(void)
QEMU_CAPS_DEVICE_X3130_UPSTREAM,
QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM,
QEMU_CAPS_DEVICE_PXB_PCIE);
-
+ DO_TEST_FULL("autoindex", WHEN_ACTIVE, GIC_NONE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM,
+ QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1,
+ QEMU_CAPS_NEC_USB_XHCI);
DO_TEST_FULL("hostdev-scsi-lsi", WHEN_ACTIVE, GIC_NONE,
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,