Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Markus Armbruster <armbru@redhat.com>]

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Introduce OS specific cmdline argument handling by calling
> os_parse_cmd_args() at the end of switch() statement.
>
> In addition move SMB argument to os-posix.c
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  os-posix.c |   34 ++++++++++++++++++++++++++++++++++
>  os-win32.c |   22 ++++++++++++++++++++++
>  sysemu.h   |    9 +++++++++
>  vl.c       |   15 ++-------------
>  4 files changed, 67 insertions(+), 13 deletions(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 621ad06..66f2bf5 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -33,6 +33,7 @@
>  /* Needed early for CONFIG_BSD etc. */
>  #include "config-host.h"
>  #include "sysemu.h"
> +#include "net/slirp.h"
>  
>  void os_setup_early_signal_handling(void)
>  {
> @@ -130,3 +131,36 @@ char *os_find_datadir(const char *argv0)
>  }
>  #undef SHARE_SUFFIX
>  #undef BUILD_SUFFIX
> +
> +/*
> + * Duplicate definition from vl.c to avoid messing up the entire build
> + */
> +enum {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +    opt_enum,
> +#define DEFHEADING(text)
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef GEN_DOCS
> +};
> +
> +/*
> + * Parse OS specific command line options.
> + * return 0 if option handled, -1 otherwise
> + */
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
> +{
> +    int ret = 0;
> +    switch (popt->index) {
> +#ifdef CONFIG_SLIRP
> +    case QEMU_OPTION_smb:
> +        if (net_slirp_smb(optarg) < 0)
> +            exit(1);
> +        break;
> +#endif

Was #ifndef _WIN32 before.  Impact?

> +    default:
> +        ret = -1;
> +    }
> +    return ret;
> +}
> diff --git a/os-win32.c b/os-win32.c
> index 1758538..a311a90 100644
> --- a/os-win32.c
> +++ b/os-win32.c
> @@ -204,3 +204,25 @@ char *os_find_datadir(const char *argv0)
>      }
>      return NULL;
>  }
> +
> +/*
> + * Duplicate definition from vl.c to avoid messing up the entire build
> + */
> +enum {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +    opt_enum,
> +#define DEFHEADING(text)
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef GEN_DOCS
> +};

I agree with Richard: this is gross.

> +
> +/*
> + * Parse OS specific command line options.
> + * return 0 if option handled, -1 otherwise
> + */
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
> +{
> +    return -1;
> +}
> diff --git a/sysemu.h b/sysemu.h
> index 72f3734..08ec323 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -79,9 +79,18 @@ int qemu_loadvm_state(QEMUFile *f);
>  /* SLIRP */
>  void do_info_slirp(Monitor *mon);
>  
> +/* This is needed for vl.c and the OS specific files */
> +typedef struct QEMUOption {
> +    const char *name;
> +    int flags;
> +    int index;
> +    uint32_t arch_mask;
> +} QEMUOption;
> +

Ugh.

>  /* OS specific functions */
>  void os_setup_early_signal_handling(void);
>  char *os_find_datadir(const char *argv0);
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg);
>  
>  typedef enum DisplayType
>  {
> diff --git a/vl.c b/vl.c
> index 7f22733..838e109 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1909,13 +1909,6 @@ enum {
>  #undef GEN_DOCS
>  };
>  
> -typedef struct QEMUOption {
> -    const char *name;
> -    int flags;
> -    int index;
> -    uint32_t arch_mask;
> -} QEMUOption;
> -
>  static const QEMUOption qemu_options[] = {
>      { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
>  #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> @@ -2624,12 +2617,6 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_bootp:
>                  legacy_bootp_filename = optarg;
>                  break;
> -#ifndef _WIN32
> -            case QEMU_OPTION_smb:
> -                if (net_slirp_smb(optarg) < 0)
> -                    exit(1);
> -                break;
> -#endif
>              case QEMU_OPTION_redir:
>                  if (net_slirp_redir(optarg) < 0)
>                      exit(1);
> @@ -3126,6 +3113,8 @@ int main(int argc, char **argv, char **envp)
>                      fclose(fp);
>                      break;
>                  }
> +            default:
> +                os_parse_cmd_args(popt, optarg);
>              }
>          }
>      }

Is this minor improvement of vl.c really worth the headaches elsewhere?