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

Re: [libvirt] [PATCH] npiv: Expose fabric_name outside



On Tue, Dec 06, 2011 at 08:42:18PM +0800, Osier Yang wrote:
> This patch is to expose the fabric_name of fc_host class, which
> might be useful for users who wants to known which fabric the
> (v)HBA connects to.
> 
> The patch also adds the missed capabilities' XML schema of scsi_host,
> (of course, with fabric_wwn added), and update the documents
> (docs/formatnode.html.in)
> ---
>  docs/formatnode.html.in                   |    7 +++++
>  docs/schemas/nodedev.rng                  |   40 +++++++++++++++++++++++++++++
>  src/conf/node_device_conf.c               |    3 ++
>  src/conf/node_device_conf.h               |    1 +
>  src/node_device/node_device_linux_sysfs.c |   10 +++++++
>  5 files changed, 61 insertions(+), 0 deletions(-)
> 
> diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
> index 126f8de..c04d04d 100644
> --- a/docs/formatnode.html.in
> +++ b/docs/formatnode.html.in
> @@ -126,6 +126,7 @@
>                <dd>A network protocol exposed by the device, where the
>                  attribute <code>type</code> can be "80203" for IEEE
>                  802.3, or "80211" for various flavors of IEEE 802.11.
> +              </dd>
>              </dl>
>            </dd>
>            <dt><code>scsi_host</code></dt>
> @@ -133,6 +134,12 @@
>              <dl>
>                <dt><code>host</code></dt>
>                <dd>The SCSI host number.</dd>
> +              <dt><code>capability</code></dt>
> +              <dd>Current capabilities include "vports_ops" (indicates
> +                vport operations are supported) and "fc_host", the later
> +                implies following sub-elements: <code>wwnn</code>,
> +                <code>wwpn</code>, <code>fabric_wwn</code>.
> +              </dd>
>              </dl>
>            </dd>
>            <dt><code>scsi</code></dt>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 55191d9..1b9a2d1 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -216,6 +216,35 @@
>      </attribute>
>    </define>
>  
> +  <define name='wwn'>
> +    <data type='string'>
> +      <param name='pattern'>(0-9a-fA-F){16}</param>
> +    </data>
> +  </define>
> +
> +  <define name='capsfchost'>
> +    <attribute name='type'>
> +      <value>fc_host</value>
> +    </attribute>
> +
> +    <element name='wwnn'>
> +      <ref name='wwn'/>
> +    </element>
> +
> +    <element name='wwpn'>
> +      <ref name='wwn'/>
> +    </element>
> +
> +    <element name='fabric_wwn'>
> +      <ref name='wwn'/>
> +    </element>
> +  </define>
> +
> +  <define name='capsvports'>
> +    <attribute name='type'>
> +      <value>vports_ops</value>
> +    </attribute>
> +  </define>
>  
>    <define name='capscsihost'>
>      <attribute name='type'>
> @@ -225,6 +254,17 @@
>      <element name='host'>
>        <ref name='uint'/>
>      </element>
> +
> +    <optional>
> +      <zeroOrMore>
> +        <element name='capability'>
> +          <choice>
> +            <ref name='capsfchost'/>
> +            <ref name='capsvports'/>
> +          </choice>
> +        </element>
> +      </zeroOrMore>
> +    </optional>
>    </define>
>  
>    <define name='capscsi'>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 9d48ff8..ea6ebde 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -389,6 +389,8 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
>                                        data->scsi_host.wwnn);
>                  virBufferEscapeString(&buf, "      <wwpn>%s</wwpn>\n",
>                                        data->scsi_host.wwpn);
> +                virBufferEscapeString(&buf, "      <fabric_wwn>%s</fabric_wwn>\n",
> +                                      data->scsi_host.fabric_wwn);
>                  virBufferAddLit(&buf, "    </capability>\n");
>              }
>              if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) {
> @@ -1405,6 +1407,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
>      case VIR_NODE_DEV_CAP_SCSI_HOST:
>          VIR_FREE(data->scsi_host.wwnn);
>          VIR_FREE(data->scsi_host.wwpn);
> +        VIR_FREE(data->scsi_host.fabric_wwn);
>          break;
>      case VIR_NODE_DEV_CAP_SCSI_TARGET:
>          VIR_FREE(data->scsi_target.name);
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index e317354..e787fc7 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -141,6 +141,7 @@ struct _virNodeDevCapsDef {
>              unsigned int host;
>              char *wwnn;
>              char *wwpn;
> +            char *fabric_wwn;
>              unsigned int flags;
>          } scsi_host;
>          struct {
> diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
> index d352800..380be9c 100644
> --- a/src/node_device/node_device_linux_sysfs.c
> +++ b/src/node_device/node_device_linux_sysfs.c
> @@ -149,10 +149,20 @@ int check_fc_host_linux(union _virNodeDevCapData *d)
>          retval = -1;
>      }
>  
> +    if (read_wwn(d->scsi_host.host,
> +                 "fabric_name",
> +                 &d->scsi_host.fabric_wwn) == -1) {
> +        VIR_ERROR(_("Failed to read fabric WWN for host%d"),
> +                  d->scsi_host.host);
> +        retval = -1;

Not your fault, since you're just following existing practice, but the
error reporting in these methods seems a little odd, using VIR_ERROR
instead of virReportError. Presumably because these codepaths are only
ever run in daemon context, not in response to a user API. These days
though the daemon code also uses virReportError, so some day we might
like to cleanup this code.

> +        goto out;
> +    }
> +
>  out:
>      if (retval == -1) {
>          VIR_FREE(d->scsi_host.wwnn);
>          VIR_FREE(d->scsi_host.wwpn);
> +        VIR_FREE(d->scsi_host.fabric_wwn);
>      }
>      VIR_FREE(sysfs_path);
>      return retval;

ACK.  The error cleanup can wait till later


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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