WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xenfb: make restartable

To: Markus Armbruster <armbru@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xenfb: make restartable
From: Samuel Thibault <samuel.thibault@xxxxxxxxxxxxx>
Date: Fri, 15 Aug 2008 23:23:46 +0100
Cc: Xen Development Mailing List <xen-devel@xxxxxxxxxxxxxxxxxxx>, Gerd Hoffmann <kraxel@xxxxxxxxxx>
Delivery-date: Fri, 15 Aug 2008 15:24:40 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <m3bpzubggl.fsf@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Mail-followup-to: Samuel Thibault <samuel.thibault@xxxxxxxxxxxxx>, Markus Armbruster <armbru@xxxxxxxxxx>, Gerd Hoffmann <kraxel@xxxxxxxxxx>, Xen Development Mailing List <xen-devel@xxxxxxxxxxxxxxxxxxx>
References: <48A40B17.2090701@xxxxxxxxxx> <20080814111520.GH4590@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <48A4232B.6040500@xxxxxxxxxx> <20080814122931.GL4590@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <48A43EAD.7040701@xxxxxxxxxx> <20080814142849.GS4590@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <48A44967.4060401@xxxxxxxxxx> <20080814151359.GT4590@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20080814180251.GD4590@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <m3bpzubggl.fsf@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.12-2006-07-14
Markus Armbruster, le Fri 15 Aug 2008 08:39:06 -0400, a écrit :
> > +   if (!strcmp(dev->devicetype, "vfb")) {
> > +       if (dev->xenfb->pixels) {
> > +               munmap(dev->xenfb->pixels, dev->xenfb->fb_len);
> > +               dev->xenfb->pixels = NULL;
> > +       }
> > +   }
> >  }
> >  
> >  
> 
> The check for vfb is ugly.  The only alternative I can see is fb ==
> dev->xenfb->fb.  Matter of taste.

Hum, that was what I had written previously actually.  I don't see why
it is "ugly", in that pixels is a device resource.  It should probably
be in xenfb->fb actually.

> > @@ -653,6 +656,16 @@ static int xenfb_on_state_change(struct 
> >                not much point in us continuing. */
> >             return -1;
> >     case XenbusStateInitialising:
> > +           if (dev->state != XenbusStateClosed)
> > +               /* Do not let a domain make us skip the closing state */
> > +               return 0;
> 
> Why does this work?

It's meant to not work.  If the frontend is not playing well (going
through the closing state), then ignore what it's doing until it plays
well. An alternative could be to just shut down.  In any case, we don't
want to let it drive us to remapping something else without unmapping
the previous fb.

> > +           xenfb_switch_state(dev, XenbusStateInitWait);
> > +           xs_unwatch(dev->xenfb->xsh, dev->otherend, "");
> > +           if (!strcmp(dev->devicetype, "vkbd")) {
> > +               fprintf(stderr, "FB: Waiting for KBD backend creation\n");
> > +               xenfb_wait_for_frontend(&dev->xenfb->kbd, 
> > xenfb_frontend_initialized_kbd);
> > +           }
> > +           break;
> 
> Man, I hate this state machine... 

I agree :)

> Gerd's new one is much clearer (I asked for that).

Then since 3.3 is about to be released, we'd probably have this change
with it.

Samuel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel