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

Re: [libvirt] [PATCH 6/7] util: Honor the passed sysfs_prefix



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;
> 


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