On 05/08/2015 01:17 PM, Laine Stump wrote: > On 05/08/2015 12:51 PM, John Ferlan wrote: >> >> On 05/05/2015 02:03 PM, Laine Stump wrote: >>> No functional change, just rearrange this function and pass in full >>> domain definition to make it simpler to add exceptions. >>> --- >>> src/qemu/qemu_command.c | 32 ++++++++++++++++++++++---------- >>> src/qemu/qemu_command.h | 2 +- >>> src/qemu/qemu_hotplug.c | 4 ++-- >>> 3 files changed, 25 insertions(+), 13 deletions(-) >>> >>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >>> index b76bd98..340478c 100644 >>> --- a/src/qemu/qemu_command.c >>> +++ b/src/qemu/qemu_command.c >>> @@ -1031,22 +1031,34 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redir >>> >>> >>> int >>> -qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller) >>> +qemuAssignDeviceControllerAlias(ATTRIBUTE_UNUSED virDomainDefPtr def, >> ?? Not using it now or in this series to add exceptions, so is there >> really a need for it? > > Patch 6 - add exceptions for primate ide/sata controllers. > oh - right... but I was wondering why no compiler complaint on the ATTRIBUTE_UNUSED... Perhaps it's "ordering related"... That is should it be: virDomainDefPtr def ATTRIBUTE_UNUSED, here... then in patch 6 it gets removed... That's why my eyes locked onto... >> >>> + virDomainControllerDefPtr controller) >>> { >>> - const char *prefix = virDomainControllerTypeToString(controller->type); >>> + int ret = 0; >>> + >>> + VIR_FREE(controller->info.alias); >>> >>> - if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { >>> - /* only pcie-root uses a different naming convention >>> - * ("pcie.0"), because it is hardcoded that way in qemu. All >>> - * other buses use the consistent "pci.%u". >>> + switch (controller->type) { >> Similar to 3/13 >> >> s/(/((virDomainControllerType)/ >> >>> + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: >>> + /* pcie-root uses a different naming convention ("pcie.0"), >>> + * because it is hardcoded that way in qemu. All other PCI >>> + * buses use the consistent "pci.%u". >>> */ >>> if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) >>> - return virAsprintf(&controller->info.alias, "pcie.%d", controller->idx); >>> + ret = virAsprintf(&controller->info.alias, "pcie.%d", controller->idx); >>> else >>> - return virAsprintf(&controller->info.alias, "pci.%d", controller->idx); >>> + ret = virAsprintf(&controller->info.alias, "pci.%d", controller->idx); >>> + break; >>> + default: >> Change default to: >> >> case VIR_DOMAIN_CONTROLLER_TYPE_IDE: >> case VIR_DOMAIN_CONTROLLER_TYPE_FDC: >> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: >> case VIR_DOMAIN_CONTROLLER_TYPE_SATA: >> case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: >> case VIR_DOMAIN_CONTROLLER_TYPE_CCID: >> case VIR_DOMAIN_CONTROLLER_TYPE_USB: >> ret = virAsprintf(&controller->info.alias, "%s%d", >> virDomainControllerTypeToString(controller->type), >> controller->idx); >> break; >> >> case VIR_DOMAIN_CONTROLLER_TYPE_LAST: >> Some sort of virReportError... >> ret = -1; >>> + break; >>> } >>> >>> - return virAsprintf(&controller->info.alias, "%s%d", prefix, controller->idx); >>> + /* catchall for anything that wasn't an exception */ >>> + if (ret == 0 && !controller->info.alias) >>> + ret = virAsprintf(&controller->info.alias, "%s%d", >>> + virDomainControllerTypeToString(controller->type), >>> + controller->idx); >> Thus removing the need for this ^^^^ > > That won't work, because the cases for IDE and SATA don't always set the > alias (they only set it or machinetypes and controller indexes where it > needs to be an exception). > Huh? Patch 6 has those exceptions, but the way I'm reading how you wrote the code an alias would be assigned (either "ide.%d" or "sata.%d") because you'd fall into the catchall code as ret would be 0 and info.alias would be NULL. >> ACK w/ adjustments... Although I'm still unsure why we want to pass >> the whole domain def... > > You need the DomainDefPtr so you check the machinetype with > qemuDomainMachineIs*() > > Yes, now I see - again, locked onto the ATTRIBUTE_UNUSED John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list