On Tue, 2011-03-08 at 14:35 +0000, Gianni Tedesco wrote:
> On Tue, 2011-03-08 at 14:17 +0000, Ian Campbell wrote:
> > On Tue, 2011-03-08 at 12:56 +0000, Gianni Tedesco wrote:
> > > On Tue, 2011-03-08 at 11:49 +0000, Ian Campbell wrote:
> > > > # HG changeset patch
> > > > # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > > # Date 1299584929 0
> > > > # Node ID 5084214b8983045d789a86c01e7a0fede46b5e58
> > > > # Parent 0e3211b5c4da98d170ed665c221bcb00e771fc56
> > > > libxl: do not rely on guest to respond when forcing pci device removal
> > > >
> > > > This is consistent with the expected semantics of a forced device
> > > > removal and also avoids a delay when destroying an HVM domain which
> > > > either does not support hot unplug (does not respond to SCI) or has
> > > > crashed.
> > > >
> > > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > >
> > > > diff -r 0e3211b5c4da -r 5084214b8983 tools/libxl/libxl_pci.c
> > > > --- a/tools/libxl/libxl_pci.c Tue Mar 08 11:13:12 2011 +0000
> > > > +++ b/tools/libxl/libxl_pci.c Tue Mar 08 11:48:49 2011 +0000
> > > > @@ -866,7 +866,7 @@ static int do_pci_remove(libxl__gc *gc,
> > > >
> > > > /* Remove all functions at once atomically by only signalling
> > > > * device-model for function 0 */
> > > > - if ( (pcidev->vdevfn & 0x7) == 0 ) {
> > > > + if ( !force && (pcidev->vdevfn & 0x7) == 0 ) {
> > > > xs_write(ctx->xsh, XBT_NULL, path, "pci-rem",
> > > > strlen("pci-rem"));
> > >
> > > Shouldn't we maybe send the pci-rem when force removing to give the
> > > guest a chance to do something if it can.
> >
> > My concern was just that if this wasn't reacted to by qemu it might
> > interfere with us sending other requests in the future (I don't know and
> > haven't checked if that is the case).
>
> It's possible but I think it's unlikely. Generally the qemu hardware
> emulation works in discrete steps advancing the state machine. The
> xenstore watching bits work in pretty much the same way.
>
> > >
> > > > if (libxl__wait_for_device_model(ctx, domid,
> > > > "pci-removed", NULL, NULL) < 0) {
> > > && !force ) {
> > >
> > > perhaps?
> >
> > Did you mean "!force && libxl__wait..." iow you need the !force to
> > short-circuit the waiting which was the point of the patch.
>
> No, I meant to do the wait first but just not whinge about it if it
> times out. But the other way could make sense too, I'll get to that.
The wait and the timeout are the specific issue I am trying to address.
When I type "xl destroy foo" I expect domain foo to get shot in the head
right now, no questions and no hanging around. Similarly if I do "xl
pci-detach --force...".
> I'm just saying, I think force should try and do things in the normal
> way and the only difference should be that when it completes, the device
> is always removed regardless of any guest non-cooperativeness.
>
> OTOH Stefano makes a good point that this code is also called on
> shutdown as well as if user requests forceful unplug so in the former
> case doing the hotplug sequence is pointless and in the latter case the
> user probably already tried a clean hotplug and it failed.
In the shutdown case libxl_domain_destroy is called in response to the
LIBXL_EVENT_DOMAIN_DEATH event. Nothing is alive to respond to any
unplug request.
Ian.
>
> Gianni
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home