qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/1] Add usb option in machine options.


From: Li Zhang
Subject: Re: [Qemu-devel] [PATCH 1/1] Add usb option in machine options.
Date: Fri, 15 Jun 2012 11:06:09 +0800



On Thu, Jun 14, 2012 at 10:27 PM, Andreas Färber <address@hidden> wrote:
Am 14.06.2012 07:17, schrieb address@hidden:
> From: Li Zhang <address@hidden>
>
> For pseries machine, it needs to enable usb
> to add kbd or usb mouse. -usb option won't
> be used in the future, and machine options
> is a better way to enable usb.
>
> So this patch is to add usb option to machine
> options (-machine type=psereis,usb=on/off)
> to enable/disable usb controller.
>
> In this patch, usb_opt is an global option
> which can be checked by machines. For example,
> on pseries, it will check if usb_opt is on, if
> it is on, it will create one usb ohci controller.
> As the following:
> if (usb_opts && strcmp(usb_opts, "on") == 0)
>      pci_create_simple(bus, -1, "pci-ohci");
>
> In this patch, usb is on by default.
> So, for -nodefault, usb should be set off in the
> command line as the following:
>  -machine type=pseries,usb=off.
>
> Signed-off-by: Li Zhang <address@hidden>
> reviewed-by:   Anthony Liguori <address@hidden>
> reviewed-by:   Benjamin Herrenschmidt <address@hidden>
> ---
>  qemu-config.c |    4 ++++
>  sysemu.h      |    1 +
>  vl.c          |   12 ++++++++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/qemu-config.c b/qemu-config.c
> index bb3bff4..258712a 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -583,6 +583,10 @@ static QemuOptsList qemu_machine_opts = {
>              .name = "dtb",
>              .type = QEMU_OPT_STRING,
>              .help = "Linux kernel device tree file",
> +        }, {
> +            .name = "usb",
> +            .type = QEMU_OPT_BOOL,
> +            .help = "Set on/off to enable/disable usb",
>          },
>          { /* End of list */ }
>      },
> diff --git a/sysemu.h b/sysemu.h
> index bc2c788..c5ea10d 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -13,6 +13,7 @@
>  /* vl.c */
>
>  extern const char *bios_name;
> +extern const char *usb_opt;
>
>  extern const char *qemu_name;
>  extern uint8_t qemu_uuid[];
> diff --git a/vl.c b/vl.c
> index 204d85b..10f8e4c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -171,6 +171,7 @@ int main(int argc, char **argv)
>
>  static const char *data_dir;
>  const char *bios_name = NULL;
> +const char *usb_opt = NULL;
>  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
>  DisplayType display_type = DT_DEFAULT;
>  int display_remote = 0;

I'd be very surprised if Anthony has actually reviewed this...

Sorry, I just think I add him here because he gives some suggestions about adding usb option to machine options in the mailing list.:)
Maybe it is not appropriate to add here. Sorry for that. 

The point of using machine options is so that you can use the QemuOpts
infrastructure to inquire this value, not to save more global state.
OK, I think it still needs one global value because other machines need to check whether usb is enabled.
 
Especially not a string when all you want is a boolean value.

From the qemu, QEMU_OPT_BOOL type  still use a string "on/off" to enable/disable usb.
Maybe it's better to convert it to boolean value. 

Further, in this patch it's only being assigned, not used anywhere.

In vl.c, it seems that it is not used.  
I have been thinking to use it in spapr.c. 

if (usb_opts && strcmp(usb_opts, "on") == 0)
            pci_create_simple(bus, -1, "pci-ohci");

But I want to see whether this way is accepted. 
If it is ok, I will add this option to spapr.c.  

Andreas

> @@ -758,6 +759,15 @@ static int bt_parse(const char *opt)
>      return 1;
>  }
>
> +static int default_enable_usb(QemuOpts *opts)
> +{
> +    if (NULL == qemu_opt_get(opts, "usb")) {
> +        qemu_opt_set(opts, "usb", "on");
> +    }
> +
> +    return 0;
> +}
> +
>  /***********************************************************/
>  /* QEMU Block devices */
>
> @@ -3356,6 +3366,8 @@ int main(int argc, char **argv, char **envp)
>          kernel_filename = qemu_opt_get(machine_opts, "kernel");
>          initrd_filename = qemu_opt_get(machine_opts, "initrd");
>          kernel_cmdline = qemu_opt_get(machine_opts, "append");
> +        default_enable_usb(machine_opts);
> +        usb_opt = qemu_opt_get(machine_opts, "usb");
>      } else {
>          kernel_filename = initrd_filename = kernel_cmdline = NULL;
>      }


--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



--

Best Regards
-Li


reply via email to

[Prev in Thread] Current Thread [Next in Thread]