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

[Markus Armbruster <armbru@xxxxxxxxxx>]

Steven Smith <sos22@xxxxxxxxx> writes:

[...]
>> >> + 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.

I coded this, and it works.

> 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.

I like this, but I'm saving it for another day.

Thanks again for your help!

[...]

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