Re: [PATCH v6 5/6] qemu: parse qemu command line for USB keyboard

[Li Zhang <zhlcindy@xxxxxxxxx>]

On 2014年02月13日 20:37, Ján Tomko wrote:
> On 02/13/2014 09:48 AM, Li Zhang wrote:
> > From: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
> >
> > This patch is to format qemu command line and xen driver for USB keyboard
> > and add test cases for it.
> >
> > Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
> > ---
> >   src/qemu/qemu_command.c                            | 41 ++++++++++++++++------
> >   src/xenxs/xen_sxpr.c                               | 27 +++++++++-----
> >   src/xenxs/xen_xm.c                                 | 30 +++++++++++-----
> >   .../qemuxml2argv-pseries-usb-kbd.args              |  9 +++++
> >   .../qemuxml2argv-pseries-usb-kbd.xml               | 19 ++++++++++
> >   tests/qemuxml2argvtest.c                           |  3 ++
> >   6 files changed, 103 insertions(+), 26 deletions(-)
> >   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
> >   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.xml
> I'd split the patch in two - one for qemu, one for xen.
>
>
> > diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
> > index d514725..79cc20e 100644
> > --- a/src/xenxs/xen_sxpr.c
> > +++ b/src/xenxs/xen_sxpr.c
> > @@ -724,21 +724,23 @@ xenParseSxprUSB(virDomainDefPtr def,
> >               tmp = sexpr_node(node, "usbdevice");
> >               if (tmp && *tmp) {
> >                   if (STREQ(tmp, "tablet") ||
> > -                    STREQ(tmp, "mouse")) {
> > +                    STREQ(tmp, "mouse") ||
> > +                    STREQ(tmp, "keyboard")) {
> >                       virDomainInputDefPtr input;
> >                       if (VIR_ALLOC(input) < 0)
> >                           goto error;
> >                       input->bus = VIR_DOMAIN_INPUT_BUS_USB;
> >                       if (STREQ(tmp, "tablet"))
> >                           input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
> > -                    else
> > +                    else if (STREQ(tmp, "mouse"))
> >                           input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
> > +                    else
> > +                        input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
> >   
> > -                    if (VIR_REALLOC_N(def->inputs, def->ninputs+1) < 0) {
> > +                    if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
> >                           VIR_FREE(input);
> >                           goto error;
> >                       }
> > -                    def->inputs[def->ninputs++] = input;
> >                   } else {
> >                       /* XXX Handle other non-input USB devices later */
> >                   }
> > @@ -2144,15 +2146,24 @@ xenFormatSxprInput(virDomainInputDefPtr input,
> >           return 0;
> >   
> >       if (input->type != VIR_DOMAIN_INPUT_TYPE_MOUSE &&
> > -        input->type != VIR_DOMAIN_INPUT_TYPE_TABLET) {
> > +        input->type != VIR_DOMAIN_INPUT_TYPE_TABLET &&
> > +        input->type != VIR_DOMAIN_INPUT_TYPE_KBD) {
> >           virReportError(VIR_ERR_INTERNAL_ERROR,
> >                          _("unexpected input type %d"), input->type);
> >           return -1;
> >       }
> >   
> > -    virBufferAsprintf(buf, "(usbdevice %s)",
> > -                      input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ?
> > -                      "mouse" : "tablet");
> > +    switch (input->type) {
> > +        case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> > +            virBufferAsprintf(buf, "(usbdevice %s)","mouse")
> > +            break;
> > +        case VIR_DOMAIN_INPUT_TYPE_TABLET:
> > +            virBufferAsprintf(buf, "(usbdevice %s)","tablet")
> > +            break;
> > +        case VIR_DOMAIN_INPUT_TYPE_KBD:
> > +            virBufferAsprintf(buf, "(usbdevice %s)","keyboard")
> > +            break;
> > +    }
> In all three cases there is a space missing between arguments and a semicolon
> missing at the end.

ah, sorry for that. :(

> >   
> >       return 0;
> >   }
> > diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> > index 5e89876..b448e99 100644
> > --- a/src/xenxs/xen_xm.c
> > +++ b/src/xenxs/xen_xm.c
> > @@ -886,14 +886,18 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
> >               goto cleanup;
> >           if (str &&
> >               (STREQ(str, "tablet") ||
> > -             STREQ(str, "mouse"))) {
> > +             STREQ(str, "mouse") ||
> > +             STREQ(str, "keyboard"))) {
> >               virDomainInputDefPtr input;
> >               if (VIR_ALLOC(input) < 0)
> >                   goto cleanup;
> >               input->bus = VIR_DOMAIN_INPUT_BUS_USB;
> > -            input->type = STREQ(str, "tablet") ?
> > -                VIR_DOMAIN_INPUT_TYPE_TABLET :
> > -                VIR_DOMAIN_INPUT_TYPE_MOUSE;
> > +            if (STREQ(str, "mouse"))
> > +                input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
> > +            else if (STREQ(str, "tablet"))
> > +                input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
> > +            else (STREQ(str, "keyboard"))
> s/else/else if/

I will change it.
> > +                input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
> >               if (VIR_ALLOC_N(def->inputs, 1) < 0) {
> >                   virDomainInputDefFree(input);
> >                   goto cleanup;
> ACK to the QEMU part. The xen part looks fine to me.
>
> Jan

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list