[libvirt] [PATCH 6/9] nodedev: Dump max vports and vports in use for HBA's XML

[Osier Yang]

On 2013年01月15日 03:42, Michal Privoznik wrote:
> On 14.01.2013 15:34, Osier Yang wrote:
>> This enrichs HBA's xml by dumping the number of max vports and
>> vports in use. Format is like:
>>
>>    <capability type='vport_ops'>
>>      <max_vports>164</max_vports>
>>      <vports>5</vports>
>>    </capability>
>>
>> * docs/formatnode.html.in: (Document the new XML)
>> * docs/schemas/nodedev.rng: (Add the schema)
>> * src/conf/node_device_conf.h: (New member for data.scsi_host)
>> * src/node_device/node_device_linux_sysfs.c: (Collect the value of
>>    max_vports and vports)
>> ---
>>   docs/formatnode.html.in                   |   10 ++++--
>>   docs/schemas/nodedev.rng                  |    6 +++
>>   src/conf/node_device_conf.c               |    7 +++-
>>   src/conf/node_device_conf.h               |    2 +
>>   src/node_device/node_device_linux_sysfs.c |   48 ++++++++++++++++++++++++++--
>>   5 files changed, 65 insertions(+), 8 deletions(-)
>
>> diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
>> index 054afea..b498726 100644
>> --- a/src/node_device/node_device_linux_sysfs.c
>> +++ b/src/node_device/node_device_linux_sysfs.c
>> @@ -40,6 +40,8 @@
>>   int
>>   detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
>>   {
>> +    char *max_vports = NULL;
>> +    char *vports = NULL;
>>       int ret = -1;
>>
>>       VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host);
>> @@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
>>           if (virReadFCHost(NULL,
>>                             d->scsi_host.host,
>>                             "port_name",
>> -&d->scsi_host.wwpn) == -1) {
>> +&d->scsi_host.wwpn)<  0) {
>>               VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host);
>>               goto cleanup;
>>           }
>> @@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
>>           if (virReadFCHost(NULL,
>>                             d->scsi_host.host,
>>                             "node_name",
>> -&d->scsi_host.wwnn) == -1) {
>> +&d->scsi_host.wwnn)<  0) {
>>               VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host);
>>               goto cleanup;
>>           }
>> @@ -66,23 +68,61 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
>>           if (virReadFCHost(NULL,
>>                             d->scsi_host.host,
>>                             "fabric_name",
>> -&d->scsi_host.fabric_wwn) == -1) {
>> +&d->scsi_host.fabric_wwn)<  0) {
>>               VIR_ERROR(_("Failed to read fabric WWN for host%d"),
>>                         d->scsi_host.host);
>>               goto cleanup;
>>           }
>>       }
>>
>> -    if (virIsCapableVport(NULL, d->scsi_host.host) == 0)
>> +    if (virIsCapableVport(NULL, d->scsi_host.host) == 0) {
>>           d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
>>
>> +        if (virReadFCHost(NULL,
>> +                          d->scsi_host.max_vports,
>> +                          "max_npiv_vports",
>> +&max_vports)<  0) {
>> +            VIR_ERROR(_("Failed to read max_npiv_vports for host%d"),
>> +                      d->scsi_host.host);
>> +            goto cleanup;
>> +        }
>> +
>> +         if (virReadFCHost(NULL,
>> +                          d->scsi_host.max_vports,
>> +                          "npiv_vports_inuse",
>> +&vports)<  0) {
>> +            VIR_ERROR(_("Failed to read npiv_vports_inuse for host%d"),
>> +                      d->scsi_host.host);
>> +            goto cleanup;
>> +        }
>> +
>> +        if (virStrToLong_i(max_vports, NULL, 10,
>> +&d->scsi_host.max_vports)<  0) {
>> +            VIR_ERROR(_("Failed to parse value of max_npiv_vports '%s'"),
>> +                      max_vports);
>> +            goto cleanup;
>> +        }
>> +
>> +        if (virStrToLong_i(vports, NULL, 10,
>> +&d->scsi_host.vports)<  0) {
>> +            VIR_ERROR(_("Failed to parse value of npiv_vports_inuse '%s'"),
>> +                      vports);
>> +            goto cleanup;
>> +        }
>> +    }
>> +
>>       ret = 0;
>>   cleanup:
>>       if (ret == -1) {
>
> And again. I'd rewrite this condition as well. But you don't have to.
>
>> +        /* Clear the flags in case of producing confusing XML output */
>> +        d->scsi_host.flags = 0;
>
> I don't think so. Shouldn't we be just clearing those two flags and
> don't touch the others?

Agreed. Though there are only the two flags. I will post a v2.

>
>> +
>>           VIR_FREE(d->scsi_host.wwnn);
>>           VIR_FREE(d->scsi_host.wwpn);
>>           VIR_FREE(d->scsi_host.fabric_wwn);
>>       }
>> +    VIR_FREE(max_vports);
>> +    VIR_FREE(vports);
>>       return ret;
>>   }
>>
>>
>
> Michal