| Message ID | 1313162460-14397-1-git-send-email-levinsasha928@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 12, 2011 at 06:20:54PM +0300, Sasha Levin wrote: >Signed-off-by: Sasha Levin <levinsasha928@gmail.com> >--- > tools/kvm/kvm-cmd.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > >diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c >index e85f22f..3a90d6d 100644 >--- a/tools/kvm/kvm-cmd.c >+++ b/tools/kvm/kvm-cmd.c >@@ -24,6 +24,7 @@ struct cmd_struct kvm_commands[] = { > { "balloon", kvm_cmd_balloon, NULL, 0 }, > { "list", kvm_cmd_list, NULL, 0 }, > { "version", kvm_cmd_version, NULL, 0 }, >+ { "--version", kvm_cmd_version, NULL, 0 }, Although it works, I think it's not good way to implement a option as a command. walimis > { "stop", kvm_cmd_stop, NULL, 0 }, > { "help", kvm_cmd_help, NULL, 0 }, > { "run", kvm_cmd_run, kvm_run_help, 0 }, >-- >1.7.6 > >-- >To unsubscribe from this list: send the line "unsubscribe kvm" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 12 Aug 2011, walimis wrote: >> @@ -24,6 +24,7 @@ struct cmd_struct kvm_commands[] = { >> { "balloon", kvm_cmd_balloon, NULL, 0 }, >> { "list", kvm_cmd_list, NULL, 0 }, >> { "version", kvm_cmd_version, NULL, 0 }, >> + { "--version", kvm_cmd_version, NULL, 0 }, > Although it works, I think it's not good way to implement a option > as a command. Well, I already merged the patch because the whole series was so super-cool. What downsides are there to implementing the alias like this? Pekka -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 12, 2011 at 06:39:46PM +0300, Pekka Enberg wrote: >On Fri, 12 Aug 2011, walimis wrote: >>>@@ -24,6 +24,7 @@ struct cmd_struct kvm_commands[] = { >>> { "balloon", kvm_cmd_balloon, NULL, 0 }, >>> { "list", kvm_cmd_list, NULL, 0 }, >>> { "version", kvm_cmd_version, NULL, 0 }, >>>+ { "--version", kvm_cmd_version, NULL, 0 }, >>Although it works, I think it's not good way to implement a option >>as a command. > >Well, I already merged the patch because the whole series was so >super-cool. What downsides are there to implementing the alias like >this? no downsides, I just consider that a option is preceded with "--", but the struct cmd_struct is to describe a command. walimis > > Pekka -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 12, 2011 at 06:47:19PM +0300, Sasha Levin wrote: >On Fri, 2011-08-12 at 23:22 +0800, walimis wrote: >> On Fri, Aug 12, 2011 at 06:20:54PM +0300, Sasha Levin wrote: >> >Signed-off-by: Sasha Levin <levinsasha928@gmail.com> >> >--- >> > tools/kvm/kvm-cmd.c | 1 + >> > 1 files changed, 1 insertions(+), 0 deletions(-) >> > >> >diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c >> >index e85f22f..3a90d6d 100644 >> >--- a/tools/kvm/kvm-cmd.c >> >+++ b/tools/kvm/kvm-cmd.c >> >@@ -24,6 +24,7 @@ struct cmd_struct kvm_commands[] = { >> > { "balloon", kvm_cmd_balloon, NULL, 0 }, >> > { "list", kvm_cmd_list, NULL, 0 }, >> > { "version", kvm_cmd_version, NULL, 0 }, >> >+ { "--version", kvm_cmd_version, NULL, 0 }, >> Although it works, I think it's not good way to implement a option >> as a command. > >Since kvm tools isn't going to support options to 'kvm' itself, I >believe that even though it looks somewhat hacky, it's the simplest and >most correct solution. > >If we were going to add more parameters besides '--version' then yes, >let's write a better interface. > >Perf for example allow '--version' by doing a strcmp() before command >parsing, which is equally not that nice :) OK, I get it. Thanks. walimis > >-- > >Sasha. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2011-08-12 at 23:22 +0800, walimis wrote: > On Fri, Aug 12, 2011 at 06:20:54PM +0300, Sasha Levin wrote: > >Signed-off-by: Sasha Levin <levinsasha928@gmail.com> > >--- > > tools/kvm/kvm-cmd.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > >diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c > >index e85f22f..3a90d6d 100644 > >--- a/tools/kvm/kvm-cmd.c > >+++ b/tools/kvm/kvm-cmd.c > >@@ -24,6 +24,7 @@ struct cmd_struct kvm_commands[] = { > > { "balloon", kvm_cmd_balloon, NULL, 0 }, > > { "list", kvm_cmd_list, NULL, 0 }, > > { "version", kvm_cmd_version, NULL, 0 }, > >+ { "--version", kvm_cmd_version, NULL, 0 }, > Although it works, I think it's not good way to implement a option > as a command. Since kvm tools isn't going to support options to 'kvm' itself, I believe that even though it looks somewhat hacky, it's the simplest and most correct solution. If we were going to add more parameters besides '--version' then yes, let's write a better interface. Perf for example allow '--version' by doing a strcmp() before command parsing, which is equally not that nice :)
diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c index e85f22f..3a90d6d 100644 --- a/tools/kvm/kvm-cmd.c +++ b/tools/kvm/kvm-cmd.c @@ -24,6 +24,7 @@ struct cmd_struct kvm_commands[] = { { "balloon", kvm_cmd_balloon, NULL, 0 }, { "list", kvm_cmd_list, NULL, 0 }, { "version", kvm_cmd_version, NULL, 0 }, + { "--version", kvm_cmd_version, NULL, 0 }, { "stop", kvm_cmd_stop, NULL, 0 }, { "help", kvm_cmd_help, NULL, 0 }, { "run", kvm_cmd_run, kvm_run_help, 0 },
Signed-off-by: Sasha Levin <levinsasha928@gmail.com> --- tools/kvm/kvm-cmd.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)