[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/2] PV framebuffer



> >> diff -r 2e35cf028ff0 tools/xenfb/vncfb.c
> >> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/tools/xenfb/vncfb.c       Thu Nov 09 10:18:58 2006 +0100
> >
> >> +int main(int argc, char **argv)
> >> +{
> >> +  rfbScreenInfoPtr server;
> >> +  char *fake_argv[7] = { "vncfb", "-rfbport", "5901", 
> >> +                               "-desktop", "xen-vncfb", 
> >> +                               "-listen", "0.0.0.0" };
> > HVM VNC server defaults to listening on 127.0.0.1 now.  You might want
> > to make this do the same thing.
> Okay.
Thanks.

> > ...
> >> +  if (xenfb_hotplug(xsh, vfb_backdir) < 0)
> >> +          goto error;
> >> +  if (xenfb_hotplug(xsh, vkbd_backdir) < 0)
> >> +          goto error;
> >> +
> >> +  if (xenfb_xs_printf(xsh, vkbd_backdir, "feature-abs-pointer", "1"))
> >> +          goto error;
> >> +  if (xenfb_xs_printf(xsh, vfb_backdir, "state", "%d",
> >> +                      XenbusStateInitWait))
> >> +          goto error;
> >> +  if (xenfb_xs_printf(xsh, vkbd_backdir, "state", "%d",
> >> +                      XenbusStateInitWait))
> >> +          goto error;
> >> +
> > I'd probably reorder this a little to look more like this:
> >
> > (1) Set feature-abs-pointer
> > (2) Set state to InitWait
> > (3) Set hotplug status
> >
> > The only actual *required* constraint is (1) before (2), so that the
> > frontend doesn't initialise before we've set the feature and
> > potentially miss it.
> >
> > (2) before (3) is kind of nice, in that it makes sure that the backend
> > is ready before xend unpauses the domain, and so the frontend'll be
> > able to connect the first time it tries, but that's a lot less
> > important here than for e.g. block devices.
> Based on our previous discussions, I designed the startup protocol
> this way:
> 
>     backend                          frontend
>     ------------------------------------------------------------------
>     hotplug_status = connected
>     [makes xend populate xenstore, set fe and be state = Initialising]
>     wait for be state = Initialising
>       [i.e. wait for xend]
>     write xs: feature-abs-pointer    write xs: feature-update
>     be state = InitWait              fe state = Initialised
>     ------------------------------ sync ------------------------------
>     wait for fe state = Initialised  wait for be state = InitWait
>     ------------------------------ sync ------------------------------
>     read xs: feature-update          read xs: feature-abs-pointer
>     write xs: request-update         write xs: request-abs-pointer
>     be state = Connected             fe state = Connected
>     ------------------------------ sync ------------------------------
>     wait for fe state = Connected    wait for be state = Connected
>     ------------------------------ sync ------------------------------
>     read xs: request-abs-pointer     read xs: request-update
> 
> The symmetry made sense to me.
Ah, sorry, I wasn't clear enough.  You've got everything right after
the first sync line, but the bit before that isn't quite right.  Xend
creates xenstore area when the domain is created, and then waits until
hotplug-status is set before starting the domain running.  The idea is
that the backend driver should be watching its area of xenbus (so
/local/domain/0/backend/vif, say), so that it notices when the area is
created and creates the appropriate backend devices.  Creating the
backend devices triggers the hotplug scripts via some udev magic which
I've never quite understood, and they then e.g. connect vifs up to the
bridge.  Once they've finished, the backends are all ready, and so
it's safe to start the guest.  If you start the guest before the
backends are ready, you potentially have issues like your root
fileystem not becoming available until after the guest has booted,
which tends to make Linux unhappy.

xend                            backend driver          hotplug scripts
Creates a new domain
        paused
Creates backend area
                                Notices new backend
                                        area, creates
                                        backend device
                                Does some basic setup
                                        on backend device
                                Go to state InitWait
                                Kicks udev
                                                        Does a bit more setup
                                                                on backend
                                                                device
                                                        Sets hotplug-status
Notices hotplug status,
        unpauses domain

Now, the obvious mapping of this protocol onto the PVFB would have
xend create the xenstore area when the guest is created and spawn the
backend itself.  The backend could then set hotplug-status to indicate
that it's ready, which would unpause the guest and things would then
proceed in the usual way.

This would work, and I'd be quite happy with it, but it does have the
slight disadvantage that you can't start a domain with a framebuffer
configured and no backend attached.  If this worries you, you could
have xend not wait for the hotplug status but instead start the domain
immediately (there's a waitForBackend method on class DevController
which you'd have to override.  I don't remember the details, but I
don't think it's very hard).  If you go this path, you need to
consider the possibility that the backend area isn't set up in time
when the _probe function is run in the frontend, but I think that
works already.

> >> + error:
> >> +  serrno = errno;
> >> +  if (xenfb->fb)
> >> +          munmap(xenfb->fb, xenfb->fb_len);
> >> +  if (fbmfns)
> >> +          munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE);
> >> +        if (xenfb->kbd_info)
> >> +          munmap(xenfb->kbd_info, XC_PAGE_SIZE);
> >> +        if (xenfb->fb_info)
> >> +          munmap(xenfb->fb_info, XC_PAGE_SIZE);
> >> +        if (xenfb->kbd_port);
> >> +  xc_evtchn_unbind(xenfb->evt_xch, xenfb->kbd_port);
> >> +        if (xenfb->fbdev_port)
> >> +          xc_evtchn_unbind(xenfb->evt_xch, xenfb->fbdev_port);
> >> +        if (xsh) 
> >> +          xs_daemon_close(xsh);
> >> +        errno = serrno;
> > It'd be good to set an error in xenbus when this fails.  Look at
> > xenbus_dev_fatal and _dev_error in drivers/xen/xenbus/xenbus_client.c
> > to see how to do this.
> Thanks, I'll look into that.
Thanks.

Steven.

Attachment: signature.asc
Description: Digital signature

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.