| Message ID | 20191008141024.10885-2-al1img@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Remove backend xen store entry on domain destroy | expand |
Oleksandr Grytsov writes ("[PATCH v1 1/2] libxl: introduce new backend type VINPUT"):
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> There are two kind of VKBD devices: with QEMU backend and user space
> backend. In current implementation they can't be distinguished as both use
> VKBD backend type. As result, user space KBD backend is started and
> stopped as QEMU backend. This commit adds new device kind VINPUT to be
> used as backend type for user space KBD backend.
I find this confusing. I'm not sure this change is right. But I'm
afraid the original patches in this area passed me by so I don't know
much about it.
I think it was a48e00f14a2d "libxl: add backend type and id to vkb"
which introduced what you are calling "user space" backends. It
appears that the vkb backend type enum was introduced there
specifically to distinguish between these two situations. For reasons
Am I wrong ? If not, why is this not working or not suitable ?
I also don't understand why the "user space" kbd backend seems to be
"linux" in the enum. Where is the implementation of this user space
backend ? Is it be controlled entirely through xenstore ?
Ian.
On Fri, Oct 11, 2019 at 5:58 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > Oleksandr Grytsov writes ("[PATCH v1 1/2] libxl: introduce new backend type VINPUT"): > > From: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > > > There are two kind of VKBD devices: with QEMU backend and user space > > backend. In current implementation they can't be distinguished as both use > > VKBD backend type. As result, user space KBD backend is started and > > stopped as QEMU backend. This commit adds new device kind VINPUT to be > > used as backend type for user space KBD backend. > > I find this confusing. I'm not sure this change is right. But I'm > afraid the original patches in this area passed me by so I don't know > much about it. > > I think it was a48e00f14a2d "libxl: add backend type and id to vkb" > which introduced what you are calling "user space" backends. It > appears that the vkb backend type enum was introduced there > specifically to distinguish between these two situations. For reasons > > Am I wrong ? If not, why is this not working or not suitable ? You are right. It is not working in some cases due to QEMU_BACKEND macro. QEMU_BACKEND treats both backends as QEMU. As result xl performs device hotplug on add/remove device. Which is not expected in case "user space" backend. In this patch I propose solution similar to VUSB device. Where VUSB used for frontend and depends on backend (kernel or QEMU) either VUSB or QUSB used for backend device type e.g. use different backend device type for different backends. It would be more clear if, for example, QEMU backend has QKBD name and all other VKBD. But it would require changes on QEMU side too. That's why I've chosen VINPUT name. Introducing new backend device type for VKBD also allows to have both backends (QEMU and non QEMU) run in same domain. Not sure if it is useful scenario but with this proposal it is possible from technical point of view. > > 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. > Where is the implementation of this user space > backend ? We have extended kbd interface (kbdif.h) to support multi-touch events as well. And we have implemented own kbd backend https://github.com/xen-troops/displ_be/ It is integrated with display backend as both use wayland API. > Is it be controlled entirely through xenstore ? Yes it is controlled entirely through xenstore: lib xl creates frontend/backend entries with initial states and configuration. > Ian.
Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type VINPUT"):
> On Fri, Oct 11, 2019 at 5:58 PM Ian Jackson <ian.jackson@citrix.com> wrote:
> > I think it was a48e00f14a2d "libxl: add backend type and id to vkb"
> > which introduced what you are calling "user space" backends. It
> > appears that the vkb backend type enum was introduced there
> > specifically to distinguish between these two situations. For reasons
> >
> > Am I wrong ? If not, why is this not working or not suitable ?
>
> You are right. It is not working in some cases due to QEMU_BACKEND macro.
> QEMU_BACKEND treats both backends as QEMU. As result xl performs device
> hotplug on add/remove device. Which is not expected in case "user
> space" backend.
Then perhaps this macro needs to be adjusted or called only
conditionally or something ?
> In this patch I propose solution similar to VUSB device. Where VUSB
> used for frontend and depends on backend (kernel or QEMU) either
> VUSB or QUSB used for backend device type e.g. use different backend
> device type for different backends.
I confess I don't quite follow all the VUSB stuff but I don't think it
is necessarily a good model.
> Introducing new backend device type for VKBD also allows to have
> both backends (QEMU and non QEMU) run in same domain. Not sure if it
> is useful scenario but with this proposal it is possible from
> technical point of view.
I don't understand why this is not possible simply by having a
different backend type.
> > 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.
> > Where is the implementation of this user space
> > backend ?
>
> We have extended kbd interface (kbdif.h) to support multi-touch events
> as well. And we have
> implemented own kbd backend https://github.com/xen-troops/displ_be/
> It is integrated with display backend as both use wayland API.
Great.
> > Is it be controlled entirely through xenstore ?
>
> Yes it is controlled entirely through xenstore: lib xl creates
> frontend/backend entries with
> initial states and configuration.
And your display backend in "troops" (is that the name of your
project) checks whether the backend type is set to "linux", so that it
knows to ignore ones that say "qemu" ?
Maybe "linux" should be "troops"...
Ian.
On Fri, Oct 11, 2019 at 8:04 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type VINPUT"): > > On Fri, Oct 11, 2019 at 5:58 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > > I think it was a48e00f14a2d "libxl: add backend type and id to vkb" > > > which introduced what you are calling "user space" backends. It > > > appears that the vkb backend type enum was introduced there > > > specifically to distinguish between these two situations. For reasons > > > > > > Am I wrong ? If not, why is this not working or not suitable ? > > > > You are right. It is not working in some cases due to QEMU_BACKEND macro. > > QEMU_BACKEND treats both backends as QEMU. As result xl performs device > > hotplug on add/remove device. Which is not expected in case "user > > space" backend. > > Then perhaps this macro needs to be adjusted or called only > conditionally or something ? I had an idea to move this macro to libxl__device_type and let device itself decides if it is qemu backend. But in case of VKBD it will read XS to determine backend type. I guess it is ok. > > > In this patch I propose solution similar to VUSB device. Where VUSB > > used for frontend and depends on backend (kernel or QEMU) either > > VUSB or QUSB used for backend device type e.g. use different backend > > device type for different backends. > > I confess I don't quite follow all the VUSB stuff but I don't think it > is necessarily a good model. If you don't mind to move QEMU_BACKEND macrto to libxl__device_type then no need to add new device type at all. > > > Introducing new backend device type for VKBD also allows to have > > both backends (QEMU and non QEMU) run in same domain. Not sure if it > > is useful scenario but with this proposal it is possible from > > technical point of view. > > I don't understand why this is not possible simply by having a > different backend type. > > > > 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. > > > > Where is the implementation of this user space > > > backend ? > > > > We have extended kbd interface (kbdif.h) to support multi-touch events > > as well. And we have > > implemented own kbd backend https://github.com/xen-troops/displ_be/ > > It is integrated with display backend as both use wayland API. > > Great. > > > > Is it be controlled entirely through xenstore ? > > > > Yes it is controlled entirely through xenstore: lib xl creates > > frontend/backend entries with > > initial states and configuration. > > And your display backend in "troops" (is that the name of your > project) checks whether the backend type is set to "linux", so that it > knows to ignore ones that say "qemu" ? > > 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. What about to have just string value instead of enum? In case QEMU we don't have such entry at all but in case custom backend the user can't put any string value here to be recognized by his backend. > Ian.
On Wed, Oct 16, 2019 at 4:26 PM Oleksandr Grytsov <al1img@gmail.com> wrote: > > On Fri, Oct 11, 2019 at 8:04 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > > > Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type VINPUT"): > > > On Fri, Oct 11, 2019 at 5:58 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > > > I think it was a48e00f14a2d "libxl: add backend type and id to vkb" > > > > which introduced what you are calling "user space" backends. It > > > > appears that the vkb backend type enum was introduced there > > > > specifically to distinguish between these two situations. For reasons > > > > > > > > Am I wrong ? If not, why is this not working or not suitable ? > > > > > > You are right. It is not working in some cases due to QEMU_BACKEND macro. > > > QEMU_BACKEND treats both backends as QEMU. As result xl performs device > > > hotplug on add/remove device. Which is not expected in case "user > > > space" backend. > > > > Then perhaps this macro needs to be adjusted or called only > > conditionally or something ? > > I had an idea to move this macro to libxl__device_type and let device > itself decides > if it is qemu backend. But in case of VKBD it will read XS to determine backend > type. I guess it is ok. > > > > > > In this patch I propose solution similar to VUSB device. Where VUSB > > > used for frontend and depends on backend (kernel or QEMU) either > > > VUSB or QUSB used for backend device type e.g. use different backend > > > device type for different backends. > > > > I confess I don't quite follow all the VUSB stuff but I don't think it > > is necessarily a good model. > > If you don't mind to move QEMU_BACKEND macrto to libxl__device_type then > no need to add new device type at all. > > > > > > Introducing new backend device type for VKBD also allows to have > > > both backends (QEMU and non QEMU) run in same domain. Not sure if it > > > is useful scenario but with this proposal it is possible from > > > technical point of view. > > > > I don't understand why this is not possible simply by having a > > different backend type. > > > > > > 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. > > > > > > Where is the implementation of this user space > > > > backend ? > > > > > > We have extended kbd interface (kbdif.h) to support multi-touch events > > > as well. And we have > > > implemented own kbd backend https://github.com/xen-troops/displ_be/ > > > It is integrated with display backend as both use wayland API. > > > > Great. > > > > > > Is it be controlled entirely through xenstore ? > > > > > > Yes it is controlled entirely through xenstore: lib xl creates > > > frontend/backend entries with > > > initial states and configuration. > > > > And your display backend in "troops" (is that the name of your > > project) checks whether the backend type is set to "linux", so that it > > knows to ignore ones that say "qemu" ? > > > > 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. > What about to have just string value instead of enum? In case QEMU > we don't have such entry at all but in case custom backend the user > can't put any string value here to be recognized by his backend. > > > Ian. ping
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: > > > > On Fri, Oct 11, 2019 at 8:04 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > > > > > Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type VINPUT"): > > > > On Fri, Oct 11, 2019 at 5:58 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > > > > I think it was a48e00f14a2d "libxl: add backend type and id to vkb" > > > > > which introduced what you are calling "user space" backends. It > > > > > appears that the vkb backend type enum was introduced there > > > > > specifically to distinguish between these two situations. For reasons > > > > > > > > > > Am I wrong ? If not, why is this not working or not suitable ? > > > > > > > > You are right. It is not working in some cases due to QEMU_BACKEND macro. > > > > QEMU_BACKEND treats both backends as QEMU. As result xl performs device > > > > hotplug on add/remove device. Which is not expected in case "user > > > > space" backend. > > > > > > Then perhaps this macro needs to be adjusted or called only > > > conditionally or something ? > > > > I had an idea to move this macro to libxl__device_type and let device > > itself decides > > if it is qemu backend. But in case of VKBD it will read XS to determine backend > > type. I guess it is ok. > > > > > > > > > In this patch I propose solution similar to VUSB device. Where VUSB > > > > used for frontend and depends on backend (kernel or QEMU) either > > > > VUSB or QUSB used for backend device type e.g. use different backend > > > > device type for different backends. > > > > > > I confess I don't quite follow all the VUSB stuff but I don't think it > > > is necessarily a good model. > > > > If you don't mind to move QEMU_BACKEND macrto to libxl__device_type then > > no need to add new device type at all. > > > > > > > > > Introducing new backend device type for VKBD also allows to have > > > > both backends (QEMU and non QEMU) run in same domain. Not sure if it > > > > is useful scenario but with this proposal it is possible from > > > > technical point of view. > > > > > > I don't understand why this is not possible simply by having a > > > different backend type. > > > > > > > > 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. > > > > > > > > Where is the implementation of this user space > > > > > backend ? > > > > > > > > We have extended kbd interface (kbdif.h) to support multi-touch events > > > > as well. And we have > > > > implemented own kbd backend https://github.com/xen-troops/displ_be/ > > > > It is integrated with display backend as both use wayland API. > > > > > > Great. > > > > > > > > Is it be controlled entirely through xenstore ? > > > > > > > > Yes it is controlled entirely through xenstore: lib xl creates > > > > frontend/backend entries with > > > > initial states and configuration. > > > > > > And your display backend in "troops" (is that the name of your > > > project) checks whether the backend type is set to "linux", so that it > > > knows to ignore ones that say "qemu" ? > > > > > > 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. > > What about to have just string value instead of enum? In case QEMU > > we don't have such entry at all but in case custom backend the user > > can't put any string value here to be recognized by his backend. > > > > > Ian. > > ping ping Ian, I'm waiting for your comments about following questions: 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. 2. Use string type for VKBD backend_type field instead of enum. It will be empty for QEMU and generic for "user space" backends. Thanks.
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.
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.
Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type VINPUT"):
> On Fri, Nov 15, 2019 at 9:43 PM Ian Jackson <ian.jackson@citrix.com> wrote:
> > 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.
Thanks.
> > 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.
We have come full circle. But on the way you have managed to get into
my thick head what is going on here. Well done and thank you.
I will go back and do a more code review of the original patch.
> Probably the name should be changed but have no idea which backend
> kind is more suitable for this purpose.
I am happy this this name. It is not in the public API so if it turns
out to be wrong we can change it.
> > 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?
I think so. What do you think ? I am just trying to get rid of the
string `linux' when it's not Linux specific. I quickly scanned
kbdif.h and it looks very like a Xen PV protocol :-). So "pv" would
sound good to me and better than "Linux", unless someone else has an
opinion.
CCing various maintainers who might have an opinion.
Ian.
Oleksandr Grytsov writes ("[PATCH v1 1/2] libxl: introduce new backend type VINPUT"):
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> There are two kind of VKBD devices: with QEMU backend and user space
> backend. In current implementation they can't be distinguished as both use
> VKBD backend type. As result, user space KBD backend is started and
> stopped as QEMU backend. This commit adds new device kind VINPUT to be
> used as backend type for user space KBD backend.
Thank you for this patch and thank you for the explanations.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
I wasn't able to find a posting of this patch before the last posting
date for 4.13 of the 13th of September. Have I missed it ? We might
be able to justify a freeze exception on the grounds that this change
affects only vkb users but it would be a matter for the RM (CC'd).
I would like to change the "linux" to "pv" or something else, for
4.13, at least.
Ian.
On Mon, Nov 18, 2019 at 7:55 PM Ian Jackson <ian.jackson@citrix.com> wrote: > > Oleksandr Grytsov writes ("[PATCH v1 1/2] libxl: introduce new backend type VINPUT"): > > From: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > > > There are two kind of VKBD devices: with QEMU backend and user space > > backend. In current implementation they can't be distinguished as both use > > VKBD backend type. As result, user space KBD backend is started and > > stopped as QEMU backend. This commit adds new device kind VINPUT to be > > used as backend type for user space KBD backend. > > Thank you for this patch and thank you for the explanations. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > I wasn't able to find a posting of this patch before the last posting > date for 4.13 of the 13th of September. Have I missed it ? We might > be able to justify a freeze exception on the grounds that this change > affects only vkb users but it would be a matter for the RM (CC'd). > This commit was submitted with patcheset [1]. Earlier I've submitted the patch to solve the issue with patchest [2]. But that patchet was totally wrong. > > I would like to change the "linux" to "pv" or something else, for > 4.13, at least. I will submit V2 with renaming and comments addressed for second commit [3] of the patchset. > > Ian. [1] https://marc.info/?l=xen-devel&m=157054390006691&w=2 [2] https://marc.info/?l=xen-devel&m=151326089604524&w=2 [3] https://marc.info/?l=xen-devel&m=157054391506708&w=2 -- Best Regards, Oleksandr Grytsov.
Oleksandr Grytsov writes ("Re: [PATCH for-4.13 v1 1/2] libxl: introduce new backend type VINPUT"):
> I will submit V2 with renaming and comments addressed for second commit [3]
> of the patchset.
Thanks. Juergen tells this is OK in principle but me he wants to take
only critical fixes after the next RC. So please be quick if you can
:-).
Thanks,
Ian.
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl index cb85c3b37f..3593e21dbb 100644 --- a/tools/libxl/libxl_types_internal.idl +++ b/tools/libxl/libxl_types_internal.idl @@ -31,6 +31,7 @@ libxl__device_kind = Enumeration("device_kind", [ (13, "VUART"), (14, "PVCALLS"), (15, "VSND"), + (16, "VINPUT"), ]) libxl__console_backend = Enumeration("console_backend", [ diff --git a/tools/libxl/libxl_vkb.c b/tools/libxl/libxl_vkb.c index 26376a7eef..4c44a813c1 100644 --- a/tools/libxl/libxl_vkb.c +++ b/tools/libxl/libxl_vkb.c @@ -38,9 +38,6 @@ static int libxl__set_xenstore_vkb(libxl__gc *gc, uint32_t domid, flexarray_t *back, flexarray_t *front, flexarray_t *ro_front) { - flexarray_append_pair(back, "backend-type", - (char *)libxl_vkb_backend_to_string(vkb->backend_type)); - if (vkb->unique_id) { flexarray_append_pair(back, XENKBD_FIELD_UNIQUE_ID, vkb->unique_id); } @@ -93,7 +90,8 @@ static int libxl__vkb_from_xenstore(libxl__gc *gc, const char *libxl_path, libxl_devid devid, libxl_device_vkb *vkb) { - const char *be_path, *be_type, *fe_path, *tmp; + const char *be_path, *fe_path, *tmp; + libxl__device dev; int rc; vkb->devid = devid; @@ -111,13 +109,11 @@ static int libxl__vkb_from_xenstore(libxl__gc *gc, const char *libxl_path, rc = libxl__backendpath_parse_domid(gc, be_path, &vkb->backend_domid); if (rc) goto out; - rc = libxl__xs_read_mandatory(gc, XBT_NULL, - GCSPRINTF("%s/backend-type", be_path), - &be_type); + rc = libxl__parse_backend_path(gc, be_path, &dev); if (rc) goto out; - rc = libxl_vkb_backend_from_string(be_type, &vkb->backend_type); - if (rc) goto out; + vkb->backend_type = dev.backend_kind == LIBXL__DEVICE_KIND_VINPUT ? + LIBXL_VKB_BACKEND_LINUX : LIBXL_VKB_BACKEND_QEMU; vkb->unique_id = xs_read(CTX->xsh, XBT_NULL, GCSPRINTF("%s/"XENKBD_FIELD_UNIQUE_ID, be_path), NULL); @@ -218,6 +214,20 @@ out: return rc; } +static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid, + libxl_device_vkb *type, libxl__device *device) +{ + device->backend_devid = type->devid; + device->backend_domid = type->backend_domid; + device->backend_kind = type->backend_type == LIBXL_VKB_BACKEND_LINUX ? + LIBXL__DEVICE_KIND_VINPUT : LIBXL__DEVICE_KIND_VKBD; + device->devid = type->devid; + device->domid = domid; + device->kind = LIBXL__DEVICE_KIND_VKBD; + + return 0; +} + int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb, const libxl_asyncop_how *ao_how) { @@ -318,7 +328,6 @@ out: return rc; } -static LIBXL_DEFINE_DEVICE_FROM_TYPE(vkb) static LIBXL_DEFINE_UPDATE_DEVID(vkb) #define libxl__add_vkbs NULL