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

Re: [libvirt] [PATCH 2/3] node device: Break out get_wwns and get_parent_node helpers



On 10/20/2009 11:55 AM, Dave Allan wrote:
> Cole Robinson wrote:
>> These will be used by the test driver, so move them to a shareable space.
>>
>> Signed-off-by: Cole Robinson <crobinso redhat com>
>> ---
>>  src/conf/node_device_conf.c          |   89 +++++++++++++++++++++++++++
>>  src/conf/node_device_conf.h          |   11 +++
>>  src/libvirt_private.syms             |    2 +
>>  src/node_device/node_device_driver.c |  112 ++++------------------------------
>>  4 files changed, 115 insertions(+), 99 deletions(-)
>>
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index f09f814..77f7be3 100644
>> --- a/src/conf/node_device_conf.c
>> +++ b/src/conf/node_device_conf.c
>> @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn,
>>      return virNodeDeviceDefParse(conn, NULL, filename, create);
>>  }
>>  
>> +/*
>> + * Return fc_host dev's WWNN and WWPN
>> + */
>> +int
>> +virNodeDeviceGetWWNs(virConnectPtr conn,
>> +                     virNodeDeviceDefPtr def,
>> +                     char **wwnn,
>> +                     char **wwpn)
>> +{
>> +    virNodeDevCapsDefPtr cap = NULL;
>> +    int ret = 0;
>> +
>> +    cap = def->caps;
>> +    while (cap != NULL) {
>> +        if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST &&
>> +            cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
>> +            *wwnn = strdup(cap->data.scsi_host.wwnn);
>> +            *wwpn = strdup(cap->data.scsi_host.wwpn);
>> +            break;
>> +        }
>> +
>> +        cap = cap->next;
>> +    }
>> +
>> +    if (cap == NULL) {
>> +        virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT,
>> +                                 "%s", _("Device is not a fibre channel HBA"));
>> +        ret = -1;
>> +    }
>> +
>> +    if (*wwnn == NULL || *wwpn == NULL) {
>> +        /* Free the other one, if allocated... */
>> +        VIR_FREE(wwnn);
>> +        VIR_FREE(wwpn);
>> +        ret = -1;
>> +        virReportOOMError(conn);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +/*
>> + * Return the NPIV dev's parent device name
>> + */
>> +int
>> +virNodeDeviceGetParentHost(virConnectPtr conn,
>> +                           const virNodeDeviceObjListPtr devs,
>> +                           const char *dev_name,
>> +                           const char *parent_name,
>> +                           int *parent_host)
>> +{
>> +    virNodeDeviceObjPtr parent = NULL;
>> +    virNodeDevCapsDefPtr cap = NULL;
>> +    int ret = 0;
>> +
>> +    parent = virNodeDeviceFindByName(devs, parent_name);
>> +    if (parent == NULL) {
>> +        virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR,
>> +                                 _("Could not find parent HBA for '%s'"),
>> +                                 dev_name);
>> +        ret = -1;
>> +        goto out;
>> +    }
>> +
>> +    cap = parent->def->caps;
>> +    while (cap != NULL) {
>> +        if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST &&
>> +            (cap->data.scsi_host.flags &
>> +             VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) {
>> +                *parent_host = cap->data.scsi_host.host;
>> +                break;
>> +        }
>> +
>> +        cap = cap->next;
>> +    }
>> +
>> +    if (cap == NULL) {
>> +        virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR,
>> +                                 _("Parent HBA %s is not capable "
>> +                                   "of vport operations"),
>> +                                 parent->def->name);
>> +        ret = -1;
>> +    }
>> +
>> +    virNodeDeviceObjUnlock(parent);
>> +
>> +out:
>> +    return ret;
>> +}
>>  
>>  void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
>>  {
>> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
>> index 29a4d43..a7bb6c6 100644
>> --- a/src/conf/node_device_conf.h
>> +++ b/src/conf/node_device_conf.h
>> @@ -221,6 +221,17 @@ virNodeDeviceDefPtr virNodeDeviceDefParseNode(virConnectPtr conn,
>>                                                xmlNodePtr root,
>>                                                int create);
>>  
>> +int virNodeDeviceGetWWNs(virConnectPtr conn,
>> +                         virNodeDeviceDefPtr def,
>> +                         char **wwnn,
>> +                         char **wwpn);
>> +
>> +int virNodeDeviceGetParentHost(virConnectPtr conn,
>> +                               const virNodeDeviceObjListPtr devs,
>> +                               const char *dev_name,
>> +                               const char *parent_name,
>> +                               int *parent_host);
>> +
>>  void virNodeDeviceDefFree(virNodeDeviceDefPtr def);
>>  
>>  void virNodeDeviceObjFree(virNodeDeviceObjPtr dev);
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 37395ab..45d1069 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -318,6 +318,8 @@ virNodeDeviceDefParseString;
>>  virNodeDeviceObjLock;
>>  virNodeDeviceObjUnlock;
>>  virNodeDeviceAssignDef;
>> +virNodeDeviceGetWWNs;
>> +virNodeDeviceGetParentHost;
>>  
>>  
>>  # pci.h
>> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
>> index 21a4c8d..f33ff48 100644
>> --- a/src/node_device/node_device_driver.c
>> +++ b/src/node_device/node_device_driver.c
>> @@ -459,92 +459,6 @@ cleanup:
>>  
>>  
>>  static int
>> -get_wwns(virConnectPtr conn,
>> -         virNodeDeviceDefPtr def,
>> -         char **wwnn,
>> -         char **wwpn)
>> -{
>> -    virNodeDevCapsDefPtr cap = NULL;
>> -    int ret = 0;
>> -
>> -    cap = def->caps;
>> -    while (cap != NULL) {
>> -        if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST &&
>> -            cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
>> -            *wwnn = strdup(cap->data.scsi_host.wwnn);
>> -            *wwpn = strdup(cap->data.scsi_host.wwpn);
>> -            break;
>> -        }
>> -
>> -        cap = cap->next;
>> -    }
>> -
>> -    if (cap == NULL) {
>> -        virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT,
>> -                                 "%s", _("Device is not a fibre channel HBA"));
>> -        ret = -1;
>> -    }
>> -
>> -    if (*wwnn == NULL || *wwpn == NULL) {
>> -        /* Free the other one, if allocated... */
>> -        VIR_FREE(wwnn);
>> -        VIR_FREE(wwpn);
>> -        ret = -1;
>> -        virReportOOMError(conn);
>> -    }
>> -
>> -    return ret;
>> -}
>> -
>> -
>> -static int
>> -get_parent_host(virConnectPtr conn,
>> -                virDeviceMonitorStatePtr driver,
>> -                const char *dev_name,
>> -                const char *parent_name,
>> -                int *parent_host)
>> -{
>> -    virNodeDeviceObjPtr parent = NULL;
>> -    virNodeDevCapsDefPtr cap = NULL;
>> -    int ret = 0;
>> -
>> -    parent = virNodeDeviceFindByName(&driver->devs, parent_name);
>> -    if (parent == NULL) {
>> -        virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR,
>> -                                 _("Could not find parent HBA for '%s'"),
>> -                                 dev_name);
>> -        ret = -1;
>> -        goto out;
>> -    }
>> -
>> -    cap = parent->def->caps;
>> -    while (cap != NULL) {
>> -        if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST &&
>> -            (cap->data.scsi_host.flags &
>> -             VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) {
>> -                *parent_host = cap->data.scsi_host.host;
>> -                break;
>> -        }
>> -
>> -        cap = cap->next;
>> -    }
>> -
>> -    if (cap == NULL) {
>> -        virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR,
>> -                                 _("Parent HBA %s is not capable "
>> -                                   "of vport operations"),
>> -                                 parent->def->name);
>> -        ret = -1;
>> -    }
>> -
>> -    virNodeDeviceObjUnlock(parent);
>> -
>> -out:
>> -    return ret;
>> -}
>> -
>> -
>> -static int
>>  get_time(virConnectPtr conn, time_t *t)
>>  {
>>      int ret = 0;
>> @@ -630,15 +544,15 @@ nodeDeviceCreateXML(virConnectPtr conn,
>>          goto cleanup;
>>      }
>>  
>> -    if (get_wwns(conn, def, &wwnn, &wwpn) == -1) {
>> +    if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) {
>>          goto cleanup;
>>      }
>>  
>> -    if (get_parent_host(conn,
>> -                        driver,
>> -                        def->name,
>> -                        def->parent,
>> -                        &parent_host) == -1) {
>> +    if (virNodeDeviceGetParentHost(conn,
>> +                                   &driver->devs,
>> +                                   def->name,
>> +                                   def->parent,
>> +                                   &parent_host) == -1) {
>>          goto cleanup;
>>      }
>>  
>> @@ -685,13 +599,13 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
>>          goto out;
>>      }
>>  
>> -    if (get_wwns(dev->conn, obj->def, &wwnn, &wwpn) == -1) {
>> +    if (virNodeDeviceGetWWNs(dev->conn, obj->def, &wwnn, &wwpn) == -1) {
>>          goto out;
>>      }
>>  
>>      parent_name = strdup(obj->def->parent);
>>  
>> -    /* get_parent_host will cause the device object's lock to be
>> +    /* virNodeDeviceGetParentHost will cause the device object's lock to be
>>       * taken, so we have to dup the parent's name and drop the lock
>>       * before calling it.  We don't need the reference to the object
>>       * any more once we have the parent's name.  */
>> @@ -703,11 +617,11 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
>>          goto out;
>>      }
>>  
>> -    if (get_parent_host(dev->conn,
>> -                        driver,
>> -                        dev->name,
>> -                        parent_name,
>> -                        &parent_host) == -1) {
>> +    if (virNodeDeviceGetParentHost(dev->conn,
>> +                                   &driver->devs,
>> +                                   dev->name,
>> +                                   parent_name,
>> +                                   &parent_host) == -1) {
>>          goto out;
>>      }
>>  
> 
> Ack
> 
> Dave
> 

Thanks, I've pushed this now.

- Cole


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