On 05/06/2013 08:45 AM, Osier Yang wrote: > The helper works for default sysfs_prefix, but for user specified > prefix, it doesn't work. (Detected when writing test cases. A later > patch will add the test cases for fc_host). > --- > src/util/virutil.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > Seems right, although it does make me wonder why the choice was made initially to use NULL instead... That is could some other assumption now be invalidated? Furthermore, are the calls from detect_scsi_host_caps() still valid with a NULL? If NULL ends up being an invalid value completely, then you should also make the change to add the ATTRIBUTE_NONNULL(1) to the definition. Should you "separate" the virIsCapableVport and virReadFCHost changes? John > diff --git a/src/util/virutil.c b/src/util/virutil.c > index c83996d..2373f1f 100644 > --- a/src/util/virutil.c > +++ b/src/util/virutil.c > @@ -3367,7 +3367,8 @@ cleanup: > /* virFindFCHostCapableVport: > * > * Iterate over the sysfs and find out the first online HBA which > - * supports vport, and not saturated,. > + * supports vport, and not saturated. Returns the host name (e.g. > + * host5) on success, or NULL on failure. > */ > char * > virFindFCHostCapableVport(const char *sysfs_prefix) > @@ -3401,10 +3402,10 @@ virFindFCHostCapableVport(const char *sysfs_prefix) > continue; > } > > - if (!virIsCapableVport(NULL, host)) > + if (!virIsCapableVport(prefix, host)) > continue; > > - if (virReadFCHost(NULL, host, "port_state", &state) < 0) { > + if (virReadFCHost(prefix, host, "port_state", &state) < 0) { > VIR_DEBUG("Failed to read port_state for host%d", host); > continue; > } > @@ -3416,12 +3417,12 @@ virFindFCHostCapableVport(const char *sysfs_prefix) > } > VIR_FREE(state); > > - if (virReadFCHost(NULL, host, "max_npiv_vports", &max_vports) < 0) { > + if (virReadFCHost(prefix, host, "max_npiv_vports", &max_vports) < 0) { > VIR_DEBUG("Failed to read max_npiv_vports for host%d", host); > continue; > } > > - if (virReadFCHost(NULL, host, "npiv_vports_inuse", &vports) < 0) { > + if (virReadFCHost(prefix, host, "npiv_vports_inuse", &vports) < 0) { > VIR_DEBUG("Failed to read npiv_vports_inuse for host%d", host); > VIR_FREE(max_vports); > continue; > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list