[libvirt] [PATCH v3 0/3] add pci-bridge support

Now, it's impossible to arrange devices into multi-pci-bus, for example: <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x1' slot='0x02' function='0x0'/> </video> libvirt will complain about "bus != 0", fortunately, qemu supports pci-to-pci bridge, if we want to use multi-pci-bus, we can define 2 pci bridge controllers, then attach 1 to the other as a subordinate pci-bus, so, 2 pci-buses appear. for example: <controller type='pci-bridge' index='0'/> <controller type='pci-bridge' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x01' slot='0x02' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> src/conf/domain_conf.c | 5 ++++- src/conf/domain_conf.h | 1 + docs/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 70 ++++++++++++++++++++----- 6 files changed, 57 insertions(+), 23 deletions(-)

add a new controller type, then one can define a pci-bridge controller like this: <controller type='pci-bridge' index='0'/> <controller type='pci-bridge' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> actually, it works as a pci-bus, so as to support multi-pci-bus via pci-to-pci bridge Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/conf/domain_conf.c | 5 ++++- src/conf/domain_conf.h | 1 + 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6a7646e..8ebe77d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -264,7 +264,8 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "sata", "virtio-serial", "ccid", - "usb") + "usb", + "pci-bridge") VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", @@ -4479,6 +4480,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; switch (def->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE: + break; case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { char *ports = virXMLPropString(node, "ports"); if (ports) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5062e07..56e5a40 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -652,6 +652,7 @@ enum virDomainControllerType { VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_CONTROLLER_TYPE_CCID, VIR_DOMAIN_CONTROLLER_TYPE_USB, + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, VIR_DOMAIN_CONTROLLER_TYPE_LAST }; -- 1.7.2.5

format command line of qemu to add pci-bridge like this: -device pci-bridge. and also add a qemu capability to check if qemu support pci-bridge device Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 70 ++++++++++++++++++++++++++++------------- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 104a3f8..90c08b9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -200,6 +200,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "cirrus-vga", "vmware-svga", "device-video-primary", + "pci-bridge", ); struct _qemuCaps { @@ -1339,6 +1340,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = { { "VGA", QEMU_CAPS_DEVICE_VGA }, { "cirrus-vga", QEMU_CAPS_DEVICE_CIRRUS_VGA }, { "vmware-svga", QEMU_CAPS_DEVICE_VMWARE_SVGA }, + { "pci-bridge", QEMU_CAPS_DEVICE_PCI_BRIDGE }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index bf4eef8..64fc73d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -162,6 +162,7 @@ enum qemuCapsFlags { QEMU_CAPS_DEVICE_VMWARE_SVGA = 122, /* -device vmware-svga */ QEMU_CAPS_DEVICE_VIDEO_PRIMARY = 123, /* safe to use -device XXX for primary video device */ + QEMU_CAPS_DEVICE_PCI_BRIDGE = 124, /* -device pci-bridge */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04a9512..48b4f46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { char *addr; - if (dev->addr.pci.domain != 0 || - dev->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI domain 0 and bus 0 are available")); - return NULL; - } - if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, @@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return addr; } +static int qemuPciBridgeSupport(virDomainDefPtr def) +{ + int i, c = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr controller = def->controllers[i]; + + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE) + c++; + } -static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + if (c > 1) + return 0; + else + return -1; +} + +static int qemuCollectPCIAddress(virDomainDefPtr def, virDomainDeviceDefPtr device, virDomainDeviceInfoPtr info, void *opaque) @@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } + if (info->addr.pci.domain != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with " + "domain=0 are supported")); + return -1; + } + + if (info->addr.pci.bus != 0 && qemuPciBridgeSupport(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with bus=0 are" + " supported without pci-bridge support")); + return -1; + } + addr = qemuPCIAddressAsString(info); if (!addr) goto cleanup; @@ -1012,7 +1035,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, if (info->addr.pci.function != 0) { virReportError(VIR_ERR_XML_ERROR, _("Attempted double use of PCI Address '%s' " - "(may need \"multifunction='on'\" for device on function 0)"), + "(may need \"multifunction='on'\" for " + "device on function 0)"), addr); } else { virReportError(VIR_ERR_XML_ERROR, @@ -1047,7 +1071,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, goto cleanup; } - VIR_DEBUG("Remembering PCI addr %s (multifunction=off for function 0)", addr); + VIR_DEBUG("Remembering PCI addr %s (multifunction=off" + " for function 0)", addr); if (virHashAddEntry(addrs->used, addr, addr)) goto cleanup; addr = NULL; @@ -1763,16 +1788,6 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, qemuCapsPtr caps) { if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (info->addr.pci.domain != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with domain=0 are supported")); - return -1; - } - if (info->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with bus=0 are supported")); - return -1; - } if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIFUNCTION)) { if (info->addr.pci.function > 7) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1787,6 +1802,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, "are supported with this QEMU binary")); return -1; } + if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'multifunction=on' is not supported with " @@ -1797,11 +1813,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, /* XXX * When QEMU grows support for > 1 PCI bus, then pci.0 changes - * to pci.1, pci.2, etc + * to pci.1, pci.2, etc, (e.g. when support pci-to-pci bridge) * When QEMU grows support for > 1 PCI domain, then pci.0 change * to pciNN.0 where NN is the domain number */ - if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) + if (qemuCapsGet(caps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) + virBufferAsprintf(buf, ",bus=pci.%d", info->addr.pci.bus); + else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) virBufferAsprintf(buf, ",bus=pci.0"); else virBufferAsprintf(buf, ",bus=pci"); @@ -3064,6 +3082,12 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, int model; switch (def->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE: + virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d", def->idx+1); + virBufferAsprintf(&buf, ",id=pci.%d", def->idx); + if (def->idx == 0) + goto out; + break; case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: model = def->model; if ((qemuSetScsiControllerModel(domainDef, caps, &model)) < 0) @@ -3137,6 +3161,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, if (qemuBuildDeviceAddressStr(&buf, &def->info, caps) < 0) goto error; +out: if (virBufferError(&buf)) { virReportOOMError(); goto error; @@ -5033,6 +5058,7 @@ qemuBuildCommandLine(virConnectPtr conn, /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to * remove the PIIX4. */ + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, VIR_DOMAIN_CONTROLLER_TYPE_SATA, -- 1.7.2.5

On 01/10/13 02:04, liguang wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04a9512..48b4f46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { char *addr;
- if (dev->addr.pci.domain != 0 || - dev->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI domain 0 and bus 0 are available")); - return NULL; - } - if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, @@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return addr; }
+static int qemuPciBridgeSupport(virDomainDefPtr def) +{ + int i, c = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr controller = def->controllers[i]; + + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE) + c++; + }
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + if (c > 1) + return 0; + else + return -1; +} + +static int qemuCollectPCIAddress(virDomainDefPtr def, virDomainDeviceDefPtr device, virDomainDeviceInfoPtr info, void *opaque) @@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; }
+ if (info->addr.pci.domain != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with " + "domain=0 are supported")); + return -1; + } + + if (info->addr.pci.bus != 0 && qemuPciBridgeSupport(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with bus=0 are" + " supported without pci-bridge support")); + return -1; + } +
This would allow any bus number, even if we don't have enough PCI bridges. It is also not the only place where qemuPCIAddressAsString is called from and where this needs to be checked. We also have other functions assuming bus = 0. qemuDomainPCIAddressGetNextSlot should also take the bridges into account, for devices without a specified address. I'll try to post patches dealing with that sometime later this week. Jan

在 2013-01-14一的 17:23 +0100,Ján Tomko写道:
On 01/10/13 02:04, liguang wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04a9512..48b4f46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { char *addr;
- if (dev->addr.pci.domain != 0 || - dev->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI domain 0 and bus 0 are available")); - return NULL; - } - if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, @@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return addr; }
+static int qemuPciBridgeSupport(virDomainDefPtr def) +{ + int i, c = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr controller = def->controllers[i]; + + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE) + c++; + }
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + if (c > 1) + return 0; + else + return -1; +} + +static int qemuCollectPCIAddress(virDomainDefPtr def, virDomainDeviceDefPtr device, virDomainDeviceInfoPtr info, void *opaque) @@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; }
+ if (info->addr.pci.domain != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with " + "domain=0 are supported")); + return -1; + } + + if (info->addr.pci.bus != 0 && qemuPciBridgeSupport(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with bus=0 are" + " supported without pci-bridge support")); + return -1; + } +
This would allow any bus number, even if we don't have enough PCI bridges.
maybe, but who tell us the pci bus number limitation? if I know the limitation I will limit it.
It is also not the only place where qemuPCIAddressAsString is called from and where this needs to be checked. We also have other functions assuming bus = 0. qemuDomainPCIAddressGetNextSlot should also take the bridges into account, for devices without a specified address.
Yes, but I removed others, for they are unnecessary.
I'll try to post patches dealing with that sometime later this week. Jan
-- regards! li guang

On Mon, Jan 14, 2013 at 05:23:54PM +0100, Ján Tomko wrote:
On 01/10/13 02:04, liguang wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04a9512..48b4f46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { char *addr;
- if (dev->addr.pci.domain != 0 || - dev->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI domain 0 and bus 0 are available")); - return NULL; - } - if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, @@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return addr; }
+static int qemuPciBridgeSupport(virDomainDefPtr def) +{ + int i, c = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr controller = def->controllers[i]; + + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE) + c++; + }
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + if (c > 1) + return 0; + else + return -1; +} + +static int qemuCollectPCIAddress(virDomainDefPtr def, virDomainDeviceDefPtr device, virDomainDeviceInfoPtr info, void *opaque) @@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; }
+ if (info->addr.pci.domain != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with " + "domain=0 are supported")); + return -1; + } + + if (info->addr.pci.bus != 0 && qemuPciBridgeSupport(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with bus=0 are" + " supported without pci-bridge support")); + return -1; + } +
This would allow any bus number, even if we don't have enough PCI bridges. It is also not the only place where qemuPCIAddressAsString is called from and where this needs to be checked.
With other types of addresses, we will auto-create the <controller> elements if we find the controller does not exist. We should probably do the same for PCI. ie if you have bus == 2 and no PCI bridge for that bus exists, then add one. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

在 2013-01-15二的 09:27 +0000,Daniel P. Berrange写道:
On Mon, Jan 14, 2013 at 05:23:54PM +0100, Ján Tomko wrote:
On 01/10/13 02:04, liguang wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04a9512..48b4f46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { char *addr;
- if (dev->addr.pci.domain != 0 || - dev->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI domain 0 and bus 0 are available")); - return NULL; - } - if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, @@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return addr; }
+static int qemuPciBridgeSupport(virDomainDefPtr def) +{ + int i, c = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr controller = def->controllers[i]; + + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE) + c++; + }
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + if (c > 1) + return 0; + else + return -1; +} + +static int qemuCollectPCIAddress(virDomainDefPtr def, virDomainDeviceDefPtr device, virDomainDeviceInfoPtr info, void *opaque) @@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; }
+ if (info->addr.pci.domain != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with " + "domain=0 are supported")); + return -1; + } + + if (info->addr.pci.bus != 0 && qemuPciBridgeSupport(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with bus=0 are" + " supported without pci-bridge support")); + return -1; + } +
This would allow any bus number, even if we don't have enough PCI bridges. It is also not the only place where qemuPCIAddressAsString is called from and where this needs to be checked.
With other types of addresses, we will auto-create the <controller> elements if we find the controller does not exist. We should probably do the same for PCI. ie if you have bus == 2 and no PCI bridge for that bus exists, then add one.
Oh, this amazing feature will remove all the need of controller pre-definition in domain's XML file, will it? could you please give me an already exist example? just can't find them by a quick search. Thanks!
Daniel
-- regards! li guang

On Wed, Jan 16, 2013 at 04:24:55PM +0800, li guang wrote:
在 2013-01-15二的 09:27 +0000,Daniel P. Berrange写道:
On Mon, Jan 14, 2013 at 05:23:54PM +0100, Ján Tomko wrote:
On 01/10/13 02:04, liguang wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04a9512..48b4f46 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { char *addr;
- if (dev->addr.pci.domain != 0 || - dev->addr.pci.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI domain 0 and bus 0 are available")); - return NULL; - } - if (virAsprintf(&addr, "%d:%d:%d.%d", dev->addr.pci.domain, dev->addr.pci.bus, @@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) return addr; }
+static int qemuPciBridgeSupport(virDomainDefPtr def) +{ + int i, c = 0; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr controller = def->controllers[i]; + + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE) + c++; + }
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + if (c > 1) + return 0; + else + return -1; +} + +static int qemuCollectPCIAddress(virDomainDefPtr def, virDomainDeviceDefPtr device, virDomainDeviceInfoPtr info, void *opaque) @@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; }
+ if (info->addr.pci.domain != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with " + "domain=0 are supported")); + return -1; + } + + if (info->addr.pci.bus != 0 && qemuPciBridgeSupport(def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with bus=0 are" + " supported without pci-bridge support")); + return -1; + } +
This would allow any bus number, even if we don't have enough PCI bridges. It is also not the only place where qemuPCIAddressAsString is called from and where this needs to be checked.
With other types of addresses, we will auto-create the <controller> elements if we find the controller does not exist. We should probably do the same for PCI. ie if you have bus == 2 and no PCI bridge for that bus exists, then add one.
Oh, this amazing feature will remove all the need of controller pre-definition in domain's XML file, will it? could you please give me an already exist example? just can't find them by a quick search.
Take a look at the virDomainDefAddImplicitControllers method in domain_conf.c Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

在 2013-01-16三的 09:26 +0000,Daniel P. Berrange写道:
On Wed, Jan 16, 2013 at 04:24:55PM +0800, li guang wrote:
在 2013-01-15二的 09:27 +0000,Daniel P. Berrange写道:
On Mon, Jan 14, 2013 at 05:23:54PM +0100, Ján Tomko wrote:
On 01/10/13 02:04, liguang wrote:
This would allow any bus number, even if we don't have enough PCI bridges. It is also not the only place where qemuPCIAddressAsString is called from and where this needs to be checked.
With other types of addresses, we will auto-create the <controller> elements if we find the controller does not exist. We should probably do the same for PCI. ie if you have bus == 2 and no PCI bridge for that bus exists, then add one.
Oh, this amazing feature will remove all the need of controller pre-definition in domain's XML file, will it? could you please give me an already exist example? just can't find them by a quick search.
Take a look at the virDomainDefAddImplicitControllers method in domain_conf.c
Regards, Daniel
Hi, Daniel do you like following changes: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8ebe77d..988e4f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11756,6 +11756,60 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def) } +static int +virDomainDefMaybeAddPcibridgeController(virDomainDefPtr def) +{ + int i, idx = 0; + + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + continue; + idx = def->nets[i]->info.addr.pci.bus; + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, + idx) < 0) + return -1; + } + + for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + continue; + idx = def->sounds[i]->info.addr.pci.bus; + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, + idx) < 0) + return -1; + } + + for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + continue; + idx = def->videos[i]->info.addr.pci.bus; + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, + idx) < 0) + return -1; + } + + if (def->watchdog->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + idx = def->watchdog->info.addr.pci.bus; + + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, + idx) < 0) + return -1; + + if (def->memballoon->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + idx = def->memballoon->info.addr.pci.bus; + + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE, + idx) < 0) + return -1; + + return 0; +} + /* * Based on the declared <address/> info for any devices, * add necessary drive controllers which are not already present @@ -11790,6 +11844,9 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def) if (virDomainDefMaybeAddSmartcardController(def) < 0) return -1; + if (virDomainDefMaybeAddPcibridgeController(def) < 0) + return -1; + return 0; } -- regards! li guang

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- docs/schemas/domaincommon.rng | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0529d62..f4c22b6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1324,6 +1324,7 @@ <value>sata</value> <value>ccid</value> <value>usb</value> + <value>pci-bridge</value> </choice> </attribute> </optional> -- 1.7.2.5

ping ... 在 2013-01-10四的 09:04 +0800,liguang写道:
Now, it's impossible to arrange devices into multi-pci-bus, for example: <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x1' slot='0x02' function='0x0'/> </video> libvirt will complain about "bus != 0", fortunately, qemu supports pci-to-pci bridge, if we want to use multi-pci-bus, we can define 2 pci bridge controllers, then attach 1 to the other as a subordinate pci-bus, so, 2 pci-buses appear. for example: <controller type='pci-bridge' index='0'/> <controller type='pci-bridge' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x01' slot='0x02' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video>
src/conf/domain_conf.c | 5 ++++- src/conf/domain_conf.h | 1 + docs/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 70 ++++++++++++++++++++----- 6 files changed, 57 insertions(+), 23 deletions(-)
-- regards! li guang

Hi, All any other comments? 在 2013-01-14一的 11:42 +0800,li guang写道:
ping ...
在 2013-01-10四的 09:04 +0800,liguang写道:
Now, it's impossible to arrange devices into multi-pci-bus, for example: <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x1' slot='0x02' function='0x0'/> </video> libvirt will complain about "bus != 0", fortunately, qemu supports pci-to-pci bridge, if we want to use multi-pci-bus, we can define 2 pci bridge controllers, then attach 1 to the other as a subordinate pci-bus, so, 2 pci-buses appear. for example: <controller type='pci-bridge' index='0'/> <controller type='pci-bridge' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x01' slot='0x02' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video>
src/conf/domain_conf.c | 5 ++++- src/conf/domain_conf.h | 1 + docs/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 70 ++++++++++++++++++++----- 6 files changed, 57 insertions(+), 23 deletions(-)
participants (4)
-
Daniel P. Berrange
-
Ján Tomko
-
li guang
-
liguang