[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 07/11] storage_scsi: Translate the stable address into scsi host number



On 06/07/2013 01:03 PM, Osier Yang wrote:
> This takes use of the two utils introduced in previous patches.
> 
> Node device HAL backend represents PCI device like PCI_8086_2922,
> (I.E PCI_$vendor_$product), to get the PCI address, we have to
> traverse /sys/devices/ to find it out.
> 
> And to get the current scsi host number assigned by the system
> for the scsi host device. We need to traverse /sys/bus/pci/devices/
> with the found PCI address by getScsiHostParentAddress, and the
> specified unique_id.
> 
> Later patch will allow to omit the "unique_id", by using the
> scsi host which has the smallest unique_id.
> ---
>  src/storage/storage_backend_scsi.c | 81 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index 13c498d..a77b9ae 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -591,6 +591,66 @@ out:
>      return retval;
>  }
>  
> +/*
> + * Node device udev backend and HAL backend represent PCI
> + * device in different style. Udev backend represents it like
> + * pci_0000_00_1f_2, and HAL backend represents it like
> + * pci_8086_2922.
> + *
> + * Covert the value of "parent" into PCI device address string
> + * (e.g. 0000:00:1f:2). Return the PCI address as string on
> + * success, or -1 on failure.
> + */
> +static char *
> +getScsiHostParentAddress(const char *parent)
> +{
> +    char **tokens = NULL;
> +    char *vendor = NULL;
> +    char *product = NULL;
> +    char *ret = NULL;
> +    int len;
> +
> +    if (!strchr(parent, '_')) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("'parent' of scsi_host adapter must "
> +                         "be consistent with name of node device"));
> +        return NULL;
> +    }
> +
> +    if (!(tokens = virStringSplit(parent, "_", 0)))
> +        return NULL;

Given the following code, the call to Split could use 4 (or 5) for the
max...  Not that it matters.

> +
> +    len = virStringListLength(tokens);
> +
> +    switch (len) {
> +    case 4:
> +        if (!(ret = virStringJoin((const char **)(&tokens[1]), ":")))
> +            goto cleanup;
> +        break;
> +
> +    case 2:
> +        vendor = tokens[1];
> +        product = tokens[2];
> +        if (!(ret = virFindPCIDeviceByVPD(NULL, vendor, product))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unable to find PCI device with vendor '%s' "
> +                             "product '%s'"), vendor, product);
> +            goto cleanup;
> +        }
> +
> +        break;
> +    default:
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("'parent' of scsi_host adapter must "
> +                         "be consistent with name of node device"));
> +        goto cleanup;
> +    }
> +
> +cleanup:
> +    virStringFreeList(tokens);
> +    return ret;
> +}
> +
>  static int
>  getHostNumber(const char *adapter_name,
>                unsigned int *result)
> @@ -627,10 +687,27 @@ static char *
>  getAdapterName(virStoragePoolSourceAdapter adapter)
>  {
>      char *name = NULL;
> +    char *addr = NULL;
>  
>      if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
> -        if (VIR_STRDUP(name, adapter.data.scsi_host.name) < 0)
> -            return NULL;
> +        if (adapter.data.scsi_host.parent) {
> +            if (!adapter.data.scsi_host.has_unique_id) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("'unique_id' is not specified for "
> +                                 "scsi_host adapter"));

so the optional unique_id isn't so optional after all for a couple of
code paths.  Issuing a message here could be confusing.  Has this been
tested within an environment where unique_id isn't specified?


> +                return NULL;
> +            } else {
> +                if (!(addr = getScsiHostParentAddress(adapter.data.scsi_host.parent)))
> +                    return NULL;
> +
> +                name = virParseStableScsiHostAddress(NULL, addr,
> +                                                     adapter.data.scsi_host.unique_id);

You need a VIR_FREE(addr); here

John
> +            }
> +        } else if (adapter.data.scsi_host.name) {
> +            if (VIR_STRDUP(name, adapter.data.scsi_host.name) < 0)
> +                return NULL;
> +        }
> +
>          return name;
>      }
>  
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]