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

[Ian Campbell <ian.campbell@citrix.com>]

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

Ian.

commit cbb9efe66938e44cc9ecca5a4c15cecced7f1385
Author: Ian Campbell <ian.campbell@citrix.com>
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@citrix.com>
    ---
    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;
     }
 }