[libvirt] [PATCH v3 12/18] storage: Rework getAdapterName to use adapter specific typedefs

[Laine Stump]

On 03/10/2017 04:10 PM, John Ferlan wrote:
> Use the FCHost and SCSIHost adapter specific typedefs
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_scsi.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index 642e795..6c8875c 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -176,15 +176,17 @@ virStoragePoolFCRefreshThread(void *opaque)
>  }
>  
>  static char *
> -getAdapterName(virStoragePoolSourceAdapter adapter)
> +getAdapterName(virStoragePoolSourceAdapterPtr adapter)

Wow. How did all of these get in?

>  {
>      char *name = NULL;
>      char *parentaddr = NULL;
>  
> -    if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
> -        if (adapter.data.scsi_host.has_parent) {
> -            virPCIDeviceAddress addr = adapter.data.scsi_host.parentaddr;
> -            unsigned int unique_id = adapter.data.scsi_host.unique_id;
> +    if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
> +        virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host;
> +
> +        if (scsi_host->has_parent) {
> +            virPCIDeviceAddress addr = scsi_host->parentaddr;

As with the previous patch that did this - how about changing it into virPCIDeviceAddressPtr to avoid the copy?

> +            unsigned int unique_id = scsi_host->unique_id;
>  
>              if (!(name = virSCSIHostGetNameByParentaddr(addr.domain,
>                                                          addr.bus,
> @@ -193,16 +195,15 @@ getAdapterName(virStoragePoolSourceAdapter adapter)
>                                                          unique_id)))
>                  goto cleanup;
>          } else {
> -            ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name));
> +            ignore_value(VIR_STRDUP(name, scsi_host->name));
>          }
> -    } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
> -        if (!(name = virVHBAGetHostByWWN(NULL,
> -                                         adapter.data.fchost.wwnn,
> -                                         adapter.data.fchost.wwpn))) {
> +    } else if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
> +        virStorageAdapterFCHostPtr fchost = &adapter->data.fchost;
> +
> +        if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
>              virReportError(VIR_ERR_XML_ERROR,
>                             _("Failed to find SCSI host with wwnn='%s', "
> -                             "wwpn='%s'"), adapter.data.fchost.wwnn,
> -                           adapter.data.fchost.wwpn);
> +                             "wwpn='%s'"), fchost->wwnn, fchost->wwpn);
>          }
>      }
>  
> @@ -458,7 +459,7 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool,
>  
>      *isActive = false;
>  
> -    if (!(name = getAdapterName(pool->def->source.adapter))) {
> +    if (!(name = getAdapterName(&pool->def->source.adapter))) {
>          /* It's normal for the pool with "fc_host" type source
>           * adapter fails to get the adapter name, since the vHBA
>           * the adapter based on might be not created yet.
> @@ -498,7 +499,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>  
>      pool->def->allocation = pool->def->capacity = pool->def->available = 0;
>  
> -    if (!(name = getAdapterName(pool->def->source.adapter)))
> +    if (!(name = getAdapterName(&pool->def->source.adapter)))
>          return -1;
>  
>      if (virSCSIHostGetNumber(name, &host) < 0)


ACK (and ACK to change the virPCIDeviceAddress to a * while you're at it.)