[PATCH] pnfsd: Add ability to clear pnfs dlm device ds list

[PATCH] pnfsd: Add ability to clear pnfs dlm device ds list

[Eric Anderle]

Added the ability to clear the pnfs dlm device ds list. Before, we
checked to make sure the list wasn't empty; this is accomplished by
examining the character after the ':' in the passed-in string. By
modifying this check, an empty list is considered valid, and everything
just works.

---
 fs/nfsd/nfs4pnfsdlm.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
index 40f9b84..befec4f 100644
--- a/fs/nfsd/nfs4pnfsdlm.c
+++ b/fs/nfsd/nfs4pnfsdlm.c
@@ -160,17 +160,15 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device,
int len)
 
        err = -EINVAL;
        bufp += len + 1;
-       if (bufp >= endp)
+       if (bufp > endp)
                goto out_free;
 
        /* data server list */
-       /* FIXME: need to check for comma separated valid ip format */
        len = strcspn(bufp, ":");
        if (len > NFSD_DLM_DS_LIST_MAX)
                goto out_free;
        memcpy(new->ds_list, bufp, len);
 
-
        /*  validate the ips */
        if (!nfsd4_validate_pnfs_dlm_device(new->ds_list,
&(new->num_ds)))
                goto out_free;
-- 
1.7.0.4

Re: [PATCH] pnfsd: Add ability to clear pnfs dlm device ds list

[Benny Halevy]

On Jun. 17, 2010, 15:53 -0400, Eric Anderle <eanderle@umich.edu> wrote:
> Added the ability to clear the pnfs dlm device ds list. Before, we
> checked to make sure the list wasn't empty; this is accomplished by
> examining the character after the ':' in the passed-in string. By
> modifying this check, an empty list is considered valid, and everything
> just works.
> 
> ---
>  fs/nfsd/nfs4pnfsdlm.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> index 40f9b84..befec4f 100644
> --- a/fs/nfsd/nfs4pnfsdlm.c
> +++ b/fs/nfsd/nfs4pnfsdlm.c
> @@ -160,17 +160,15 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device,
> int len)
>  
>         err = -EINVAL;
>         bufp += len + 1;
> -       if (bufp >= endp)
> +       if (bufp > endp)
>                 goto out_free;
>  
>         /* data server list */ and 
> -       /* FIXME: need to check for comma separated valid ip format */
>         len = strcspn(bufp, ":");
>         if (len > NFSD_DLM_DS_LIST_MAX)
>                 goto out_free;
>         memcpy(new->ds_list, bufp, len);

BTW, what about null termination in case len == NFSD_DLM_DS_LIST_MAX?
and same for new->disk_name and DISK_NAME_LEN

Benny

>  
> -
>         /*  validate the ips */
>         if (!nfsd4_validate_pnfs_dlm_device(new->ds_list,
> &(new->num_ds)))
>                 goto out_free;

Re: [PATCH] pnfsd: Add ability to clear pnfs dlm device ds list

[Benny Halevy]

On Jun. 17, 2010, 15:53 -0400, Eric Anderle <eanderle@umich.edu> wrote:
> Added the ability to clear the pnfs dlm device ds list. Before, we
> checked to make sure the list wasn't empty; this is accomplished by
> examining the character after the ':' in the passed-in string. By
> modifying this check, an empty list is considered valid, and everything
> just works.
> 

Eric, a couple technical nits:
One, please sign-off your patches.
This is the fact convention that the author also signs off his/her
own patches.

> ---
>  fs/nfsd/nfs4pnfsdlm.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> index 40f9b84..befec4f 100644
> --- a/fs/nfsd/nfs4pnfsdlm.c
> +++ b/fs/nfsd/nfs4pnfsdlm.c
> @@ -160,17 +160,15 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device,
> int len)
>  
>         err = -EINVAL;
>         bufp += len + 1;
> -       if (bufp >= endp)
> +       if (bufp > endp)

Second, please make sure not to convert tabs to spaces...

Thanks!

Benny

>                 goto out_free;
>  
>         /* data server list */
> -       /* FIXME: need to check for comma separated valid ip format */
>         len = strcspn(bufp, ":");
>         if (len > NFSD_DLM_DS_LIST_MAX)
>                 goto out_free;
>         memcpy(new->ds_list, bufp, len);
>  
> -
>         /*  validate the ips */
>         if (!nfsd4_validate_pnfs_dlm_device(new->ds_list,
> &(new->num_ds)))
>                 goto out_free;

Re: [PATCH] pnfsd: Add ability to clear pnfs dlm device ds list

[Benny Halevy]

On Jun. 17, 2010, 17:06 -0400, Benny Halevy <bhalevy@panasas.com> wrote:
> On Jun. 17, 2010, 15:53 -0400, Eric Anderle <eanderle@umich.edu> wrote:
>> Added the ability to clear the pnfs dlm device ds list. Before, we
>> checked to make sure the list wasn't empty; this is accomplished by
>> examining the character after the ':' in the passed-in string. By
>> modifying this check, an empty list is considered valid, and everything
>> just works.
>>
> 
> Eric, a couple technical nits:
> One, please sign-off your patches.
> This is the fact convention that the author also signs off his/her
> own patches.
> 
>> ---
>>  fs/nfsd/nfs4pnfsdlm.c |    4 +---
>>  1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
>> index 40f9b84..befec4f 100644
>> --- a/fs/nfsd/nfs4pnfsdlm.c
>> +++ b/fs/nfsd/nfs4pnfsdlm.c
>> @@ -160,17 +160,15 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device,
>> int len)
>>  
>>         err = -EINVAL;
>>         bufp += len + 1;
>> -       if (bufp >= endp)
>> +       if (bufp > endp)
> 
> Second, please make sure not to convert tabs to spaces...
> 
> Thanks!
> 
> Benny
> 
>>                 goto out_free;
>>  
>>         /* data server list */
>> -       /* FIXME: need to check for comma separated valid ip format */
>>         len = strcspn(bufp, ":");
>>         if (len > NFSD_DLM_DS_LIST_MAX)
>>                 goto out_free;
>>         memcpy(new->ds_list, bufp, len);
>>  
>> -
>>         /*  validate the ips */
>>         if (!nfsd4_validate_pnfs_dlm_device(new->ds_list,
>> &(new->num_ds)))

Oh, and beware of long line wrapping...

>>                 goto out_free;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pnfsd: Add ability to clear pnfs dlm device ds list

[J. Bruce Fields]

On Thu, Jun 17, 2010 at 05:07:29PM -0400, Benny Halevy wrote:
> On Jun. 17, 2010, 17:06 -0400, Benny Halevy <bhalevy@panasas.com> wrote:
> > On Jun. 17, 2010, 15:53 -0400, Eric Anderle <eanderle@umich.edu> wrote:
> >> Added the ability to clear the pnfs dlm device ds list. Before, we
> >> checked to make sure the list wasn't empty; this is accomplished by
> >> examining the character after the ':' in the passed-in string. By
> >> modifying this check, an empty list is considered valid, and everything
> >> just works.
> >>
> > 
> > Eric, a couple technical nits:
> > One, please sign-off your patches.
> > This is the fact convention that the author also signs off his/her
> > own patches.

Note: see "120 Sign your work" under Documentation/SubmittingPatches to
understand what you're agreeing to.  (Basically, it's just certifying
that the work you're submitting is really yours.)

> > 
> >> ---
> >>  fs/nfsd/nfs4pnfsdlm.c |    4 +---
> >>  1 files changed, 1 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> >> index 40f9b84..befec4f 100644
> >> --- a/fs/nfsd/nfs4pnfsdlm.c
> >> +++ b/fs/nfsd/nfs4pnfsdlm.c
> >> @@ -160,17 +160,15 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device,
> >> int len)
> >>  
> >>         err = -EINVAL;
> >>         bufp += len + 1;
> >> -       if (bufp >= endp)
> >> +       if (bufp > endp)
> > 
> > Second, please make sure not to convert tabs to spaces...
...
> 
> Oh, and beware of long line wrapping...

The instructions for Evolution in Documentation/email-clients.txt may
help.

--b.