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

From: Ian Jackson <ian.jackson@citrix.com>
To: Oleksandr Grytsov <al1img@gmail.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Oleksandr Grytsov <oleksandr_grytsov@epam.com>,
	"wl@xen.org" <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH v1 1/2] libxl: introduce new backend type VINPUT
Date: Fri, 15 Nov 2019 19:43:49 +0000	[thread overview]
Message-ID: <24014.65525.944108.509444@mariner.uk.xensource.com> (raw)
In-Reply-To: <CACvf2oUpk=bP4QB8c9QTPcomuOpYm88+G6Bm_DyFf2h_4_MFGA@mail.gmail.com>

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 ?

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.

> 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" ?

So maybe we could call it "pv" ?  Is there a protocol doc I should be
looking at that defines this vkb interface ?

Sorry still to be so confused.

Regards,
Ian.

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