Michael Ellerman | 10 Aug 2012 07:38
Picon
Gravatar

[PATCH 1/2] kvm tools: Fix formatting of error message in TAP handling

This error message is missing a space, and has a redundant ":" at the end,
currently it produces:

  You have requested a TAP device, but creation of one hasfailed because:: No such file or directory

Add a space to "hasfailed" and remove the extra ":".

Don't split the line to improve grepability.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---
 tools/kvm/virtio/net.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 10420ae..8f3735b 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
 <at>  <at>  -526,8 +526,7  <at>  <at>  void virtio_net__init(const struct virtio_net_params *params)
 	ndev->mode = params->mode;
 	if (ndev->mode == NET_MODE_TAP) {
 		if (!virtio_net__tap_init(params, ndev))
-			die_perror("You have requested a TAP device, but creation of one has"
-					"failed because:");
+			die_perror("You have requested a TAP device, but creation of one has failed because");
 		ndev->ops = &tap_ops;
 	} else {
 		ndev->info.host_ip		= ntohl(inet_addr(params->host_ip));
--

-- 
1.7.9.5
(Continue reading)

Michael Ellerman | 10 Aug 2012 07:38
Picon
Gravatar

[PATCH 2/2] kvm tools: Check for unknown parameters in network option handling

Currently the parsing of network command line parameters doesn't check
for unknown parameters.

For example "-n mod=none" will cause kvmtool to setup TAP networking.

Save users from their typos by checking for unknown parameters and bailing.

Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
---
 tools/kvm/builtin-run.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index a36bd00..9e5c1d4 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
 <at>  <at>  -257,7 +257,8  <at>  <at>  static int set_net_param(struct virtio_net_params *p, const char *param,
 		p->vhost = atoi(val);
 	} else if (strcmp(param, "fd") == 0) {
 		p->fd = atoi(val);
-	}
+	} else
+		die("Unknown network parameter %s", param);

 	return 0;
 }
--

-- 
1.7.9.5

--
(Continue reading)

Asias He | 11 Aug 2012 02:21
Picon

Re: [PATCH 2/2] kvm tools: Check for unknown parameters in network option handling

On Fri, Aug 10, 2012 at 1:38 PM, Michael Ellerman
<michael <at> ellerman.id.au> wrote:
> Currently the parsing of network command line parameters doesn't check
> for unknown parameters.
>
> For example "-n mod=none" will cause kvmtool to setup TAP networking.
>
> Save users from their typos by checking for unknown parameters and bailing.
>
> Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
> ---
>  tools/kvm/builtin-run.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index a36bd00..9e5c1d4 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
>  <at>  <at>  -257,7 +257,8  <at>  <at>  static int set_net_param(struct virtio_net_params *p, const char *param,
>                 p->vhost = atoi(val);
>         } else if (strcmp(param, "fd") == 0) {
>                 p->fd = atoi(val);
> -       }
> +       } else
> +               die("Unknown network parameter %s", param);

we need braces here:

+ } else {
+           die("Unknown network parameter %s", param);
(Continue reading)

Michael Ellerman | 13 Aug 2012 05:31
Picon
Gravatar

Re: [PATCH 2/2] kvm tools: Check for unknown parameters in network option handling

On Sat, 2012-08-11 at 08:21 +0800, Asias He wrote:
> On Fri, Aug 10, 2012 at 1:38 PM, Michael Ellerman
> <michael <at> ellerman.id.au> wrote:
> > Currently the parsing of network command line parameters doesn't check
> > for unknown parameters.
> >
> > For example "-n mod=none" will cause kvmtool to setup TAP networking.
> >
> > Save users from their typos by checking for unknown parameters and bailing.
> >
> > Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
> > ---
> >  tools/kvm/builtin-run.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> > index a36bd00..9e5c1d4 100644
> > --- a/tools/kvm/builtin-run.c
> > +++ b/tools/kvm/builtin-run.c
> >  <at>  <at>  -257,7 +257,8  <at>  <at>  static int set_net_param(struct virtio_net_params *p, const char *param,
> >                 p->vhost = atoi(val);
> >         } else if (strcmp(param, "fd") == 0) {
> >                 p->fd = atoi(val);
> > -       }
> > +       } else
> > +               die("Unknown network parameter %s", param);
> 
> we need braces here:

We don't _need_ braces, but I assume you mean you'd prefer braces?
(Continue reading)

Pekka Enberg | 13 Aug 2012 09:27
Gravatar

Re: [PATCH 2/2] kvm tools: Check for unknown parameters in network option handling

On Mon, Aug 13, 2012 at 6:31 AM, Michael Ellerman
<michael <at> ellerman.id.au> wrote:
>> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
>> > index a36bd00..9e5c1d4 100644
>> > --- a/tools/kvm/builtin-run.c
>> > +++ b/tools/kvm/builtin-run.c
>> >  <at>  <at>  -257,7 +257,8  <at>  <at>  static int set_net_param(struct virtio_net_params *p, const char *param,
>> >                 p->vhost = atoi(val);
>> >         } else if (strcmp(param, "fd") == 0) {
>> >                 p->fd = atoi(val);
>> > -       }
>> > +       } else
>> > +               die("Unknown network parameter %s", param);
>>
>> we need braces here:
>
> We don't _need_ braces, but I assume you mean you'd prefer braces?

This is Linux coding style so we don't prefer them either.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Asias He | 14 Aug 2012 03:27
Picon

Re: [PATCH 2/2] kvm tools: Check for unknown parameters in network option handling

On Mon, Aug 13, 2012 at 3:27 PM, Pekka Enberg <penberg <at> kernel.org> wrote:
> On Mon, Aug 13, 2012 at 6:31 AM, Michael Ellerman
> <michael <at> ellerman.id.au> wrote:
>>> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
>>> > index a36bd00..9e5c1d4 100644
>>> > --- a/tools/kvm/builtin-run.c
>>> > +++ b/tools/kvm/builtin-run.c
>>> >  <at>  <at>  -257,7 +257,8  <at>  <at>  static int set_net_param(struct virtio_net_params *p, const char *param,
>>> >                 p->vhost = atoi(val);
>>> >         } else if (strcmp(param, "fd") == 0) {
>>> >                 p->fd = atoi(val);
>>> > -       }
>>> > +       } else
>>> > +               die("Unknown network parameter %s", param);
>>>
>>> we need braces here:
>>
>> We don't _need_ braces, but I assume you mean you'd prefer braces?
>
> This is Linux coding style so we don't prefer them either.

Documentation/CodingStyle says:
'''
Do not unnecessarily use braces where a single statement will do.

if (condition)
        action();

and

(Continue reading)

Asias He | 11 Aug 2012 01:58
Picon

Re: [PATCH 1/2] kvm tools: Fix formatting of error message in TAP handling

On Fri, Aug 10, 2012 at 1:38 PM, Michael Ellerman
<michael <at> ellerman.id.au> wrote:
> This error message is missing a space, and has a redundant ":" at the end,
> currently it produces:
>
>   You have requested a TAP device, but creation of one hasfailed because:: No such file or directory
>
> Add a space to "hasfailed" and remove the extra ":".
>
> Don't split the line to improve grepability.
>
> Signed-off-by: Michael Ellerman <michael <at> ellerman.id.au>
> ---
>  tools/kvm/virtio/net.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
> index 10420ae..8f3735b 100644
> --- a/tools/kvm/virtio/net.c
> +++ b/tools/kvm/virtio/net.c
>  <at>  <at>  -526,8 +526,7  <at>  <at>  void virtio_net__init(const struct virtio_net_params *params)
>         ndev->mode = params->mode;
>         if (ndev->mode == NET_MODE_TAP) {
>                 if (!virtio_net__tap_init(params, ndev))
> -                       die_perror("You have requested a TAP device, but creation of one has"
> -                                       "failed because:");
> +                       die_perror("You have requested a TAP device, but creation of one has failed because");
>                 ndev->ops = &tap_ops;
>         } else {
>                 ndev->info.host_ip              = ntohl(inet_addr(params->host_ip));
(Continue reading)


Gmane