Markus Armbruster <armbru@xxxxxxxxxx> writes:
> 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.
Not quite. I fear there's a race.
My xenkbd otherend_changed callback runs twice with backend_state ==
XenbusStateConnected instead of first with XenbusStateInitWait and
then with XenbusStateConnected.
Here's what I think happens. First how backend, xend and frontend
synchronize:
backend xend frontend
------------------------------------------------------------------
create domain paused
create xenstore,
fe and be state =
Initialising
________ sync _______/
/
wait for be state
Initialising
write xs: feature-*
be state = InitWait
hotplug_status =
connected
\________ sync _______
\
wait for
hotplug_status
connected
unpause domain
\________ sync _______
\
write xs: feature-*
fe state = Initialised
\___________________ sync __________________/
/ \
wait for fe state wait for be state
Initialised InitWait
read xs: feature-* read xs: feature-*
write xs: request-* write xs: request-*
be state = Connected fe state = Connected
\___________________ sync __________________/
/ \
wait for fe state wait for be state
Connected Connected
read xs: request-* read xs: request-*
Except the frontend doesn't actually wait for be state, it runs a
callback (otherend_changed) when the state changes.
Now, when the this callback in the frontend is delayed sufficiently,
the order of events could be:
frontend backend
------------------------------------------------------------------
[...]
be state = InitWait
hotplug_status =
connected
[...]
fe state = Initialised
wait for fe state
Initialised
completes
read xs: feature-*
write xs: request-*
be state = Connected
otherend_changed runs
and sees be state
Connected
(actual trigger
was InitWait)
otherend_changed runs
and sees be state
Connected
How do you want me to deal with this?
Note the funny code in drivers/xen/blkback/xenbus.c's
frontend_changed:
case XenbusStateInitialised:
case XenbusStateConnected:
/* Ensure we connect even when two watches fire in
close successsion and we miss the intermediate value
of frontend_state. */
If this is the right thing to do there, why isn't it done elsewhere?
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home