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

[Li Zhang <zhlcindy@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4496 bytes --]

OK,sorry for my fault.

best regards

Li Zhang
在 2012-6-15 上午6:05，"Benjamin Herrenschmidt" <benh@au1.ibm.com>写道：
>
> On Thu, 2012-06-14 at 16:27 +0200, Andreas Färber wrote:
> > Am 14.06.2012 07:17, schrieb zhlcindy@gmail.com:
> > > From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
> > >
> > > 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 <zhlcindy@linux.vnet.ibm.com>
> > > reviewed-by:   Anthony Liguori <aliguori@us.ibm.com>
> > > reviewed-by:   Benjamin Herrenschmidt <benh@au1.ibm.com>
>
> I have not reviewed that patch. Li, you must -never- make up such
> reviewed-by: lines ! You put them in if and only if you've had them
> given to you by the reviewer who has actually reviewed the patch in
> details.
>
> Ben.
>
> > > ---
> > >  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...
> >
> > 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.
> > Especially not a string when all you want is a boolean value.
> >
> > Further, in this patch it's only being assigned, not used anywhere.
> >
> > 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;
> > >      }
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 6596 bytes --]