[libvirt] [PATCH 04/13] qemu: restructure qemuAssignDeviceControllerAlias

[John Ferlan]

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?

> +                                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 ^^^^

ACK w/ adjustments... Although I'm still unsure why we want to pass
the whole domain def...

John

> +    return ret;
>  }
>  
>  static ssize_t
> @@ -1174,7 +1186,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
>              return -1;
>      }
>      for (i = 0; i < def->ncontrollers; i++) {
> -        if (qemuAssignDeviceControllerAlias(def->controllers[i]) < 0)
> +        if (qemuAssignDeviceControllerAlias(def, def->controllers[i]) < 0)
>              return -1;
>      }
>      for (i = 0; i < def->ninputs; i++) {
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 675eb62..d066953 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -287,7 +287,7 @@ int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
>                                virDomainDiskDefPtr def,
>                                virQEMUCapsPtr qemuCaps);
>  int qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev, int idx);
> -int qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller);
> +int qemuAssignDeviceControllerAlias(virDomainDefPtr def, virDomainControllerDefPtr controller);
>  int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, int idx);
>  int qemuAssignDeviceChrAlias(virDomainDefPtr def,
>                               virDomainChrDefPtr chr,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 613b728..24a5f51 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -465,7 +465,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
>                  goto cleanup;
>          }
>          releaseaddr = true;
> -        if (qemuAssignDeviceControllerAlias(controller) < 0)
> +        if (qemuAssignDeviceControllerAlias(vm->def, controller) < 0)
>              goto cleanup;
>  
>          if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
> @@ -3631,7 +3631,7 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>  
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
>          !detach->info.alias) {
> -        if (qemuAssignDeviceControllerAlias(detach) < 0)
> +        if (qemuAssignDeviceControllerAlias(vm->def, detach) < 0)
>              goto cleanup;
>      }
>  
>