[libvirt] [PATCH 0/3] Introduce QEMUD_CMD_FLAG_PCI_MULTIBUS for -device bus=pci(.0)

This series introduces a new (fake) flag for the qemu binary. The flag QEMUD_CMD_FLAG_PCI_MULTIBUS is used to mark an architecture to be able to have multiple PCI-busses. When executing qemu, -device should take either a bus-string like "pci" or "pci.0". Where the latter can be used to select the appropriate bus for the PCI-device. Currently libvirt breaks starting qemu-system-$ARCH as the -device option always sets bus=pci.0. On most architectures, there is no PCI-Multibus support in qemu. This series enables libvirt to select the architectures that support PCI-multibus and sets the flag for x86_64 and i686 only. Reference: - https://bugzilla.redhat.com/show_bug.cgi?id=667345 Kind regards, Niels -- Niels de Vos (3): qemuBuildDeviceAddressStr() checks for QEMUD_CMD_FLAG_PCI_MULTIBUS Set QEMUD_CMD_FLAG_PCI_MULTIBUS for x86_64 and i686 architectures Add myself to AUTHORS AUTHORS | 1 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 67 ++++++++++++++++++++++++++--------------- src/qemu/qemu_command.h | 18 +++++++---- src/qemu/qemu_hotplug.c | 5 ++- 5 files changed, 59 insertions(+), 33 deletions(-) -- 1.7.3.5

Depending if the qemu binary supports multiple pci-busses, the device options will contain "bus=pci" or "bus=pci.0". Signed-off-by: Niels de Vos <devos@fedoraproject.org> --- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 61 ++++++++++++++++++++++++----------------- src/qemu/qemu_command.h | 18 ++++++++---- src/qemu/qemu_hotplug.c | 5 ++- 4 files changed, 52 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 59bb22a..e04f6a8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -86,6 +86,7 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_BOOTINDEX = (1LL << 49), /* -device bootindex property */ QEMUD_CMD_FLAG_HDA_DUPLEX = (1LL << 50), /* -device hda-duplex */ QEMUD_CMD_FLAG_DRIVE_AIO = (1LL << 51), /* -drive aio= supported */ + QEMUD_CMD_FLAG_PCI_MULTIBUS = (1LL << 52), /* bus=pci.0 vs bus=pci */ }; virCapsPtr qemuCapsInit(virCapsPtr old_caps); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a0f86a3..457c8b0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1088,7 +1088,8 @@ error: static int qemuBuildDeviceAddressStr(virBufferPtr buf, - virDomainDeviceInfoPtr info) + virDomainDeviceInfoPtr info, + unsigned long long qemuCmdFlags) { if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { if (info->addr.pci.domain != 0) { @@ -1113,7 +1114,10 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, * When QEMU grows support for > 1 PCI domain, then pci.0 change * to pciNN.0 where NN is the domain number */ - virBufferVSprintf(buf, ",bus=pci.0,addr=0x%x", info->addr.pci.slot); + if (qemuCmdFlags & QEMUD_CMD_FLAG_PCI_MULTIBUS) + virBufferVSprintf(buf, ",bus=pci.0,addr=0x%x", info->addr.pci.slot); + else + virBufferVSprintf(buf, ",bus=pci,addr=0x%x", info->addr.pci.slot); } return 0; } @@ -1383,7 +1387,7 @@ qemuBuildDriveDevStr(virDomainDiskDefPtr disk, break; case VIR_DOMAIN_DISK_BUS_VIRTIO: virBufferAddLit(&opt, "virtio-blk-pci"); - qemuBuildDeviceAddressStr(&opt, &disk->info); + qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCmdFlags); break; case VIR_DOMAIN_DISK_BUS_USB: virBufferAddLit(&opt, "usb-storage"); @@ -1447,7 +1451,8 @@ error: char * -qemuBuildFSDevStr(virDomainFSDefPtr fs) +qemuBuildFSDevStr(virDomainFSDefPtr fs, + unsigned long long qemuCmdFlags) { virBuffer opt = VIR_BUFFER_INITIALIZER; @@ -1461,7 +1466,7 @@ qemuBuildFSDevStr(virDomainFSDefPtr fs) virBufferVSprintf(&opt, ",id=%s", fs->info.alias); virBufferVSprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferVSprintf(&opt, ",mount_tag=%s", fs->dst); - qemuBuildDeviceAddressStr(&opt, &fs->info); + qemuBuildDeviceAddressStr(&opt, &fs->info, qemuCmdFlags); if (virBufferError(&opt)) { virReportOOMError(); @@ -1477,7 +1482,8 @@ error: char * -qemuBuildControllerDevStr(virDomainControllerDefPtr def) +qemuBuildControllerDevStr(virDomainControllerDefPtr def, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1514,7 +1520,7 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def) goto error; } - if (qemuBuildDeviceAddressStr(&buf, &def->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &def->info, qemuCmdFlags) < 0) goto error; if (virBufferError(&buf)) { @@ -1581,7 +1587,7 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, net->mac[0], net->mac[1], net->mac[2], net->mac[3], net->mac[4], net->mac[5]); - if (qemuBuildDeviceAddressStr(&buf, &net->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &net->info, qemuCmdFlags) < 0) goto error; if (net->bootIndex && (qemuCmdFlags & QEMUD_CMD_FLAG_BOOTINDEX)) virBufferVSprintf(&buf, ",bootindex=%d", net->bootIndex); @@ -1702,7 +1708,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, char * -qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev) +qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1715,7 +1722,7 @@ qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev) virBufferVSprintf(&buf, "%s", model); virBufferVSprintf(&buf, ",id=%s", dev->info.alias); - if (qemuBuildDeviceAddressStr(&buf, &dev->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCmdFlags) < 0) goto error; if (virBufferError(&buf)) { @@ -1732,13 +1739,14 @@ error: char * -qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev) +qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "virtio-balloon-pci"); virBufferVSprintf(&buf, ",id=%s", dev->info.alias); - if (qemuBuildDeviceAddressStr(&buf, &dev->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCmdFlags) < 0) goto error; if (virBufferError(&buf)) { @@ -1778,7 +1786,8 @@ error: char * -qemuBuildSoundDevStr(virDomainSoundDefPtr sound) +qemuBuildSoundDevStr(virDomainSoundDefPtr sound, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; const char *model = virDomainSoundModelTypeToString(sound->model); @@ -1799,7 +1808,7 @@ qemuBuildSoundDevStr(virDomainSoundDefPtr sound) virBufferVSprintf(&buf, "%s", model); virBufferVSprintf(&buf, ",id=%s", sound->info.alias); - if (qemuBuildDeviceAddressStr(&buf, &sound->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &sound->info, qemuCmdFlags) < 0) goto error; if (virBufferError(&buf)) { @@ -1839,7 +1848,8 @@ error: } static char * -qemuBuildVideoDevStr(virDomainVideoDefPtr video) +qemuBuildVideoDevStr(virDomainVideoDefPtr video, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; const char *model = qemuVideoTypeToString(video->type); @@ -1852,7 +1862,7 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video) virBufferVSprintf(&buf, "%s", model); virBufferVSprintf(&buf, ",id=%s", video->info.alias); - if (qemuBuildDeviceAddressStr(&buf, &video->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &video->info, qemuCmdFlags) < 0) goto error; if (virBufferError(&buf)) { @@ -1894,7 +1904,8 @@ qemuOpenPCIConfig(virDomainHostdevDefPtr dev) } char * -qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd) +qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1906,7 +1917,7 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd) virBufferVSprintf(&buf, ",id=%s", dev->info.alias); if (configfd && *configfd) virBufferVSprintf(&buf, ",configfd=%s", configfd); - if (qemuBuildDeviceAddressStr(&buf, &dev->info) < 0) + if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCmdFlags) < 0) goto error; if (virBufferError(&buf)) { @@ -3044,7 +3055,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-device"); char *devstr; - if (!(devstr = qemuBuildControllerDevStr(def->controllers[i]))) + if (!(devstr = qemuBuildControllerDevStr(def->controllers[i], qemuCmdFlags))) goto no_memory; virCommandAddArg(cmd, devstr); @@ -3295,7 +3306,7 @@ qemuBuildCommandLine(virConnectPtr conn, VIR_FREE(optstr); virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildFSDevStr(fs))) + if (!(optstr = qemuBuildFSDevStr(fs, qemuCmdFlags))) goto error; virCommandAddArg(cmd, optstr); VIR_FREE(optstr); @@ -3826,7 +3837,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-device"); - if (!(str = qemuBuildVideoDevStr(def->videos[i]))) + if (!(str = qemuBuildVideoDevStr(def->videos[i], qemuCmdFlags))) goto error; virCommandAddArg(cmd, str); @@ -3861,7 +3872,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL); } else { virCommandAddArg(cmd, "-device"); - if (!(str = qemuBuildSoundDevStr(sound))) + if (!(str = qemuBuildSoundDevStr(sound, qemuCmdFlags))) goto error; virCommandAddArg(cmd, str); @@ -3927,7 +3938,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { virCommandAddArg(cmd, "-device"); - optstr = qemuBuildWatchdogDevStr(watchdog); + optstr = qemuBuildWatchdogDevStr(watchdog, qemuCmdFlags); if (!optstr) goto error; } else { @@ -4001,7 +4012,7 @@ qemuBuildCommandLine(virConnectPtr conn, } } virCommandAddArg(cmd, "-device"); - devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name); + devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name, qemuCmdFlags); VIR_FREE(configfd_name); if (!devstr) goto error; @@ -4092,7 +4103,7 @@ qemuBuildCommandLine(virConnectPtr conn, char *optstr; virCommandAddArg(cmd, "-device"); - optstr = qemuBuildMemballoonDevStr(def->memballoon); + optstr = qemuBuildMemballoonDevStr(def->memballoon, qemuCmdFlags); if (!optstr) goto error; virCommandAddArg(cmd, optstr); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index e410259..6d57007 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -79,23 +79,29 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, /* Current, best practice */ char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk, unsigned long long qemuCmdFlags); -char * qemuBuildFSDevStr(virDomainFSDefPtr fs); +char * qemuBuildFSDevStr(virDomainFSDefPtr fs, + unsigned long long qemuCmdFlags); /* Current, best practice */ -char * qemuBuildControllerDevStr(virDomainControllerDefPtr def); +char * qemuBuildControllerDevStr(virDomainControllerDefPtr def, + unsigned long long qemuCmdFlags); -char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev); +char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev, + unsigned long long qemuCmdFlags); -char * qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev); +char * qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev, + unsigned long long qemuCmdFlags); char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev); -char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound); +char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound, + unsigned long long qemuCmdFlags); /* Legacy, pre device support */ char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev); /* Current, best practice */ char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, - const char *configfd); + const char *configfd, + unsigned long long qemuCmdFlags); int qemuOpenPCIConfig(virDomainHostdevDefPtr dev); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c334f52..1af767e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -259,7 +259,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, if (qemuAssignDeviceControllerAlias(controller) < 0) goto cleanup; - if (!(devstr = qemuBuildControllerDevStr(controller))) { + if (!(devstr = qemuBuildControllerDevStr(controller, qemuCmdFlags))) { goto cleanup; } } @@ -823,7 +823,8 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, goto error; } - if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name))) + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name, + qemuCmdFlags))) goto error; qemuDomainObjEnterMonitorWithDriver(driver, vm); -- 1.7.3.5

On Tue, Feb 01, 2011 at 04:22:01PM +0000, Niels de Vos wrote:
Depending if the qemu binary supports multiple pci-busses, the device options will contain "bus=pci" or "bus=pci.0".
Signed-off-by: Niels de Vos <devos@fedoraproject.org>
Is that your primary email addr you want recorded in the GIT history & AUTHORS file, or is there an authoritative one that should be used ?
--- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 61 ++++++++++++++++++++++++----------------- src/qemu/qemu_command.h | 18 ++++++++---- src/qemu/qemu_hotplug.c | 5 ++- 4 files changed, 52 insertions(+), 33 deletions(-)
ACK Daniel

On 02/01/2011 09:22 AM, Niels de Vos wrote:
- virBufferVSprintf(buf, ",bus=pci.0,addr=0x%x", info->addr.pci.slot); + if (qemuCmdFlags & QEMUD_CMD_FLAG_PCI_MULTIBUS) + virBufferVSprintf(buf, ",bus=pci.0,addr=0x%x", info->addr.pci.slot); + else + virBufferVSprintf(buf, ",bus=pci,addr=0x%x", info->addr.pci.slot);
I probably would have done: virBufferVSprintf(buf, ",bus=pci%s,addr=0x%x", qemuCmdFlags & QEMU_CMD_FLAG_PCI_MULTIBUS ? ".0", "", info->addr.pci.slot); to avoid the if/else, but that's not worth changing. Agree with danpb's ack. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Only x86_64 and i686 seem to have support for multiple PCI-busses. When a guest of these architectures is started, set the QEMUD_CMD_FLAG_PCI_MULTIBUS flag. Signed-off-by: Niels de Vos <devos@fedoraproject.org> --- src/qemu/qemu_command.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 457c8b0..0899710 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2640,6 +2640,12 @@ qemuBuildCommandLine(virConnectPtr conn, break; } + /* Currently only x86_64 and i686 support PCI-multibus. */ + if (STREQLEN(def->os.arch, "x86_64", 6) || + STREQLEN(def->os.arch, "i686", 4)) { + qemuCmdFlags |= QEMUD_CMD_FLAG_PCI_MULTIBUS; + } + cmd = virCommandNewArgList(emulator, "-S", NULL); virCommandAddEnvPassCommon(cmd); -- 1.7.3.5

On Tue, Feb 01, 2011 at 04:22:02PM +0000, Niels de Vos wrote:
Only x86_64 and i686 seem to have support for multiple PCI-busses. When a guest of these architectures is started, set the QEMUD_CMD_FLAG_PCI_MULTIBUS flag.
Signed-off-by: Niels de Vos <devos@fedoraproject.org> --- src/qemu/qemu_command.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 457c8b0..0899710 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2640,6 +2640,12 @@ qemuBuildCommandLine(virConnectPtr conn, break; }
+ /* Currently only x86_64 and i686 support PCI-multibus. */ + if (STREQLEN(def->os.arch, "x86_64", 6) || + STREQLEN(def->os.arch, "i686", 4)) { + qemuCmdFlags |= QEMUD_CMD_FLAG_PCI_MULTIBUS; + } + cmd = virCommandNewArgList(emulator, "-S", NULL);
We can just do a straight STREQ() here, rather than STREQLEN. Also this patch should be merged into the previous one, otherwise there is a temporary regression for x86 after patch 1 ACK Daniel

Without this, ./autobuild.sh fails. Signed-off-by: Niels de Vos <devos@fedoraproject.org> --- AUTHORS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/AUTHORS b/AUTHORS index 131b30a..c69f0b5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -147,6 +147,7 @@ Patches have also been contributed by: Michal Prívozník <mprivozn@redhat.com> Juerg Haefliger <juerg.haefliger@hp.com> Matthias Dahl <mdvirt@designassembly.de> + Niels de Vos <devos@fedoraproject.org> [....send patches to get your name here....] -- 1.7.3.5

On Tue, Feb 01, 2011 at 04:22:03PM +0000, Niels de Vos wrote:
Without this, ./autobuild.sh fails.
Signed-off-by: Niels de Vos <devos@fedoraproject.org> --- AUTHORS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/AUTHORS b/AUTHORS index 131b30a..c69f0b5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -147,6 +147,7 @@ Patches have also been contributed by: Michal Prívozník <mprivozn@redhat.com> Juerg Haefliger <juerg.haefliger@hp.com> Matthias Dahl <mdvirt@designassembly.de> + Niels de Vos <devos@fedoraproject.org>
[....send patches to get your name here....]
ACK assuming you want this particular email addr recorded Daniel

On Wed, Feb 2, 2011 at 12:04 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Feb 01, 2011 at 04:22:03PM +0000, Niels de Vos wrote:
Without this, ./autobuild.sh fails.
Signed-off-by: Niels de Vos <devos@fedoraproject.org> --- AUTHORS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/AUTHORS b/AUTHORS index 131b30a..c69f0b5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -147,6 +147,7 @@ Patches have also been contributed by: Michal Prívozník <mprivozn@redhat.com> Juerg Haefliger <juerg.haefliger@hp.com> Matthias Dahl <mdvirt@designassembly.de> + Niels de Vos <devos@fedoraproject.org>
[....send patches to get your name here....]
ACK assuming you want this particular email addr recorded
Daniel
Thanks Daniel! Please replace my address with <ndevos@redhat.com> if possible. Cheers, Niels

On 02/02/2011 08:30 AM, Niels de Vos wrote:
On Wed, Feb 2, 2011 at 12:04 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Feb 01, 2011 at 04:22:03PM +0000, Niels de Vos wrote:
Without this, ./autobuild.sh fails.
ACK assuming you want this particular email addr recorded
Daniel
Thanks Daniel!
Please replace my address with <ndevos@redhat.com> if possible.
I've squashed the three patches into one, tweaked the commit author, and fixed the line in AUTHORS to your redhat address; then pushed. Thanks again for the patch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Feb 2, 2011 at 4:04 PM, Eric Blake <eblake@redhat.com> wrote:
On 02/02/2011 08:30 AM, Niels de Vos wrote:
On Wed, Feb 2, 2011 at 12:04 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Feb 01, 2011 at 04:22:03PM +0000, Niels de Vos wrote:
Without this, ./autobuild.sh fails.
ACK assuming you want this particular email addr recorded
Daniel
Thanks Daniel!
Please replace my address with <ndevos@redhat.com> if possible.
I've squashed the three patches into one, tweaked the commit author, and fixed the line in AUTHORS to your redhat address; then pushed.
Thanks again for the patch.
You're welcome! And thanks for merging, fixing and pushing. Cheers, Niels
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Niels de Vos