Re: [libvirt] [PATCH 06/13] qemu: add exceptions for alias names of primary sata/ide controllers

On 05/08/2015 01:47 PM, John Ferlan wrote: > > On 05/05/2015 02:03 PM, Laine Stump wrote: >> If a machine is Q35, the primary sata controller is hardcoded in qemu >> to have the id "ide" (with no index in the name). Likewise on >> 440fx-based machinetypes, the primary ide controller is called >> "ide". All other SATA controllers will have the standard name >> "sata%d", where %d is the index of the controller, and if any >> additional IDE controllers are ever supported, they will have the name >> "ide%d". >> --- >> src/qemu/qemu_command.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 340478c..89beeb3 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -1049,6 +1049,21 @@ qemuAssignDeviceControllerAlias(ATTRIBUTE_UNUSED virDomainDefPtr def, >> else >> ret = virAsprintf(&controller->info.alias, "pci.%d", controller->idx); >> break; >> + case VIR_DOMAIN_CONTROLLER_TYPE_IDE: >> + /* for any machine based on I440FX, the first (currently only) >> + * IDE controller is an integrated controller hardcoded with >> + * id "ide" >> + */ >> + if (qemuDomainMachineIsI440FX(def) && controller->idx == 0) >> + ret = VIR_STRDUP(controller->info.alias, "ide"); > Based on my review of 4/13 and my followup comment, there'd need to be > an "else" here with does the : > > ret = virAsprintf(&controller->info.alias, "%s%d", > virDomainControllerTypeToString(controller->type), > controller->idx); Right, that's why I put that extra "catchall" at the bottom of the function. I wanted to avoid duplicating code. > > >> + break; >> + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: >> + /* for any Q35 machine, the first SATA controller is the >> + * integrated one, and it too is hardcoded with id "ide" >> + */ >> + if (qemuDomainMachineIsQ35(def) && controller->idx == 0) >> + ret = VIR_STRDUP(controller->info.alias, "ide"); > similarly, else > ret = virAsprintf(&controller->info.alias, "%s%d", > virDomainControllerTypeToString(controller->type), > controller->idx); ... like this :-)