diff mbox

[v2,1/2] fix a typo of strict option

Message ID 1386576358-10469-2-git-send-email-akong@redhat.com
State New
Headers show

Commit Message

Amos Kong Dec. 9, 2013, 8:05 a.m. UTC
The type of "strict" should be bool.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 vl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Markus Armbruster Dec. 9, 2013, 9:16 a.m. UTC | #1
Amos Kong <akong@redhat.com> writes:

> The type of "strict" should be bool.
>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  vl.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index b0399de..12fe1f7 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -461,7 +461,7 @@ static QemuOptsList qemu_boot_opts = {
>              .type = QEMU_OPT_STRING,
>          }, {
>              .name = "strict",
> -            .type = QEMU_OPT_STRING,
> +            .type = QEMU_OPT_BOOL,
>          },
>          { /*End of list */ }
>      },

No, this isn't just a typo fix, it's an interface change.  It's not
immediately obvious whether it's okay.

-boot parameter 'strict' appeared in commit c8a6ae8.  It was declared
QEMU_OPT_STRING, but the code using the value rejected anything but 'on'
and 'off'.  This shipped in 1.5.0.

Commit 6ef4716 accidentally dropped the code using the value of
'strict'.  Since then, any parameter value is accepted and silently
ignored.  Shipped in 1.6.0.

Your change restores the "reject anything but 'on' and 'off'" part (the
"use the value" part gets restored in the next patch).  Okay, because
the values erroneously accepted in the meantime didn't do anything.
Amos Kong Dec. 9, 2013, 11:55 a.m. UTC | #2
On Mon, Dec 09, 2013 at 10:16:38AM +0100, Markus Armbruster wrote:
> Amos Kong <akong@redhat.com> writes:
> 
> > The type of "strict" should be bool.
> >
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  vl.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index b0399de..12fe1f7 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -461,7 +461,7 @@ static QemuOptsList qemu_boot_opts = {
> >              .type = QEMU_OPT_STRING,
> >          }, {
> >              .name = "strict",
> > -            .type = QEMU_OPT_STRING,
> > +            .type = QEMU_OPT_BOOL,
> >          },
> >          { /*End of list */ }
> >      },
> 
> No, this isn't just a typo fix, it's an interface change.  It's not
> immediately obvious whether it's okay.
 
You are correct, it's not a typo, menu's type was also changed from STRING to BOOL.
We can just process strict as menu.

> -boot parameter 'strict' appeared in commit c8a6ae8.  It was declared
> QEMU_OPT_STRING, but the code using the value rejected anything but 'on'
> and 'off'.  This shipped in 1.5.0.
> 
> Commit 6ef4716 accidentally dropped the code using the value of
> 'strict'.  Since then, any parameter value is accepted and silently
> ignored.  Shipped in 1.6.0.
> 
> Your change restores the "reject anything but 'on' and 'off'" part (the
> "use the value" part gets restored in the next patch).  Okay, because
> the values erroneously accepted in the meantime didn't do anything.

Thanks your explanation :)
diff mbox

Patch

diff --git a/vl.c b/vl.c
index b0399de..12fe1f7 100644
--- a/vl.c
+++ b/vl.c
@@ -461,7 +461,7 @@  static QemuOptsList qemu_boot_opts = {
             .type = QEMU_OPT_STRING,
         }, {
             .name = "strict",
-            .type = QEMU_OPT_STRING,
+            .type = QEMU_OPT_BOOL,
         },
         { /*End of list */ }
     },