Re: [PATCH v1 3/6] libxl: add backend type and id to vkb

[Oleksandr Grytsov <al1img@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4394 bytes --]

On Tue, Feb 6, 2018 at 4:25 PM, Wei Liu <wei.liu2@citrix.com> wrote:

> On Wed, Nov 01, 2017 at 05:05:04PM +0200, Oleksandr Grytsov wrote:
> > From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >
> > New field backend_type is added to vkb device
> > in order to have QEMU and user space backend
> > simultaneously. Each vkb backend shall read
> > appropriate XS entry and service only own
> > frontends.
> > Id is a string field which used by the backend
> > to indentify the frontend.
> >
> > Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> > ---
> >  tools/libxl/libxl_create.c  |  3 +++
> >  tools/libxl/libxl_dm.c      |  1 +
> >  tools/libxl/libxl_types.idl |  8 ++++++++
> >  tools/libxl/libxl_vkb.c     | 33 ++++++++++++++++++++++++++++++++-
> >  4 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index f813114..60d8686 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -1376,6 +1376,9 @@ static void domcreate_launch_dm(libxl__egc *egc,
> libxl__multidev *multidev,
> >          for (i = 0; i < d_config->num_vfbs; i++) {
> >              libxl__device_add(gc, domid, &libxl__vfb_devtype,
> >                                &d_config->vfbs[i]);
> > +        }
> > +
> > +        for (i = 0; i < d_config->num_vkbs; i++) {
> >              libxl__device_add(gc, domid, &libxl__vkb_devtype,
> >                                &d_config->vkbs[i]);
> >          }
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 98f89a9..f07de35 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -1728,6 +1728,7 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc
> *gc,
> >
> >      vkb->backend_domid = 0;
> >      vkb->devid = 0;
> > +
>
> Stray change. I don't have objection though.
>
> >      return 0;
> >  }
> >
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index cd0c06f..c3876a2 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -240,6 +240,12 @@ libxl_checkpointed_stream =
> Enumeration("checkpointed_stream", [
> >      (2, "COLO"),
> >      ])
> >
> > +libxl_vkb_backend = Enumeration("vkb_backend", [
> > +    (0, "UNKNOWN"),
> > +    (1, "QEMU"),
> > +    (2, "LINUX")
> > +    ])
> > +
> >  #
> >  # Complex libxl types
> >  #
> > @@ -603,6 +609,8 @@ libxl_device_vkb = Struct("device_vkb", [
> >      ("backend_domid", libxl_domid),
> >      ("backend_domname", string),
> >      ("devid", libxl_devid),
> > +    ("backend_type", libxl_vkb_backend),
> > +    ("id", string)
> >      ])
> >
> >  libxl_device_disk = Struct("device_disk", [
> > diff --git a/tools/libxl/libxl_vkb.c b/tools/libxl/libxl_vkb.c
> > index ea6fca8..88ab186 100644
> > --- a/tools/libxl/libxl_vkb.c
> > +++ b/tools/libxl/libxl_vkb.c
> > @@ -17,6 +17,10 @@
> >  static int libxl__device_vkb_setdefault(libxl__gc *gc, uint32_t domid,
> >                                          libxl_device_vkb *vkb, bool
> hotplug)
> >  {
> > +    if (vkb->backend_type == LIBXL_VKB_BACKEND_UNKNOWN) {
> > +        vkb->backend_type = LIBXL_VKB_BACKEND_QEMU;
> > +    }
> > +
> >      return libxl__resolve_domid(gc, vkb->backend_domname,
> &vkb->backend_domid);
> >  }
> >
> > @@ -34,6 +38,30 @@ static int libxl__device_from_vkb(libxl__gc *gc,
> uint32_t domid,
> >      return 0;
> >  }
> >
> > +static int libxl__device_vkb_dm_needed(libxl_device_vkb *vkb, uint32_t
> domid)
> > +{
> > +   if (vkb->backend_type == LIBXL_VKB_BACKEND_QEMU) {
> > +        return 1;
> > +   }
>
> No need to have {} for a single statement here.
>
> > +
> > +    return 0;
> > +}
> > +
> > +static int libxl__set_xenstore_vkb(libxl__gc *gc, uint32_t domid,
> > +                                   libxl_device_vkb *vkb,
> > +                                   flexarray_t *back, flexarray_t
> *front,
> > +                                   flexarray_t *ro_front)
> > +{
> > +    if (vkb->id) {
> > +        flexarray_append_pair(front, "id", vkb->id);
> > +    }
> > +
>
> Ditto.
>
> And, isn't 0 a valid device id?
>
>
id here is char *. I put it to xenstore only if it is defined. It shall be
defined for linux backend
in case of multiple instances of kbd device are defined. The backend
distinguishes them
by id.

Wei.
>



-- 
Best Regards,
Oleksandr Grytsov.

[-- Attachment #1.2: Type: text/html, Size: 6438 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel