Re: [Xen-devel] [PATCH v1 1/2] libxl: introduce new backend type VINPUT

[Oleksandr Grytsov <al1img@gmail.com>]

On Fri, Nov 15, 2019 at 9:43 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type VINPUT"):
> > 1. Move QEMU_BACKEND macro to libxl__device_type structure as function
> >     and let the device to decide it has QEMU backend:
> >
> > struct libxl__device_type {
> >     ...
> >     device_qemu_backend_fn_t qemu_backend
> > }
> >
> > In this case, introducing new device type for VKBD is not needed. The VKBD
> > device will check backend type XS entry to define which backend is running.
>
> Sorry for the delay replying.  In your earlier mails I had trouble
> figuring out what you meant but this little vignette makes it clear to
> me.
>
> I think the problem you are trying to solve is this: in your case
> QEMU_BACKEND needs to depend on the visible vkb_backend field, but the
> device->backend_kind is set unconditionally to just VKB ?

Exactly.

>
> Could you solve this problem by inventing a new backend_kind, and
> writing your own function libxl__device_from_vkb, and putting
> *different* values into backend_kind ?  I think that is what
> backend_kind is for.  See for example various console functions and
> also libxl__device_from_disk.
>

This what was done in this patch. VINPUT backend type was introduced.
Probably the name should be changed but have no idea which backend
kind is more suitable for this purpose.

Also bakcend-type xenstore entry was removed as redundant in this case.
As the PV backend expects device kind VINPUT.

> > 2. Use string type for VKBD backend_type field instead of enum. It will be
> > empty for QEMU and generic for "user space" backends.
>
> This seems worse.
>
> > On Mon, Oct 28, 2019 at 4:06 PM Oleksandr Grytsov <al1img@gmail.com> wrote:
> > > On Wed, Oct 16, 2019 at 4:26 PM Oleksandr Grytsov <al1img@gmail.com> wrote:
> > > > [Ian:]
> > > > > [Oleksandr:]
> > > > > > [Ian:]
> > > > > > > I also don't understand why the "user space" kbd backend seems to be
> > > > > > > "linux" in the enum.
> > > > > >
> > > > > > I agree this is not so good name. But I don't know how to call
> > > > > > backends which are not running
> > > > > > inside QEMU in general.
> > > > >
> > > > > I think this would be useable on freebsd ?  "linux" definitely seems
> > > > > wrong.  I see it hasn't been in a release so it is not too late to
> > > > > rename it, subject to discussion with Juergen as RM.
> ...
> > > > > Maybe "linux" should be "troops"...
> > > >
> > > > It doesn't look as generic solution. If some user implements own backend
> > > > it should add new entry into backend type enum.
>
> Would you be prepared to change it to *something* else ?
>
> AFAICT from the code it just uses what would the `usual' xenstore pv
> control plane path for a device called "vkb" ?
>

I guess yes.

> So maybe we could call it "pv" ?

Do you mean LIBXL_VKB_BACKEND_PV?

> Is there a protocol doc I should be
> looking at that defines this vkb interface ?
>

This PV backend utilizes the protocol described in kbdif.h

> Sorry still to be so confused.
>
> Regards,
> Ian.



-- 
Best Regards,
Oleksandr Grytsov.

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