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

[Osier Yang <jyang@xxxxxxxxxx>]

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list