All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crushtool: fix args parsing
@ 2011-09-22 16:39 Henry C Chang
  2011-09-22 16:45 ` Sage Weil
  0 siblings, 1 reply; 5+ messages in thread
From: Henry C Chang @ 2011-09-22 16:39 UTC (permalink / raw)
  To: ceph-devel; +Cc: Henry C Chang

Fix --add-item and --loc args parsing.

Signed-off-by: Henry C Chang <henry.cy.chang@gmail.com>
---
 src/crushtool.cc |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/src/crushtool.cc b/src/crushtool.cc
index 51531aa..f0ccb9b 100644
--- a/src/crushtool.cc
+++ b/src/crushtool.cc
@@ -872,9 +872,6 @@ int main(int argc, const char **argv)
       }
       if (i == args.end())
 	usage();
-      i = args.erase(i);
-      if (i == args.end())
-	usage();
       add_weight = atof(*i);
       i = args.erase(i);
       if (i == args.end())
@@ -885,9 +882,6 @@ int main(int argc, const char **argv)
       std::string type(val);
       if (i == args.end())
 	usage();
-      i = args.erase(i);
-      if (i == args.end())
-	usage();
       std::string name(*i);
       i = args.erase(i);
       add_loc[type] = name;
@@ -897,9 +891,6 @@ int main(int argc, const char **argv)
       reweight_name = val;
       if (i == args.end())
 	usage();
-      i = args.erase(i);
-      if (i == args.end())
-	usage();
       reweight_weight = atof(*i);
       i = args.erase(i);
     } else if (ceph_argparse_flag(args, i, "--build", (char*)NULL)) {
@@ -954,10 +945,8 @@ int main(int argc, const char **argv)
       int dev = tmp;
       if (i == args.end())
 	usage();
-      i = args.erase(i);
-      if (i == args.end())
-	usage();
       float f = atof(*i);
+      i = args.erase(i);
       int w = (int)(f * 0x10000);
       if (w < 0)
 	w = 0;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] crushtool: fix args parsing
  2011-09-22 16:39 [PATCH] crushtool: fix args parsing Henry C Chang
@ 2011-09-22 16:45 ` Sage Weil
  2011-09-23  3:21   ` Henry C Chang
  0 siblings, 1 reply; 5+ messages in thread
From: Sage Weil @ 2011-09-22 16:45 UTC (permalink / raw)
  To: Henry C Chang; +Cc: ceph-devel

Hey Henry-

Do you have a list of commands that were failing handy?  I'd like to 
extend the cli unit tests at the same time (src/test/cli/crushtool).

Thanks-
sage


On Fri, 23 Sep 2011, Henry C Chang wrote:

> Fix --add-item and --loc args parsing.
> 
> Signed-off-by: Henry C Chang <henry.cy.chang@gmail.com>
> ---
>  src/crushtool.cc |   13 +------------
>  1 files changed, 1 insertions(+), 12 deletions(-)
> 
> diff --git a/src/crushtool.cc b/src/crushtool.cc
> index 51531aa..f0ccb9b 100644
> --- a/src/crushtool.cc
> +++ b/src/crushtool.cc
> @@ -872,9 +872,6 @@ int main(int argc, const char **argv)
>        }
>        if (i == args.end())
>  	usage();
> -      i = args.erase(i);
> -      if (i == args.end())
> -	usage();
>        add_weight = atof(*i);
>        i = args.erase(i);
>        if (i == args.end())
> @@ -885,9 +882,6 @@ int main(int argc, const char **argv)
>        std::string type(val);
>        if (i == args.end())
>  	usage();
> -      i = args.erase(i);
> -      if (i == args.end())
> -	usage();
>        std::string name(*i);
>        i = args.erase(i);
>        add_loc[type] = name;
> @@ -897,9 +891,6 @@ int main(int argc, const char **argv)
>        reweight_name = val;
>        if (i == args.end())
>  	usage();
> -      i = args.erase(i);
> -      if (i == args.end())
> -	usage();
>        reweight_weight = atof(*i);
>        i = args.erase(i);
>      } else if (ceph_argparse_flag(args, i, "--build", (char*)NULL)) {
> @@ -954,10 +945,8 @@ int main(int argc, const char **argv)
>        int dev = tmp;
>        if (i == args.end())
>  	usage();
> -      i = args.erase(i);
> -      if (i == args.end())
> -	usage();
>        float f = atof(*i);
> +      i = args.erase(i);
>        int w = (int)(f * 0x10000);
>        if (w < 0)
>  	w = 0;
> -- 
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] crushtool: fix args parsing
  2011-09-22 16:45 ` Sage Weil
@ 2011-09-23  3:21   ` Henry C Chang
  2011-09-23  4:40     ` Sage Weil
  0 siblings, 1 reply; 5+ messages in thread
From: Henry C Chang @ 2011-09-23  3:21 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel

[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]

Hi Sage,

In fact, I got two problems with the crushtool after upgrading from
0.34 to 0.35.

1. crushtool failed to parse args.

I have a template crushmap (as attached) which only consists of the
root. For each disk, I will use the crushtool to add it to the
crushmap. The command series look like:

$ crushtool -i /etc/ceph/crush.template --add-item 0 1.0 device0 --loc
host host0 --loc cluster cluster0 -o /tmp/crush0
$ crushtool -i /tmp/crush0 --add-item 1 1.0 device1 --loc host host0
--loc cluster cluster0 -o /tmp/crush1


2. crushtool rejected the command due to the commit 58f98e7.

I inverted the commit when upgrading to v0.35.

Henry

2011/9/23 Sage Weil <sage@newdream.net>:
> Hey Henry-
>
> Do you have a list of commands that were failing handy?  I'd like to
> extend the cli unit tests at the same time (src/test/cli/crushtool).
>
> Thanks-
> sage
>
>
> On Fri, 23 Sep 2011, Henry C Chang wrote:
>
>> Fix --add-item and --loc args parsing.
>>
>> Signed-off-by: Henry C Chang <henry.cy.chang@gmail.com>
>> ---
>>  src/crushtool.cc |   13 +------------
>>  1 files changed, 1 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/crushtool.cc b/src/crushtool.cc
>> index 51531aa..f0ccb9b 100644
>> --- a/src/crushtool.cc
>> +++ b/src/crushtool.cc
>> @@ -872,9 +872,6 @@ int main(int argc, const char **argv)
>>        }
>>        if (i == args.end())
>>       usage();
>> -      i = args.erase(i);
>> -      if (i == args.end())
>> -     usage();
>>        add_weight = atof(*i);
>>        i = args.erase(i);
>>        if (i == args.end())
>> @@ -885,9 +882,6 @@ int main(int argc, const char **argv)
>>        std::string type(val);
>>        if (i == args.end())
>>       usage();
>> -      i = args.erase(i);
>> -      if (i == args.end())
>> -     usage();
>>        std::string name(*i);
>>        i = args.erase(i);
>>        add_loc[type] = name;
>> @@ -897,9 +891,6 @@ int main(int argc, const char **argv)
>>        reweight_name = val;
>>        if (i == args.end())
>>       usage();
>> -      i = args.erase(i);
>> -      if (i == args.end())
>> -     usage();
>>        reweight_weight = atof(*i);
>>        i = args.erase(i);
>>      } else if (ceph_argparse_flag(args, i, "--build", (char*)NULL)) {
>> @@ -954,10 +945,8 @@ int main(int argc, const char **argv)
>>        int dev = tmp;
>>        if (i == args.end())
>>       usage();
>> -      i = args.erase(i);
>> -      if (i == args.end())
>> -     usage();
>>        float f = atof(*i);
>> +      i = args.erase(i);
>>        int w = (int)(f * 0x10000);
>>        if (w < 0)
>>       w = 0;
>> --
>> 1.7.4.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>

[-- Attachment #2: crush.template --]
[-- Type: application/octet-stream, Size: 316 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] crushtool: fix args parsing
  2011-09-23  3:21   ` Henry C Chang
@ 2011-09-23  4:40     ` Sage Weil
  2011-09-23  5:07       ` Henry C Chang
  0 siblings, 1 reply; 5+ messages in thread
From: Sage Weil @ 2011-09-23  4:40 UTC (permalink / raw)
  To: Henry C Chang; +Cc: ceph-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3357 bytes --]

On Fri, 23 Sep 2011, Henry C Chang wrote:
> Hi Sage,
> 
> In fact, I got two problems with the crushtool after upgrading from
> 0.34 to 0.35.
> 
> 1. crushtool failed to parse args.
> 
> I have a template crushmap (as attached) which only consists of the
> root. For each disk, I will use the crushtool to add it to the
> crushmap. The command series look like:
> 
> $ crushtool -i /etc/ceph/crush.template --add-item 0 1.0 device0 --loc
> host host0 --loc cluster cluster0 -o /tmp/crush0
> $ crushtool -i /tmp/crush0 --add-item 1 1.0 device1 --loc host host0
> --loc cluster cluster0 -o /tmp/crush1
> 
> 
> 2. crushtool rejected the command due to the commit 58f98e7.
> 
> I inverted the commit when upgrading to v0.35.

Yep, those were both broken.  Fixed it in the stable branch, and added a 
simple unit test to make sure it stays that way.  Please let me know it if 
gives you any further problems.

Thanks!
sage


> 
> Henry
> 
> 2011/9/23 Sage Weil <sage@newdream.net>:
> > Hey Henry-
> >
> > Do you have a list of commands that were failing handy?  I'd like to
> > extend the cli unit tests at the same time (src/test/cli/crushtool).
> >
> > Thanks-
> > sage
> >
> >
> > On Fri, 23 Sep 2011, Henry C Chang wrote:
> >
> >> Fix --add-item and --loc args parsing.
> >>
> >> Signed-off-by: Henry C Chang <henry.cy.chang@gmail.com>
> >> ---
> >>  src/crushtool.cc |   13 +------------
> >>  1 files changed, 1 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/src/crushtool.cc b/src/crushtool.cc
> >> index 51531aa..f0ccb9b 100644
> >> --- a/src/crushtool.cc
> >> +++ b/src/crushtool.cc
> >> @@ -872,9 +872,6 @@ int main(int argc, const char **argv)
> >>        }
> >>        if (i == args.end())
> >>       usage();
> >> -      i = args.erase(i);
> >> -      if (i == args.end())
> >> -     usage();
> >>        add_weight = atof(*i);
> >>        i = args.erase(i);
> >>        if (i == args.end())
> >> @@ -885,9 +882,6 @@ int main(int argc, const char **argv)
> >>        std::string type(val);
> >>        if (i == args.end())
> >>       usage();
> >> -      i = args.erase(i);
> >> -      if (i == args.end())
> >> -     usage();
> >>        std::string name(*i);
> >>        i = args.erase(i);
> >>        add_loc[type] = name;
> >> @@ -897,9 +891,6 @@ int main(int argc, const char **argv)
> >>        reweight_name = val;
> >>        if (i == args.end())
> >>       usage();
> >> -      i = args.erase(i);
> >> -      if (i == args.end())
> >> -     usage();
> >>        reweight_weight = atof(*i);
> >>        i = args.erase(i);
> >>      } else if (ceph_argparse_flag(args, i, "--build", (char*)NULL)) {
> >> @@ -954,10 +945,8 @@ int main(int argc, const char **argv)
> >>        int dev = tmp;
> >>        if (i == args.end())
> >>       usage();
> >> -      i = args.erase(i);
> >> -      if (i == args.end())
> >> -     usage();
> >>        float f = atof(*i);
> >> +      i = args.erase(i);
> >>        int w = (int)(f * 0x10000);
> >>        if (w < 0)
> >>       w = 0;
> >> --
> >> 1.7.4.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] crushtool: fix args parsing
  2011-09-23  4:40     ` Sage Weil
@ 2011-09-23  5:07       ` Henry C Chang
  0 siblings, 0 replies; 5+ messages in thread
From: Henry C Chang @ 2011-09-23  5:07 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel

> Yep, those were both broken.  Fixed it in the stable branch, and added a
> simple unit test to make sure it stays that way.  Please let me know it if
> gives you any further problems.
>

I can't see your commit on github. Have you pushed it on?

Henry
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-09-23  5:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-22 16:39 [PATCH] crushtool: fix args parsing Henry C Chang
2011-09-22 16:45 ` Sage Weil
2011-09-23  3:21   ` Henry C Chang
2011-09-23  4:40     ` Sage Weil
2011-09-23  5:07       ` Henry C Chang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.