Re: [Xen-devel] [PATCH QEMU-XEN v3 5/8] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 15 Oct 2015, Ian Campbell wrote:
> On Wed, 2015-10-14 at 18:17 +0100, Stefano Stabellini wrote:
> > On Wed, 14 Oct 2015, Ian Campbell wrote:
> > > On Wed, 2015-10-14 at 17:29 +0100, Stefano Stabellini wrote:
> > > 
> > > > > Obviously the call to xen_be_unbind_evtchn is not useful as is, but
> > > > > I do
> > > > > wonder where the evtchn which the primary console must have
> > > > > somewhere
> > > > > actually is then...
> > > > 
> > > > Actually I think that xen_be_unbind_evtchn might be useful too, I
> > > > think
> > > > that is the primary console evtchn. I wonder what specific bug I was
> > > > trying to fix when I introduced that if (!xendev->dev) check.
> > > 
> > > I misread xen_be_unbind_evtchn(&con->xendev) as taking xendev->dev
> > > instead,
> > > which would be NULL and hence pointless... But given that it isn't then
> > > yes
> > > it seems like it would be worth calling.
> > > 
> > > Is it not the case that &con->xendev == xendev here, leading to another
> > > potential cleanup?
> > 
> > Yes, it is the same
> 
> I ended up with this at the head of the queue. I've not tested yet, how
> would I, AFAIK this code is mainly to do with the PV guest console, which
> would normally be handled by xenconsoled and not qemu at all. Can I force
> this to run for HVM guests or can I force a PV guest not to use xenconsoled
> (hacky ways are fine)?

Yes, you just need to set consback to LIBXL__CONSOLE_BACKEND_IOEMU.
Replacing LIBXL__CONSOLE_BACKEND_XENCONSOLED with
LIBXL__CONSOLE_BACKEND_IOEMU in init_console_info is probably enough.


> commit cbb9efe66938e44cc9ecca5a4c15cecced7f1385
> Author: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Date:   Wed Oct 14 16:49:25 2015 +0100
> 
>     xen_console: correctly cleanup primary console on teardown.
>     
>     All of the work in con_disconnect applies to the primary console case
>     (when xendev->dev is NULL). Therefore remove the early check and bail
>     and allow it to fall through. All of the existing code is correctly
>     conditional already.
>     
>     The ->dev and ->gnttabdev handles are either both set or neither. For
>     consistency with con_initialise() with to the former here too.
>     
>     With this con_initialise and con_disconnect now mirror each other.
>     
>     Fix up a hard tab in the function while editing.
>     
>     Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

I think it's good.


>     ---
>     v4: New patch based on feedback to "xen: Switch uses of
>     xc_map_foreign_bulk to use libxenforeignmemory API."
> 
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index eb7f450..63ade33 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -265,9 +265,6 @@ static void con_disconnect(struct XenDevice *xendev)
>  {
>      struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
>  
> -    if (!xendev->dev) {
> -        return;
> -    }
>      if (con->chr) {
>          qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL);
>          qemu_chr_fe_release(con->chr);
> @@ -275,12 +272,12 @@ static void con_disconnect(struct XenDevice *xendev)
>      xen_be_unbind_evtchn(&con->xendev);
>  
>      if (con->sring) {
> -        if (!xendev->gnttabdev) {
> +        if (!xendev->dev) {
>              munmap(con->sring, XC_PAGE_SIZE);
>          } else {
>              xc_gnttab_munmap(xendev->gnttabdev, con->sring, 1);
>          }
> -     con->sring = NULL;
> +        con->sring = NULL;
>      }
>  }
>  
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel