| Message ID | 1470119552-16170-1-git-send-email-jgross@suse.com |
|---|---|
| State | New |
| Headers | show |
On Di, 2016-08-02 at 08:32 +0200, Juergen Gross wrote: > Instead of calling xen_be_register() for each supported backend type > for hvm and pv guests in their machine init functions use a common > function in order not to have to add new backends twice. > > This at once fixes the error that hvm domains couldn't use the qusb > backend. Looks good to me. Should I take this through the usb patch queue, together with the other xen-usb fixes (once codestyle issues are fixed)? If so, can I get an ack from xen please, preferably fast enough for -rc2? thanks, Gerd
On Tue, Aug 02, 2016 at 08:32:32AM +0200, Juergen Gross wrote: > Instead of calling xen_be_register() for each supported backend type > for hvm and pv guests in their machine init functions use a common > function in order not to have to add new backends twice. > > This at once fixes the error that hvm domains couldn't use the qusb > backend. > > Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> > --- > Is it on purpose the qnic and vfb backends are not registered for HVM? I guess it has not been usefull. Stefano may have a better answer.
On Tue, 2 Aug 2016, Juergen Gross wrote: > Instead of calling xen_be_register() for each supported backend type > for hvm and pv guests in their machine init functions use a common > function in order not to have to add new backends twice. > > This at once fixes the error that hvm domains couldn't use the qusb > backend. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > Is it on purpose the qnic and vfb backends are not registered for HVM? Yes, it is on purpose: there is no code in any toolstacks to use qnic, and the presence of vfb can cause problems to Linux HVM guests (or at least it used to), additionally vfb for HVM guests is also disabled in libxl. In general, it is a good idea to disable code that is not supposed to be used. Can qusb be used with HVM guests with libxl/xl? > hw/xen/xen_backend.c | 10 ++++++++++ > hw/xenpv/xen_machine_pv.c | 7 +------ > include/hw/xen/xen_backend.h | 1 + > xen-hvm.c | 4 +--- > 4 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index bab79b1..1b88891 100644 > --- a/hw/xen/xen_backend.c > +++ b/hw/xen/xen_backend.c > @@ -800,6 +800,16 @@ int xen_be_register(const char *type, struct XenDevOps *ops) > return xenstore_scan(type, xen_domid, ops); > } > > +void xen_be_register_common(void) > +{ > + xen_be_register("console", &xen_console_ops); > + xen_be_register("vkbd", &xen_kbdmouse_ops); > + xen_be_register("qdisk", &xen_blkdev_ops); > +#ifdef CONFIG_USB_LIBUSB > + xen_be_register("qusb", &xen_usb_ops); > +#endif > +} > + > int xen_be_bind_evtchn(struct XenDevice *xendev) > { > if (xendev->local_port != -1) { > diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c > index 48f725c..79aef4e 100644 > --- a/hw/xenpv/xen_machine_pv.c > +++ b/hw/xenpv/xen_machine_pv.c > @@ -67,14 +67,9 @@ static void xen_init_pv(MachineState *machine) > break; > } > > - xen_be_register("console", &xen_console_ops); > - xen_be_register("vkbd", &xen_kbdmouse_ops); > + xen_be_register_common(); > xen_be_register("vfb", &xen_framebuffer_ops); > - xen_be_register("qdisk", &xen_blkdev_ops); > xen_be_register("qnic", &xen_netdev_ops); > -#ifdef CONFIG_USB_LIBUSB > - xen_be_register("qusb", &xen_usb_ops); > -#endif > > /* configure framebuffer */ > if (xenfb_enabled) { > diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h > index 754c0a4..0df282a 100644 > --- a/include/hw/xen/xen_backend.h > +++ b/include/hw/xen/xen_backend.h > @@ -87,6 +87,7 @@ void xen_be_check_state(struct XenDevice *xendev); > > /* xen backend driver bits */ > int xen_be_init(void); > +void xen_be_register_common(void); > int xen_be_register(const char *type, struct XenDevOps *ops); > int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state); > int xen_be_bind_evtchn(struct XenDevice *xendev); > diff --git a/xen-hvm.c b/xen-hvm.c > index eb57792..3b0343a 100644 > --- a/xen-hvm.c > +++ b/xen-hvm.c > @@ -1318,9 +1318,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory) > error_report("xen backend core setup failed"); > goto err; > } > - xen_be_register("console", &xen_console_ops); > - xen_be_register("vkbd", &xen_kbdmouse_ops); > - xen_be_register("qdisk", &xen_blkdev_ops); > + xen_be_register_common(); > xen_read_physmap(state); > return; > > -- > 2.6.6 >
On Tue, 2 Aug 2016, Gerd Hoffmann wrote: > On Di, 2016-08-02 at 08:32 +0200, Juergen Gross wrote: > > Instead of calling xen_be_register() for each supported backend type > > for hvm and pv guests in their machine init functions use a common > > function in order not to have to add new backends twice. > > > > This at once fixes the error that hvm domains couldn't use the qusb > > backend. > > Looks good to me. Should I take this through the usb patch queue, > together with the other xen-usb fixes (once codestyle issues are fixed)? > If so, can I get an ack from xen please, preferably fast enough for > -rc2? Hi Gerd, I am happy for you to handle all three patches (if for any reasons you change your mind I can do it). "xen: bug fixes in Xen backend handling" v2 is ready to be committed, and I am just waiting for an answer on this patch.
On 02/08/16 20:27, Stefano Stabellini wrote: > On Tue, 2 Aug 2016, Juergen Gross wrote: >> Instead of calling xen_be_register() for each supported backend type >> for hvm and pv guests in their machine init functions use a common >> function in order not to have to add new backends twice. >> >> This at once fixes the error that hvm domains couldn't use the qusb >> backend. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> Is it on purpose the qnic and vfb backends are not registered for HVM? > > Yes, it is on purpose: there is no code in any toolstacks to use qnic, > and the presence of vfb can cause problems to Linux HVM guests (or at > least it used to), additionally vfb for HVM guests is also disabled in > libxl. > > In general, it is a good idea to disable code that is not supposed to be > used. > > Can qusb be used with HVM guests with libxl/xl? Yes. You have to specify "type=qusb" for usbctrl, then it will work. I have verified that. Juergen
On Wed, 3 Aug 2016, Juergen Gross wrote: > On 02/08/16 20:27, Stefano Stabellini wrote: > > On Tue, 2 Aug 2016, Juergen Gross wrote: > >> Instead of calling xen_be_register() for each supported backend type > >> for hvm and pv guests in their machine init functions use a common > >> function in order not to have to add new backends twice. > >> > >> This at once fixes the error that hvm domains couldn't use the qusb > >> backend. > >> > >> Signed-off-by: Juergen Gross <jgross@suse.com> > >> --- > >> Is it on purpose the qnic and vfb backends are not registered for HVM? > > > > Yes, it is on purpose: there is no code in any toolstacks to use qnic, > > and the presence of vfb can cause problems to Linux HVM guests (or at > > least it used to), additionally vfb for HVM guests is also disabled in > > libxl. > > > > In general, it is a good idea to disable code that is not supposed to be > > used. > > > > Can qusb be used with HVM guests with libxl/xl? > > Yes. You have to specify "type=qusb" for usbctrl, then it will work. > I have verified that. Good, thanks.
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index bab79b1..1b88891 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -800,6 +800,16 @@ int xen_be_register(const char *type, struct XenDevOps *ops) return xenstore_scan(type, xen_domid, ops); } +void xen_be_register_common(void) +{ + xen_be_register("console", &xen_console_ops); + xen_be_register("vkbd", &xen_kbdmouse_ops); + xen_be_register("qdisk", &xen_blkdev_ops); +#ifdef CONFIG_USB_LIBUSB + xen_be_register("qusb", &xen_usb_ops); +#endif +} + int xen_be_bind_evtchn(struct XenDevice *xendev) { if (xendev->local_port != -1) { diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c index 48f725c..79aef4e 100644 --- a/hw/xenpv/xen_machine_pv.c +++ b/hw/xenpv/xen_machine_pv.c @@ -67,14 +67,9 @@ static void xen_init_pv(MachineState *machine) break; } - xen_be_register("console", &xen_console_ops); - xen_be_register("vkbd", &xen_kbdmouse_ops); + xen_be_register_common(); xen_be_register("vfb", &xen_framebuffer_ops); - xen_be_register("qdisk", &xen_blkdev_ops); xen_be_register("qnic", &xen_netdev_ops); -#ifdef CONFIG_USB_LIBUSB - xen_be_register("qusb", &xen_usb_ops); -#endif /* configure framebuffer */ if (xenfb_enabled) { diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h index 754c0a4..0df282a 100644 --- a/include/hw/xen/xen_backend.h +++ b/include/hw/xen/xen_backend.h @@ -87,6 +87,7 @@ void xen_be_check_state(struct XenDevice *xendev); /* xen backend driver bits */ int xen_be_init(void); +void xen_be_register_common(void); int xen_be_register(const char *type, struct XenDevOps *ops); int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state); int xen_be_bind_evtchn(struct XenDevice *xendev); diff --git a/xen-hvm.c b/xen-hvm.c index eb57792..3b0343a 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -1318,9 +1318,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory) error_report("xen backend core setup failed"); goto err; } - xen_be_register("console", &xen_console_ops); - xen_be_register("vkbd", &xen_kbdmouse_ops); - xen_be_register("qdisk", &xen_blkdev_ops); + xen_be_register_common(); xen_read_physmap(state); return;
Instead of calling xen_be_register() for each supported backend type for hvm and pv guests in their machine init functions use a common function in order not to have to add new backends twice. This at once fixes the error that hvm domains couldn't use the qusb backend. Signed-off-by: Juergen Gross <jgross@suse.com> --- Is it on purpose the qnic and vfb backends are not registered for HVM? --- hw/xen/xen_backend.c | 10 ++++++++++ hw/xenpv/xen_machine_pv.c | 7 +------ include/hw/xen/xen_backend.h | 1 + xen-hvm.c | 4 +--- 4 files changed, 13 insertions(+), 9 deletions(-)