diff mbox

[v2,2/5] qemu-options: Bail out on unsupported options instead of silently ignoring them

Message ID 1525453270-23074-3-git-send-email-thuth@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Huth May 4, 2018, 5:01 p.m. UTC
The dangling remainder of the -tdf option revealed a deficiency in our
option parsing: Options that have been declared, but are not supported
in the switch-case statement in vl.c and not handled in the OS-specifc
os_parse_cmd_args() functions are currently silently ignored. We should
rather tell the users that they specified something that we can not
handle, so let's print an error message and exit instead.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/qemu-common.h | 2 +-
 os-posix.c            | 6 +++++-
 os-win32.c            | 4 ++--
 vl.c                  | 5 ++++-
 4 files changed, 12 insertions(+), 5 deletions(-)

Comments

Anthony PERARD May 16, 2018, 4:49 p.m. UTC | #1
On Fri, May 04, 2018 at 07:01:07PM +0200, Thomas Huth wrote:
> diff --git a/vl.c b/vl.c
> index 806eec2..457e93b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4042,7 +4042,10 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>              default:
> -                os_parse_cmd_args(popt->index, optarg);
> +                if (os_parse_cmd_args(popt->index, optarg)) {
> +                    error_report("Option not supported in this build");
> +                    exit(1);
> +                }

I think that report false errors, I've got:
qemu-system-i386: -no-user-config: Option not supported in this build

Regards,
Thomas Huth May 16, 2018, 4:51 p.m. UTC | #2
On 16.05.2018 18:49, Anthony PERARD wrote:
> On Fri, May 04, 2018 at 07:01:07PM +0200, Thomas Huth wrote:
>> diff --git a/vl.c b/vl.c
>> index 806eec2..457e93b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4042,7 +4042,10 @@ int main(int argc, char **argv, char **envp)
>>                  }
>>                  break;
>>              default:
>> -                os_parse_cmd_args(popt->index, optarg);
>> +                if (os_parse_cmd_args(popt->index, optarg)) {
>> +                    error_report("Option not supported in this build");
>> +                    exit(1);
>> +                }
> 
> I think that report false errors, I've got:
> qemu-system-i386: -no-user-config: Option not supported in this build

Yes, sorry for that. Patch is already on the way:

https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg02972.html

 Thomas
diff mbox

Patch

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 8a4f63c..85f4749 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -137,7 +137,7 @@  char *qemu_find_file(int type, const char *name);
 /* OS specific functions */
 void os_setup_early_signal_handling(void);
 char *os_find_datadir(void);
-void os_parse_cmd_args(int index, const char *optarg);
+int os_parse_cmd_args(int index, const char *optarg);
 
 #include "qemu/module.h"
 
diff --git a/os-posix.c b/os-posix.c
index 24eb700..9ce6f74 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -165,7 +165,7 @@  static bool os_parse_runas_uid_gid(const char *optarg)
  * Parse OS specific command line options.
  * return 0 if option handled, -1 otherwise
  */
-void os_parse_cmd_args(int index, const char *optarg)
+int os_parse_cmd_args(int index, const char *optarg)
 {
     switch (index) {
 #ifdef CONFIG_SLIRP
@@ -199,7 +199,11 @@  void os_parse_cmd_args(int index, const char *optarg)
         fips_set_state(true);
         break;
 #endif
+    default:
+        return -1;
     }
+
+    return 0;
 }
 
 static void change_process_uid(void)
diff --git a/os-win32.c b/os-win32.c
index 586a7c7..0674f94 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -93,9 +93,9 @@  void os_set_line_buffering(void)
  * Parse OS specific command line options.
  * return 0 if option handled, -1 otherwise
  */
-void os_parse_cmd_args(int index, const char *optarg)
+int os_parse_cmd_args(int index, const char *optarg)
 {
-    return;
+    return -1;
 }
 
 int qemu_create_pidfile(const char *filename)
diff --git a/vl.c b/vl.c
index 806eec2..457e93b 100644
--- a/vl.c
+++ b/vl.c
@@ -4042,7 +4042,10 @@  int main(int argc, char **argv, char **envp)
                 }
                 break;
             default:
-                os_parse_cmd_args(popt->index, optarg);
+                if (os_parse_cmd_args(popt->index, optarg)) {
+                    error_report("Option not supported in this build");
+                    exit(1);
+                }
             }
         }
     }