Re: [libvirt] [PATCH 1/3] qemu: PPCs G3Beige has a default IDE controller

On 11/20/2015 03:20 PM, Guido Günther wrote: >so handle it like I440FX >--- > src/qemu/qemu_command.c | 17 ++++++++++++----- > src/qemu/qemu_domain.c | 7 +++++++ > src/qemu/qemu_domain.h | 1 + > 3 files changed, 20 insertions(+), 5 deletions(-) > >diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >index ef5ef93..d6b7f09 100644 >--- a/src/qemu/qemu_command.c >+++ b/src/qemu/qemu_command.c >@@ -1054,11 +1054,13 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef, > */ > return virAsprintf(&controller->info.alias, "pci.%d", controller->idx); > } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) { >- /* for any machine based on I440FX, the first (and currently >+ /* for any machine based on I440FX or G3Beige, the first (and currently > * only) IDE controller is an integrated controller hardcoded > * with id "ide" > */ >- if (qemuDomainMachineIsI440FX(domainDef) && controller->idx == 0) >+ if ((qemuDomainMachineIsI440FX(domainDef) || >+ qemuDomainMachineIsG3Beige(domainDef)) && >+ controller->idx == 0) > return VIR_STRDUP(controller->info.alias, "ide"); > } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { > /* for any Q35 machine, the first SATA controller is the >@@ -4915,10 +4917,13 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, > case VIR_DOMAIN_CONTROLLER_TYPE_IDE: > /* Since we currently only support the integrated IDE controller >- * on 440fx, if we ever get to here, it's because some other >+ * on 440fx and G3Beige, if we ever get to here, it's because some other > * machinetype had an IDE controller specified, or a 440fx had > * multiple ide controllers. > */ >+ if (qemuDomainMachineIsG3Beige(domainDef)) >+ break; >+ This would cause us to ignore the fact that the config has a secondary IDE controller defined, and we don't support adding a 2nd IDE controller to the qemu commandline. I think it should instead report the following error message just like i440FX (unless the g3beige has an implicit secondary IDE and it is properly named).

Other than that, ACK (and thanks for taking the time to look for other machines that have builtin IDE controllers and writing the patches! I got a private mail about the breakage and asked the question about which machines have builtint IDE on qemu-devel, but hadn't had any more time to follow up).

> if (qemuDomainMachineIsI440FX(domainDef)) > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("Only a single IDE controller is unsupported " Ugh. I should have said "supported" there rather than "unsupported". Can you fix that while you're touching things?