Re: [libvirt] [PATCH] qemuBuildDriveDevStr: Prefer default aliases for IDE bus

On Thu, Nov 09, 2017 at 01:43:22PM +0100, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1434451 > > When testing user aliases it was discovered that for 440fx > machine type which has default IDE bus builtin, domain cannot > start if IDE controller has the user provided alias. This is > because for 440fx we don't put the IDE controller onto the > command line (since it is builtin) and therefore any device that > is plugged onto the bus must use the default alias. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/qemu/qemu_command.c                               | 14 +++++++++++--- > tests/qemuxml2argvdata/qemuxml2argv-user-aliases.args |  4 ++-- > 2 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 364196783..6cc77df2e 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -1886,9 +1886,17 @@ qemuBuildDriveDevStr(const virDomainDef *def, >             virBufferAddLit(&opt, "ide-drive"); >         } > > -        if (!(contAlias = virDomainControllerAliasFind(def, > VIR_DOMAIN_CONTROLLER_TYPE_IDE, > -                                                       > disk->info.addr.drive.controller))) > -           goto error; > +        /* When domain has builtin IDE controller we don't put it > onto cmd > +         * line. Therefore we can't set its alias. In that case, use the > +         * default one. */ > +        if (qemuDomainHasBuiltinIDE(def)) { > +            contAlias = "ide"; This logic would fit better inside virDomainControllerAliasFind.

Also, the implicit pci-root on i440fx could also use that treatment.

We do not format aliases for PS2 mouse and keyboard either, but I cannot imagine why the qemu alias would need to match the libvirt alias in that case.

Any other devices come to mind?