[libvirt] [PATCH v2 0/3] Fix user provided aliases for some devices
In case user provides an alias for a device that is implicit (e.g. IDE controller for 440fx, SATA for Q35, etc.) we don't put the device onto qemu command line. Therefore we can't set its alias. diff to v1: - more controllers covered Michal Privoznik (3): virQEMUCapsHasPCIMultiBus: Fix @def type qemuBuildDriveDevStr: Prefer default alias for SATA bus qemuBuildDeviceAddressStr: Prefer default alias for PCI bus src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 45 ++++++++++++++++------ .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 +- .../qemuxml2argv-user-aliases2.args | 1 + .../qemuxml2argv-user-aliases2.xml | 34 ++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 7 files changed, 75 insertions(+), 14 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.xml -- 2.13.6
This function only queries domani @def. It doesn't change it. Therefore it should take const pointer. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1badadbc2..00c00c3c3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2390,7 +2390,7 @@ virQEMUCapsGet(virQEMUCapsPtr qemuCaps, bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, - virDomainDefPtr def) + const virDomainDef *def) { /* x86_64 and i686 support PCI-multibus on all machine types * since forever */ diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f0e2e9016..e7d766f80 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -451,7 +451,7 @@ bool virQEMUCapsGet(virQEMUCapsPtr qemuCaps, virQEMUCapsFlags flag); bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, - virDomainDefPtr def); + const virDomainDef *def); bool virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps, const virDomainDef *def); -- 2.13.6
On Wed, Nov 15, 2017 at 03:40:15PM +0100, Michal Privoznik wrote:
This function only queries domani @def. It doesn't change it.
s/domani/domain/
Therefore it should take const pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Jan
https://bugzilla.redhat.com/show_bug.cgi?id=1434451 Just like in 9324f67a572f9b32 we need to put default sata alias (which is hardcoded to "ide", obvious, right?) onto the command line instead of the one provided by user. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 15 ++++++++-- .../qemuxml2argv-user-aliases2.args | 1 + .../qemuxml2argv-user-aliases2.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 48 insertions(+), 3 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 56729e498..4eda12179 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2020,9 +2020,18 @@ qemuBuildDriveDevStr(const virDomainDef *def, virBufferAddLit(&opt, "ide-drive"); } - if (!(contAlias = virDomainControllerAliasFind(def, VIR_DOMAIN_CONTROLLER_TYPE_SATA, - disk->info.addr.drive.controller))) - goto error; + /* When domain has builtin SATA controller we don't put it onto cmd + * line. Therefore we can't set its alias. In that case, use the + * default one. */ + if (qemuDomainIsQ35(def) && + disk->info.addr.drive.controller == 0) { + contAlias = "ide"; + } else { + if (!(contAlias = virDomainControllerAliasFind(def, + VIR_DOMAIN_CONTROLLER_TYPE_SATA, + disk->info.addr.drive.controller))) + goto error; + } virBufferAsprintf(&opt, ",bus=%s.%d", contAlias, disk->info.addr.drive.unit); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.args b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.args new file mode 120000 index 000000000..e029bc0ec --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.args @@ -0,0 +1 @@ +qemuxml2argv-boot-floppy-q35.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.xml b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.xml new file mode 100644 index 000000000..a288b8611 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.4'>hvm</type> + <boot dev='fd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='floppy'> + <driver name='qemu' type='raw'/> + <source file='/tmp/firmware.img'/> + <target dev='fda' bus='fdc'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'> + <alias name='ua-MySataController'/> + </controller> + <controller type='fdc' index='0'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1bedc6874..d66e77ed4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2827,6 +2827,7 @@ mymain(void) QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4, QEMU_CAPS_VNC, QEMU_CAPS_HDA_DUPLEX); + DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 2.13.6
https://bugzilla.redhat.com/show_bug.cgi?id=1434451 Just like in 9324f67a572f9b32 we need to put default pci-root alias onto the command line instead of the one provided by user. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 30 ++++++++++++++++------ .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 ++- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4eda12179..84aa1b933 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -318,16 +318,30 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && cont->idx == info->addr.pci.bus) { - contAlias = cont->info.alias; contIsPHB = virDomainControllerIsPSeriesPHB(cont); contTargetIndex = cont->opts.pciopts.targetIndex; - if (!contAlias) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Device alias was not set for PCI " - "controller with index %u required " - "for device at address %s"), - info->addr.pci.bus, devStr); - goto cleanup; + + /* When domain has builtin pci-root controller we don't put it + * onto cmd line. Therefore we can't set its alias. In that + * case, use the default one. */ + if (!qemuDomainIsPSeries(domainDef) && + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { + if (virQEMUCapsHasPCIMultiBus(qemuCaps, domainDef)) + contAlias = "pci.0"; + else + contAlias = "pci"; + } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { + contAlias = "pcie.0"; + } else { + contAlias = cont->info.alias; + if (!contAlias) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Device alias was not set for PCI " + "controller with index %u required " + "for device at address %s"), + info->addr.pci.bus, devStr); + goto cleanup; + } } break; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml index d1cb8fea6..c760098fe 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml @@ -74,7 +74,9 @@ <alias name='ua-SomeWeirdController'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='ua-MyPCIRootController'/> + </controller> <controller type='ide' index='0'> <alias name='ua-DoesAnybodyStillUseIDE'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> -- 2.13.6
On Wed, Nov 15, 2017 at 03:40:14PM +0100, Michal Privoznik wrote:
In case user provides an alias for a device that is implicit (e.g. IDE controller for 440fx, SATA for Q35, etc.) we don't put the device onto qemu command line. Therefore we can't set its alias.
diff to v1: - more controllers covered
Michal Privoznik (3): virQEMUCapsHasPCIMultiBus: Fix @def type qemuBuildDriveDevStr: Prefer default alias for SATA bus qemuBuildDeviceAddressStr: Prefer default alias for PCI bus
src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 45 ++++++++++++++++------ .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 +- .../qemuxml2argv-user-aliases2.args | 1 + .../qemuxml2argv-user-aliases2.xml | 34 ++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 7 files changed, 75 insertions(+), 14 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases2.xml
ACK series Jan
participants (2)
-
Ján Tomko -
Michal Privoznik