#6 - linux: pv drivers miss shutdown command if issued too early

Owner: George Dunlap <George.Dunlap@eu.citrix.com>

Date: Wed May 22 11:36:51 2013

Last Update: Wed May 22 11:36:51 2013

Severity: normal

Affects:

State: Closed

[ Retrieve as mbox ]


Missing Control message: <CAFLBxZaTr0W31mT4pikKfw1FcLpgbi4D=FedtLrX0TSKXf-UMA@mail.gmail.com>; (Archives: gmane, marc.info)


Missing Control message: <20141111150258.GA11580@laptop.dumpdata.com>; (Archives: gmane, marc.info)


From: George Dunlap <George.Dunlap@eu.citrix.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Diana Crisan <dcrisan@flexiant.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 15:16:33 +0100
Message-ID: <CAFLBxZYCQdPU3_iTj_j-OAcXy2q0e=3-V32=xb-sXdEC4-T=6Q@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, May 21, 2013 at 2:39 PM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> On Sat, May 18, 2013 at 10:55 AM, Alex Bligh <alex@alex.org.uk> wrote:
>> Ian,
>>
>>
>> --On 17 May 2013 18:35:51 +0100 Ian Campbell <ian.campbell@citrix.com>
>> wrote:
>>
>>>> We are using Xen-4.3-rc1 with dom0 running Ubuntu Precise and
>>>> 3.5.0-23-generic kernel, and domU running Ubuntu Precise (12.04) cloud
>>>> images running 3.2.0-39-virtual. We are using the xl.conf below on
>>>> qemu-upstream-dm and HVM and two identical sending and receiving
>>>> machines (hardware and software)
>>>>
>>>> If a dom-U HVM server is started with ACPI enabled, and an ACPI event
>>>> (such as shutdown or reboot) is sent to domU before the BIOS and/or
>>>> operating system in the dom-U has initialised ACPI, then all
>>>> subsequent ACPI events are ignored.
>>>
>>>
>>> I suppose something must be getting latched and since no one was around
>>> at the time to clear it no further events are possible. Not sure if this
>>> is a qemu or a guest issue though.
>>
>>
>> I can confirm it does not happen with KVM with the same guest, and it
>> happens with multiple Linux guests on Xen. Our test suite triggers it
>> every time.
>>
>> It's a pain because if someone starts a server by accident and immediately
>> goes to shut it down, they have to do an ungraceful destroy rather than
>> a clean shutdown.
>
> So this appears to be an xl toolstack thing.  I managed to reproduce
> your results using "xl shutdown -F [domain]"; but if you then do "xl
> trigger [domian] power", the domain shuts down as normal.
>
> That at least should give you a work-around so you don't have to do
> "xl destroy".
>
> I'll take a look and see if I can figure out what's going on...

Actually, this has nothing to do with ACPI at all.

"xl shutdown" will *always* attempt to issue a PV shutdown command
first.  If it thinks there is no PV drivers present, *and* the '-F'
option is added, then it will also attempt to send an acpi power
event.

So a plain "xl shutdown" maps to:
if( pv_drivers_available(domain) )
  write_xenstore_node();

The PV drivers in domU should have watches on the xenstore node
written to cause the shutdown.  But apparently there's a window
between the time pv_drivers_available() returns succeeds and the time
this watch is set up; and if the node is written between these two
times, then the notification gets dropped.

xl seems to use HVM_PARAM_CALLBACK_IRQ being non-zero as an indicator.

Dave and Paul, does this race exist in XenServer with the Citrix PV
drivers?  How does xe detect whether pv drivers are available for an
HVM domain? And will the Citrix PV drivers check for the existence of
shutdown commands before they start a watch?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Dave Scott <Dave.Scott@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 15:20:37 +0100
Message-ID: <1369146037.21246.89.camel@zakaz.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, 2013-05-21 at 15:16 +0100, George Dunlap wrote:
> And will the Citrix PV drivers check for the existence of
> shutdown commands before they start a watch? 

Watches fire immediately when you register them, to allow this race to
be easily avoided without really having to think about it.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 15:34:34 +0100
Message-ID: <519B85FA.6060809@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On 05/21/2013 03:20 PM, Ian Campbell wrote:
> On Tue, 2013-05-21 at 15:16 +0100, George Dunlap wrote:
>> And will the Citrix PV drivers check for the existence of
>> shutdown commands before they start a watch?
>
> Watches fire immediately when you register them, to allow this race to
> be easily avoided without really having to think about it.

Well then that watch mechanism is broken, at least for values written 
before some point in time:

# xenstore-ls -f /local/domain/10
[...]
/local/domain/10/control/shutdown = "poweroff"
[...]

But the domain doesn't shut down.

Konrad, sorry to keep sending stuff your way, but I'm not sure who else 
to bring in for this one.  The original thread is here:

http://www.gossamer-threads.com/lists/xen/devel/282467

But the problem actually has nothing to do with ACPI -- it's the PV 
shutdown command that is getting lost after the driver registers 
HVM_PARAM_CALLBACK_IRQ but before something else comes up that starts 
watching ../control/shutdown.

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Konrad Wilk <konrad.wilk@oracle.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 15:42:48 +0100
Message-ID: <1369147368.21246.91.camel@zakaz.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, 2013-05-21 at 15:34 +0100, George Dunlap wrote:
> On 05/21/2013 03:20 PM, Ian Campbell wrote:
> > On Tue, 2013-05-21 at 15:16 +0100, George Dunlap wrote:
> >> And will the Citrix PV drivers check for the existence of
> >> shutdown commands before they start a watch?
> >
> > Watches fire immediately when you register them, to allow this race to
> > be easily avoided without really having to think about it.
> 
> Well then that watch mechanism is broken, 

The firing happens in xenstored, and that certainly works everywhere
else, or all sorts of things would be broken.

That's not to say there isn't a problem with e.g. the kernels watch
event handling causing the watch which is fired to not get as far as the
appropriate handler.

> at least for values written 
> before some point in time:
> 
> # xenstore-ls -f /local/domain/10
> [...]
> /local/domain/10/control/shutdown = "poweroff"
> [...]
> 
> But the domain doesn't shut down.
> 
> Konrad, sorry to keep sending stuff your way, but I'm not sure who else 
> to bring in for this one.  The original thread is here:
> 
> http://www.gossamer-threads.com/lists/xen/devel/282467
> 
> But the problem actually has nothing to do with ACPI -- it's the PV 
> shutdown command that is getting lost after the driver registers 
> HVM_PARAM_CALLBACK_IRQ but before something else comes up that starts 
> watching ../control/shutdown.
> 
>   -George



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Alex Bligh <alex@alex.org.uk>
To: Ian Campbell <Ian.Campbell@citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 16:17:00 +0100
Message-ID: <86B77960F7BEC777A3C5C339@Ximines.local>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

--On 21 May 2013 15:34:34 +0100 George Dunlap <george.dunlap@eu.citrix.com> 
wrote:

> But the problem actually has nothing to do with ACPI -- it's the PV
> shutdown command that is getting lost after the driver registers
> HVM_PARAM_CALLBACK_IRQ but before something else comes up that starts
> watching ../control/shutdown.

I'm puzzled as to why you say that as the problem does not appear
without ACPI in the xl.conf file.

-- 
Alex Bligh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org, Diana Crisan <dcrisan@flexiant.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 16:36:29 +0100
Message-ID: <519B947D.8030607@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On 05/21/2013 04:17 PM, Alex Bligh wrote:
>
>
> --On 21 May 2013 15:34:34 +0100 George Dunlap
> <george.dunlap@eu.citrix.com> wrote:
>
>> But the problem actually has nothing to do with ACPI -- it's the PV
>> shutdown command that is getting lost after the driver registers
>> HVM_PARAM_CALLBACK_IRQ but before something else comes up that starts
>> watching ../control/shutdown.
>
> I'm puzzled as to why you say that as the problem does not appear
> without ACPI in the xl.conf file.

I can still get the same effect -- the only difference for me seems to 
be that the "window" in which I can send a shutdown with no effect it is 
shorter.  Maybe ACPI causes the extra few seconds at boot which makes it 
more likely you're going to hit it?

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, xen-devel@lists.xen.org, Diana Crisan <dcrisan@flexiant.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 16:51:29 +0100
Message-ID: <519B9801.3080901@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On 05/21/2013 04:36 PM, George Dunlap wrote:
> On 05/21/2013 04:17 PM, Alex Bligh wrote:
>>
>>
>> --On 21 May 2013 15:34:34 +0100 George Dunlap
>> <george.dunlap@eu.citrix.com> wrote:
>>
>>> But the problem actually has nothing to do with ACPI -- it's the PV
>>> shutdown command that is getting lost after the driver registers
>>> HVM_PARAM_CALLBACK_IRQ but before something else comes up that starts
>>> watching ../control/shutdown.
>>
>> I'm puzzled as to why you say that as the problem does not appear
>> without ACPI in the xl.conf file.
>
> I can still get the same effect -- the only difference for me seems to
> be that the "window" in which I can send a shutdown with no effect it is
> shorter.  Maybe ACPI causes the extra few seconds at boot which makes it
> more likely you're going to hit it?

If you can't trigger it manually, try something like this:

xl create dom.cfg ; while ! xl shutdown dom ; do sleep 1 ; done

Before PARAM_CALLBACK_IRQ is set, "xl shutdown dom" will fail, so this 
will keep trying every 1 second until it succeeds.  Since this is most 
likely within 1 second of PARAM_CALLBACK_IRQ being set, it should be 
within the window where the watch isn't set yet (or whatever it is that 
makes domU responsive to the PV shutdown commands).

It works reliably for me with acpi=0.

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Alex Bligh <alex@alex.org.uk>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Ian Campbell <Ian.Campbell@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 17:22:58 +0100
Message-ID: <F1543404081BA08AA1674696@Ximines.local>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

George,

--On 21 May 2013 16:51:29 +0100 George Dunlap <george.dunlap@eu.citrix.com> 
wrote:

> If you can't trigger it manually, try something like this:
>
> xl create dom.cfg ; while ! xl shutdown dom ; do sleep 1 ; done

Diana (who is actually doing the testing) says I may be wrong about it
working with ACPI. She also says that 'xl trigger power' also do not work
reliably even when the machine is fully booted :-/ She's going to file a
proper bug report.

-- 
Alex Bligh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Dave Scott <Dave.Scott@eu.citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 12:45:31 -0400
Message-ID: <20130521164531.GE3669@phenom.dumpdata.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, May 21, 2013 at 04:51:29PM +0100, George Dunlap wrote:
> On 05/21/2013 04:36 PM, George Dunlap wrote:
> >On 05/21/2013 04:17 PM, Alex Bligh wrote:
> >>
> >>
> >>--On 21 May 2013 15:34:34 +0100 George Dunlap
> >><george.dunlap@eu.citrix.com> wrote:
> >>
> >>>But the problem actually has nothing to do with ACPI -- it's the PV
> >>>shutdown command that is getting lost after the driver registers
> >>>HVM_PARAM_CALLBACK_IRQ but before something else comes up that starts
> >>>watching ../control/shutdown.
> >>
> >>I'm puzzled as to why you say that as the problem does not appear
> >>without ACPI in the xl.conf file.
> >
> >I can still get the same effect -- the only difference for me seems to
> >be that the "window" in which I can send a shutdown with no effect it is
> >shorter.  Maybe ACPI causes the extra few seconds at boot which makes it
> >more likely you're going to hit it?
> 
> If you can't trigger it manually, try something like this:
> 
> xl create dom.cfg ; while ! xl shutdown dom ; do sleep 1 ; done
> 
> Before PARAM_CALLBACK_IRQ is set, "xl shutdown dom" will fail, so
> this will keep trying every 1 second until it succeeds.  Since this
> is most likely within 1 second of PARAM_CALLBACK_IRQ being set, it
> should be within the window where the watch isn't set yet (or
> whatever it is that makes domU responsive to the PV shutdown
> commands).
> 
> It works reliably for me with acpi=0.

And with 'acpi=1' ?

From the Linux side, there are three paths that can cause a shutdown.
Internally the user can invoke poweroff, which will end up calling the
shutdown procecedure.

PV path - which works on both PVHVM and PV. This is triggered by
watch firring on control/shutdown. Thought interestingly if you wrote
the initial value of "poweroff" there, the kernel would not read it unless
a trigger followed. A change can be done in there to read the value
at bootup and see if "poweroff" is present.

And the emulated path. I think this is ACPI S5, but I would have to
dig a bit in the QEMU to see if that is how it does it. This is the
same path that Windows or any HVM guests would shutdown.

For the emulated path I am not entirely sure how the ACPI path works 
when a guest hasn't initialized its ACPI machinery. I would think the
ACPI SCI would be triggered, but it might not be since the ACPI AML
has no way of running (as the OS hasn't even started reading it). In
which case the emulated path ought to use a fallback, whatever that is.
But I am not sure if there is a fallback except "yanking the power" - which
I think is what normal machines do if you hold the power off button for
more than 3 seconds.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Dave Scott <Dave.Scott@eu.citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>, George Dunlap <George.Dunlap@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Paul Durrant <Paul.Durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 16:51:15 +0000
Message-ID: <6FB4516F0E9B0F43B54F88D855ABB79012F62A@LONPEX01CL03.citrite.net>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

Hi,

> On Tue, 2013-05-21 at 15:34 +0100, George Dunlap wrote:
> > On 05/21/2013 03:20 PM, Ian Campbell wrote:
> > > On Tue, 2013-05-21 at 15:16 +0100, George Dunlap wrote:
> > >> And will the Citrix PV drivers check for the existence of shutdown
> > >> commands before they start a watch?
> > >
> > > Watches fire immediately when you register them, to allow this race
> > > to be easily avoided without really having to think about it.
> >
> > Well then that watch mechanism is broken,
> 
> The firing happens in xenstored, and that certainly works everywhere else,
> or all sorts of things would be broken.

IIRC the watch event should appear in the xenstore (access?) log. I agree that it's very unlikely that xenstore is broken.
 
> That's not to say there isn't a problem with e.g. the kernels watch event
> handling causing the watch which is fired to not get as far as the appropriate
> handler.

For a long time our test cases have had a 'sleep 10' after a 'xe vm-start' and before a 'xe vm-shutdown'. IIRC when I last looked into this (years ago) it was claimed that the PV kernel was trying to signal init to change runlevel, but it was too soon and the event/signal/whatever  was dropped. Recently to speed up the tests we've started switching to a custom miniOS kernel (based on Mirage) which doesn't have this problem. However it would be great to finally fix this race...

Cheers,
Dave
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Alex Bligh <alex@alex.org.uk>
To: George Dunlap <george.dunlap@eu.citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 18:48:16 +0100
Message-ID: <6D7B8289F0F8A65C8C2A25C7@Ximines.local>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

Konrad,

--On 21 May 2013 12:45:31 -0400 Konrad Rzeszutek Wilk 
<konrad.wilk@oracle.com> wrote:

> For the emulated path I am not entirely sure how the ACPI path works
> when a guest hasn't initialized its ACPI machinery. I would think the
> ACPI SCI would be triggered, but it might not be since the ACPI AML
> has no way of running (as the OS hasn't even started reading it). In
> which case the emulated path ought to use a fallback, whatever that is.
> But I am not sure if there is a fallback except "yanking the power" -
> which I think is what normal machines do if you hold the power off button
> for more than 3 seconds.

The problem isn't that a shutdown doesn't work if there are no drivers
for it. The problem is that if you issue a shutdown when there are
no drivers, then later issue a shutdown when there are drivers, it
still doesn't work.

Somewhat to my surprise we are using the PV/PVHVM route not the ACPI
route (libxl_domain_shutdown / xl shutdown) so the subject line would
appear to be inaccurate.

The second problem is that ACPI shutdown appears not to work reliably
either (whenever issued).

-- 
Alex Bligh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Dave Scott <Dave.Scott@eu.citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>, xen-devel@lists.xen.org, Diana Crisan <dcrisan@flexiant.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 15:33:44 -0400
Message-ID: <20130521193344.GB6231@phenom.dumpdata.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, May 21, 2013 at 06:48:16PM +0100, Alex Bligh wrote:
> Konrad,
> 
> --On 21 May 2013 12:45:31 -0400 Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> 
> >For the emulated path I am not entirely sure how the ACPI path works
> >when a guest hasn't initialized its ACPI machinery. I would think the
> >ACPI SCI would be triggered, but it might not be since the ACPI AML
> >has no way of running (as the OS hasn't even started reading it). In
> >which case the emulated path ought to use a fallback, whatever that is.
> >But I am not sure if there is a fallback except "yanking the power" -
> >which I think is what normal machines do if you hold the power off button
> >for more than 3 seconds.
> 
> The problem isn't that a shutdown doesn't work if there are no drivers
> for it. The problem is that if you issue a shutdown when there are
> no drivers, then later issue a shutdown when there are drivers, it
> still doesn't work.

OK, but we should be able to fix this in Linux by doing something along
these lines (not compile tested):


diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 412b96c..34f967f 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -194,7 +194,6 @@ static void do_reboot(void)
 	shutting_down = SHUTDOWN_POWEROFF; /* ? */
 	ctrl_alt_del();
 }
-
 static void shutdown_handler(struct xenbus_watch *watch,
 			     const char **vec, unsigned int len)
 {
@@ -252,6 +251,10 @@ static void shutdown_handler(struct xenbus_watch *watch,
 	kfree(str);
 }
 
+static void check_shutdown_handler(void)
+{
+	 shutdown_handler(NULL, NULL, 0);
+}
 #ifdef CONFIG_MAGIC_SYSRQ
 static void sysrq_handler(struct xenbus_watch *watch, const char **vec,
 			  unsigned int len)
@@ -310,7 +313,7 @@ static int setup_shutdown_watcher(void)
 		return err;
 	}
 #endif
-
+	check_shutdown_handler();
 	return 0;
 }
 
> 
> Somewhat to my surprise we are using the PV/PVHVM route not the ACPI
> route (libxl_domain_shutdown / xl shutdown) so the subject line would
> appear to be inaccurate.
> 
> The second problem is that ACPI shutdown appears not to work reliably
> either (whenever issued).

Right and that one would require some more investigation as I think it 
works as it would work on baremetal. Meaning if you press the 'shutdown'
button and only BIOS is running it will ignore you. This would
be similar to the 'xl trigger' call that hopefully invokes the ACPI signal.

You have to hold it for 5 seconds or so for the BIOS to do anything
about it.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Alex Bligh <alex@alex.org.uk>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Ian Campbell <Ian.Campbell@citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>, Dave Scott <Dave.Scott@eu.citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 20:46:40 +0100
Message-ID: <FE73E0372DDFA378353F4AE0@nimrod.local>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

Konrad,

--On 21 May 2013 15:33:44 -0400 Konrad Rzeszutek Wilk 
<konrad.wilk@oracle.com> wrote:

> OK, but we should be able to fix this in Linux by doing something along
> these lines (not compile tested):

Yep. Not much use on guests with current OS's though :-(

>> Somewhat to my surprise we are using the PV/PVHVM route not the ACPI
>> route (libxl_domain_shutdown / xl shutdown) so the subject line would
>> appear to be inaccurate.
>>
>> The second problem is that ACPI shutdown appears not to work reliably
>> either (whenever issued).
>
> Right and that one would require some more investigation as I think it
> works as it would work on baremetal.

Diana thinks it does not - i.e. when you have a fully booted running Ubuntu
OS, does not reliably cause shutdown (as per Diana's testing), whether
or not you send an a trigger during early boot. Pressing the hardware
button does one would presume.

> Meaning if you press the 'shutdown'
> button and only BIOS is running it will ignore you. This would
> be similar to the 'xl trigger' call that hopefully invokes the ACPI
> signal.
>
> You have to hold it for 5 seconds or so for the BIOS to do anything
> about it.

I'm fine with ACPI shutdown not working before ACPI's been initialised,
in BIOS or whatever, so long as it works it has been booted!

-- 
Alex Bligh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Dave Scott <Dave.Scott@eu.citrix.com>
Cc: Paul Durrant <Paul.Durrant@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Anthony Perard <anthony.perard@citrix.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Tue, 21 May 2013 20:58:09 +0100
Message-ID: <1369166289.12438.47.camel@dagon.hellion.org.uk>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, 2013-05-21 at 17:51 +0100, Dave Scott wrote:
> For a long time our test cases have had a 'sleep 10' after a 'xe
> vm-start' and before a 'xe vm-shutdown'. IIRC when I last looked into
> this (years ago) it was claimed that the PV kernel was trying to
> signal init to change runlevel, but it was too soon and the
> event/signal/whatever  was dropped. Recently to speed up the tests
> we've started switching to a custom miniOS kernel (based on Mirage)
> which doesn't have this problem. However it would be great to finally
> fix this race...

Oh yes, this is true -- there is an issue with the guest internal (so ~=
native) shutdown stuff not working if you signal it too early. i.e. you
are confusing /sbin/init and not anything Xen specific.

I expect that the way to determine this would be to check if the
xenstore key was "" or "something". If it is blank then the guest has
acknowledged the request and the fault is /sbin/init otherwise it is a
Xen specific issue, probably internal to the kernel though IMHO.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <George.Dunlap@eu.citrix.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Diana Crisan <dcrisan@flexiant.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 10:21:44 +0100
Message-ID: <CAFLBxZaGvkOMa3D8VBDWSNkUaC_6yHo1bsWeLAYC=t=tP67Yxg@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Tue, May 21, 2013 at 6:48 PM, Alex Bligh <alex@alex.org.uk> wrote:
> The problem isn't that a shutdown doesn't work if there are no drivers
> for it. The problem is that if you issue a shutdown when there are
> no drivers, then later issue a shutdown when there are drivers, it
> still doesn't work.

Just to be clear, the issue is if you issue a shutdown when the
drivers are partially initialized, but before the whole system has
come up.  If you issue a shutdown command in BIOS or GRUB, for
example, the toolstack returns an error and doesn't cause the "wedge".

(From the other thread of the discussion, it's not 100% clear whether
the problem is that the kernel driver isn't getting the shutdown
signal, or whether it has sent the signal but it's too early for the
rest of the kernel to heed it.)

> The second problem is that ACPI shutdown appears not to work reliably
> either (whenever issued).

I'll have another look today, but it seemed fairly reliable to me.
Just to double-check -- you are issuing the ACPI commands after the VM
has come all the way up, right?  Or are you issuing them early during
boot, like you were for the xl shutdown commands?

The other thing is that I'm using Wheezy, not Ubuntu, which may cause
a difference.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Dave Scott <Dave.Scott@eu.citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 10:57:38 +0100
Message-ID: <1369216658.21246.164.camel@zakaz.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index 412b96c..34f967f 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -194,7 +194,6 @@ static void do_reboot(void)
>  	shutting_down = SHUTDOWN_POWEROFF; /* ? */
>  	ctrl_alt_del();
>  }
> -
>  static void shutdown_handler(struct xenbus_watch *watch,
>  			     const char **vec, unsigned int len)
>  {
> @@ -252,6 +251,10 @@ static void shutdown_handler(struct xenbus_watch *watch,
>  	kfree(str);
>  }
>  
> +static void check_shutdown_handler(void)
> +{
> +	 shutdown_handler(NULL, NULL, 0);
> +}
>  #ifdef CONFIG_MAGIC_SYSRQ
>  static void sysrq_handler(struct xenbus_watch *watch, const char **vec,
>  			  unsigned int len)
> @@ -310,7 +313,7 @@ static int setup_shutdown_watcher(void)
>  		return err;
>  	}
>  #endif
> -
> +	check_shutdown_handler();

If this is necessary then there is a bug in the kernel's watch handling,
for which this is just a workaround, since the call to
register_xenbus_watch should have triggered an initial watch.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Alex Bligh <alex@alex.org.uk>
To: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, Dave Scott <Dave.Scott@eu.citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 11:08:34 +0100
Message-ID: <C29108ACEBFC6FC03E9152C2@Ximines.local>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

--On 22 May 2013 10:21:44 +0100 George Dunlap <George.Dunlap@eu.citrix.com> 
wrote:

>> The second problem is that ACPI shutdown appears not to work reliably
>> either (whenever issued).
>
> I'll have another look today, but it seemed fairly reliable to me.
> Just to double-check -- you are issuing the ACPI commands after the VM
> has come all the way up, right?  Or are you issuing them early during
> boot, like you were for the xl shutdown commands?

I believe Diana has tried both. Diana?

> The other thing is that I'm using Wheezy, not Ubuntu, which may cause
> a difference.

Possibly. I guess Diana could try that too :-)

-- 
Alex Bligh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Diana Crisan <dcrisan@flexiant.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, xen-devel@lists.xen.org
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 11:45:20 +0100 (BST)
Message-ID: <1809930721.71216.1369219520131.JavaMail.root@flexiant.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

>--On 22 May 2013 10:21:44 +0100 George Dunlap <George.Dunlap@eu.citrix.com> 
>wrote:

>>> The second problem is that ACPI shutdown appears not to work reliably
>>> either (whenever issued).
>>
>> I'll have another look today, but it seemed fairly reliable to me.
>> Just to double-check -- you are issuing the ACPI commands after the VM
>> has come all the way up, right?  Or are you issuing them early during
>> boot, like you were for the xl shutdown commands?
>
>I believe Diana has tried both. Diana?

I have tested both cases with Ubuntu.
Sending the trigger is reliable if you wait for boot to fully complete. 
However, if I issue it during boot it does not get executed. Any subsequent triggers do not get executed as well until one is sent when the vm has fully booted.


>
>> The other thing is that I'm using Wheezy, not Ubuntu, which may cause
>> a difference.
>
>Possibly. I guess Diana could try that too :-)

>-- 
>Alex Bligh

-- 
Diana Crisan 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Diana Crisan <dcrisan@flexiant.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, Alex Bligh <alex@alex.org.uk>, xen-devel@lists.xen.org, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 11:55:03 +0100
Message-ID: <519CA407.7000609@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On 22/05/13 11:45, Diana Crisan wrote:
>> --On 22 May 2013 10:21:44 +0100 George Dunlap <George.Dunlap@eu.citrix.com>
>> wrote:
>>>> The second problem is that ACPI shutdown appears not to work reliably
>>>> either (whenever issued).
>>> I'll have another look today, but it seemed fairly reliable to me.
>>> Just to double-check -- you are issuing the ACPI commands after the VM
>>> has come all the way up, right?  Or are you issuing them early during
>>> boot, like you were for the xl shutdown commands?
>> I believe Diana has tried both. Diana?
> I have tested both cases with Ubuntu.
> Sending the trigger is reliable if you wait for boot to fully complete.
> However, if I issue it during boot it does not get executed. Any subsequent triggers do not get executed as well until one is sent when the vm has fully booted.

Right -- so what I hear you saying is, "ACPI commands issued before the 
OS is paying attention are ignored."  I think that's expected behavior.

I can see that "Shut this vm down as soon as possible" is a useful thing 
to have. The problem at the moment guest OSes can ignore signals sent 
too early, it doesn't really seem within the scope of libxl to work 
around that.

You could hold off issuing shutdown commands until the guest responds to 
a ping; here is a rune I use for that in my test harness:

ping -c 1 -i 5 -q -w ${timeout} ${host}

This will ping ${host} every 5 seconds until it receives 1 ping back, 
failing with an error after ${timeout} seconds.

Alternately, you could find another xenstore key that indicates that the 
guest is ready to receive a shutdown command; or you could add a line to 
/etc/rc.local to write a xenstore key once the system is done booting.

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Alex Bligh <alex@alex.org.uk>
To: Diana Crisan <dcrisan@flexiant.com>, George Dunlap <george.dunlap@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, xen-devel@lists.xen.org, Alex Bligh <alex@alex.org.uk>, Dave Scott <Dave.Scott@eu.citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 12:16:57 +0100
Message-ID: <8AFD571E8A65CC3529A5604A@Ximines.local>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

George,

--On 22 May 2013 11:55:03 +0100 George Dunlap <george.dunlap@eu.citrix.com> 
wrote:

>> I have tested both cases with Ubuntu.
>> Sending the trigger is reliable if you wait for boot to fully complete.
>> However, if I issue it during boot it does not get executed. Any
>> subsequent triggers do not get executed as well until one is sent when
>> the vm has fully booted.
>
> Right -- so what I hear you saying is, "ACPI commands issued before the
> OS is paying attention are ignored."  I think that's expected behavior.
>
> I can see that "Shut this vm down as soon as possible" is a useful thing
> to have. The problem at the moment guest OSes can ignore signals sent too
> early, it doesn't really seem within the scope of libxl to work around
> that.

I (now) think xl trigger power is working as well as could be expected.
xl shutdown has a problem (in that an early use of this
breaks later use), but we can avoid that by using xl trigger power
(or rather the libxl equivalent). So we have a workaround for this one.

FWIW our workaround actually will be send xl trigger power every
second until the machine disappears, as there is in general no IP
connectivity between hypervisor and guest in our environment. (Just
in case anyone is reading the archive and wants a hint).

So that leaves our one (hurray!) outstanding issue with xen 4.2 (and
4.3) Diana's first problem - the stuck clock after migration.

-- 
Alex Bligh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: George Dunlap <george.dunlap@eu.citrix.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, Diana Crisan <dcrisan@flexiant.com>, xen-devel@lists.xen.org
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 12:50:50 +0100
Message-ID: <519CB11A.5030907@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On 22/05/13 12:16, Alex Bligh wrote:
> George,
>
> --On 22 May 2013 11:55:03 +0100 George Dunlap 
> <george.dunlap@eu.citrix.com> wrote:
>
>>> I have tested both cases with Ubuntu.
>>> Sending the trigger is reliable if you wait for boot to fully complete.
>>> However, if I issue it during boot it does not get executed. Any
>>> subsequent triggers do not get executed as well until one is sent when
>>> the vm has fully booted.
>>
>> Right -- so what I hear you saying is, "ACPI commands issued before the
>> OS is paying attention are ignored."  I think that's expected behavior.
>>
>> I can see that "Shut this vm down as soon as possible" is a useful thing
>> to have. The problem at the moment guest OSes can ignore signals sent 
>> too
>> early, it doesn't really seem within the scope of libxl to work around
>> that.
>
> I (now) think xl trigger power is working as well as could be expected.
> xl shutdown has a problem (in that an early use of this
> breaks later use), but we can avoid that by using xl trigger power
> (or rather the libxl equivalent). So we have a workaround for this one.

OK -- this will still be on our list of bugs to track, but as it's 
probably a linux issue, it may take some time to filter back into 
distributions.

> FWIW our workaround actually will be send xl trigger power every
> second until the machine disappears, as there is in general no IP
> connectivity between hypervisor and guest in our environment. (Just
> in case anyone is reading the archive and wants a hint).

Ah right -- that makes sense.  Poking it until it shuts off it a bit 
blunt, but should be effective. :-)

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 22 May 2013 10:43:01 -0400
Message-ID: <20130522144301.GB8162@phenom.dumpdata.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Wed, May 22, 2013 at 12:50:50PM +0100, George Dunlap wrote:
> On 22/05/13 12:16, Alex Bligh wrote:
> >George,
> >
> >--On 22 May 2013 11:55:03 +0100 George Dunlap
> ><george.dunlap@eu.citrix.com> wrote:
> >
> >>>I have tested both cases with Ubuntu.
> >>>Sending the trigger is reliable if you wait for boot to fully complete.
> >>>However, if I issue it during boot it does not get executed. Any
> >>>subsequent triggers do not get executed as well until one is sent when
> >>>the vm has fully booted.
> >>
> >>Right -- so what I hear you saying is, "ACPI commands issued before the
> >>OS is paying attention are ignored."  I think that's expected behavior.
> >>
> >>I can see that "Shut this vm down as soon as possible" is a useful thing
> >>to have. The problem at the moment guest OSes can ignore signals
> >>sent too
> >>early, it doesn't really seem within the scope of libxl to work around
> >>that.
> >
> >I (now) think xl trigger power is working as well as could be expected.
> >xl shutdown has a problem (in that an early use of this
> >breaks later use), but we can avoid that by using xl trigger power
> >(or rather the libxl equivalent). So we have a workaround for this one.
> 
> OK -- this will still be on our list of bugs to track, but as it's
> probably a linux issue, it may take some time to filter back into
> distributions.

There is an bug somewhere. I did this:

#xl create /pv.cfg && xl pause latest && xl shutdown latest
#xenstore-ls | grep poweroff
    shutdown = "poweroff"
#xl unpause latest
#xl console latest
...

and the guest continues on as if the change never happend and won't
shutdown.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Dave Scott <Dave.Scott@eu.citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, Ian Campbell <Ian.Campbell@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 6 Nov 2013 11:05:08 -0500
Message-ID: <20131106160508.GA8490@phenom.dumpdata.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Wed, May 22, 2013 at 10:43:01AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, May 22, 2013 at 12:50:50PM +0100, George Dunlap wrote:
> > On 22/05/13 12:16, Alex Bligh wrote:
> > >George,
> > >
> > >--On 22 May 2013 11:55:03 +0100 George Dunlap
> > ><george.dunlap@eu.citrix.com> wrote:
> > >
> > >>>I have tested both cases with Ubuntu.
> > >>>Sending the trigger is reliable if you wait for boot to fully complete.
> > >>>However, if I issue it during boot it does not get executed. Any
> > >>>subsequent triggers do not get executed as well until one is sent when
> > >>>the vm has fully booted.
> > >>
> > >>Right -- so what I hear you saying is, "ACPI commands issued before the
> > >>OS is paying attention are ignored."  I think that's expected behavior.
> > >>
> > >>I can see that "Shut this vm down as soon as possible" is a useful thing
> > >>to have. The problem at the moment guest OSes can ignore signals
> > >>sent too
> > >>early, it doesn't really seem within the scope of libxl to work around
> > >>that.
> > >
> > >I (now) think xl trigger power is working as well as could be expected.
> > >xl shutdown has a problem (in that an early use of this
> > >breaks later use), but we can avoid that by using xl trigger power
> > >(or rather the libxl equivalent). So we have a workaround for this one.
> > 
> > OK -- this will still be on our list of bugs to track, but as it's
> > probably a linux issue, it may take some time to filter back into
> > distributions.
> 
> There is an bug somewhere. I did this:
> 
> #xl create /pv.cfg && xl pause latest && xl shutdown latest
> #xenstore-ls | grep poweroff
>     shutdown = "poweroff"
> #xl unpause latest
> #xl console latest
> ...
> 
> and the guest continues on as if the change never happend and won't
> shutdown.

This patch should fix it.
From f53c1f691f726a9d72ad11be56f84389c3f3d7b0 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 6 Nov 2013 10:57:56 -0500
Subject: [PATCH] xen/manage: Poweroff forcefully if user-space is not yet up.

The user can launch the guest in this sequence:

xl create -p /vm.cfg	[launch, but pause it]
xl shutdown latest	[sets control/shutdown=poweroff]
xl unpause latest
xl console latest	[and see that the guest has completlty
ignored the shutdown request]

In reality the guest hasn't ignored it. It registers a watch
and gets a notification that there is value. It then calls
the shutdown_handler which ends up calling orderly_shutdown.

Unfortunately that is so early in the bootup that there
are no user-space. Which means that the orderly_shutdown fails.
But since the force flag was set to false it continues on without
reporting.

We check if the system is still in the booting stage and if so
enable the force option (which will shutdown in early bootup
process). If in normal running case we don't force it.

Fixes-Bug: http://bugs.xenproject.org/xen/bug/6
Reported-by:  Alex Bligh <alex@alex.org.uk>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/manage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 624e8dc..fe1c0a6 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -185,7 +185,7 @@ struct shutdown_handler {
 static void do_poweroff(void)
 {
 	shutting_down = SHUTDOWN_POWEROFF;
-	orderly_poweroff(false);
+	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
 }
 
 static void do_reboot(void)
-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>, Diana Crisan <dcrisan@flexiant.com>, Alex Bligh <alex@alex.org.uk>, Dave Scott <Dave.Scott@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 6 Nov 2013 16:14:20 +0000
Message-ID: <1383754460.26213.133.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Wed, 2013-11-06 at 11:05 -0500, Konrad Rzeszutek Wilk wrote:
> From f53c1f691f726a9d72ad11be56f84389c3f3d7b0 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Wed, 6 Nov 2013 10:57:56 -0500
> Subject: [PATCH] xen/manage: Poweroff forcefully if user-space is not yet up.
> 
> The user can launch the guest in this sequence:
> 
> xl create -p /vm.cfg	[launch, but pause it]
> xl shutdown latest	[sets control/shutdown=poweroff]
> xl unpause latest
> xl console latest	[and see that the guest has completlty

"completely"

> ignored the shutdown request]
> 
> In reality the guest hasn't ignored it. It registers a watch
> and gets a notification that there is value. It then calls
> the shutdown_handler which ends up calling orderly_shutdown.
> 
> Unfortunately that is so early in the bootup that there
> are no user-space. Which means that the orderly_shutdown fails.
> But since the force flag was set to false it continues on without
> reporting.
> 
> We check if the system is still in the booting stage and if so
> enable the force option (which will shutdown in early bootup
> process). If in normal running case we don't force it.
> 
> Fixes-Bug: http://bugs.xenproject.org/xen/bug/6
> Reported-by:  Alex Bligh <alex@alex.org.uk>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/xen/manage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index 624e8dc..fe1c0a6 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -185,7 +185,7 @@ struct shutdown_handler {
>  static void do_poweroff(void)
>  {
>  	shutting_down = SHUTDOWN_POWEROFF;
> -	orderly_poweroff(false);
> +	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);

Does this DTRT for systems in SYSTEM_{HALTED,POWEROFF}? I suppose under
those circumstances forcing is the desired action, insn't it

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: "Jan Beulich" <JBeulich@suse.com>
To: "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>
Cc: Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, George Dunlap <george.dunlap@eu.citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, Dave Scott <Dave.Scott@eu.citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 06 Nov 2013 16:18:15 +0000
Message-ID: <527A79D702000078001003FC@nat28.tlf.novell.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

>>> On 06.11.13 at 17:05, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -185,7 +185,7 @@ struct shutdown_handler {
>  static void do_poweroff(void)
>  {
>  	shutting_down = SHUTDOWN_POWEROFF;
> -	orderly_poweroff(false);
> +	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);

What's the ?: operator good for here?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Dave Scott <Dave.Scott@eu.citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, George Dunlap <george.dunlap@eu.citrix.com>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Wed, 6 Nov 2013 15:16:37 -0500
Message-ID: <20131106201637.GA21935@phenom.dumpdata.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Wed, Nov 06, 2013 at 04:14:20PM +0000, Ian Campbell wrote:
> On Wed, 2013-11-06 at 11:05 -0500, Konrad Rzeszutek Wilk wrote:
> > From f53c1f691f726a9d72ad11be56f84389c3f3d7b0 Mon Sep 17 00:00:00 2001
> > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Date: Wed, 6 Nov 2013 10:57:56 -0500
> > Subject: [PATCH] xen/manage: Poweroff forcefully if user-space is not yet up.
> > 
> > The user can launch the guest in this sequence:
> > 
> > xl create -p /vm.cfg	[launch, but pause it]
> > xl shutdown latest	[sets control/shutdown=poweroff]
> > xl unpause latest
> > xl console latest	[and see that the guest has completlty
> 
> "completely"
> 
> > ignored the shutdown request]
> > 
> > In reality the guest hasn't ignored it. It registers a watch
> > and gets a notification that there is value. It then calls
> > the shutdown_handler which ends up calling orderly_shutdown.
> > 
> > Unfortunately that is so early in the bootup that there
> > are no user-space. Which means that the orderly_shutdown fails.
> > But since the force flag was set to false it continues on without
> > reporting.
> > 
> > We check if the system is still in the booting stage and if so
> > enable the force option (which will shutdown in early bootup
> > process). If in normal running case we don't force it.
> > 
> > Fixes-Bug: http://bugs.xenproject.org/xen/bug/6
> > Reported-by:  Alex Bligh <alex@alex.org.uk>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  drivers/xen/manage.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> > index 624e8dc..fe1c0a6 100644
> > --- a/drivers/xen/manage.c
> > +++ b/drivers/xen/manage.c
> > @@ -185,7 +185,7 @@ struct shutdown_handler {
> >  static void do_poweroff(void)
> >  {
> >  	shutting_down = SHUTDOWN_POWEROFF;
> > -	orderly_poweroff(false);
> > +	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
> 
> Does this DTRT for systems in SYSTEM_{HALTED,POWEROFF}? I suppose under
> those circumstances forcing is the desired action, insn't it

Yes. And there is also a guard (shutting_down) that gets set on
drivers/xen/manage.c so that the 'do_poweroff' will only get called
once. Which would guard against us powering off and then
receiving another 'xl shutdown' when the the system_state is in
HALTED or POWEROFF.

But I hadn't tested the case where the user does 'poweroff'
and at the same time the system admin does 'xl shutdown'.

Depending on the race, the state will be SYSTEM_RUNNING or
SYSTEM_POWER_OFF. If SYSTEM_RUNNING we just end up making
a duplicate call to 'poweroff' (while it is running).

That will fail or execute (And if executed then it will be
stuck in the reboot_mutex mutex). But nobody will care b/c the
machine is in poweroff sequence.

If the state is SYSTEM_POWER_OFF then we end up making
a duplicate call to kernel_power_off. There is no locking
there so we walk in the same steps as what 'poweroff'
has been doing.

This code in kernel/reboot.c doesn't look that safe when it
comes to a user-invoked 'poweroff' operation and a kernel
'orderly_poweroff' operation.

Perhaps what we should do is just:

	if (system_state == SYSTEM_BOOTING)
		orderly_poweroff(true);
	else if (system_state == SYSTEM_RUNNING)
		orderly_poweroff(false);
	else
		printk("Shutdown in progress. Ignoring xl shutdown");

But then 'system_state' is not guarded by a spinlock or such. Thought
it is guarded by the xenwatch mutex.

Perhaps to be extra safe we should add ourselves to the
register_reboot_notifier like so (not compile tested)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index fe1c0a6..fb43db6 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -36,7 +36,8 @@ enum shutdown_state {
 	 SHUTDOWN_HALT = 4,
 };
 
-/* Ignore multiple shutdown requests. */
+/* Ignore multiple shutdown requests. Our mutex for this is that
+ * shutdown handler is called with a mutex from xenwatch. */
 static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
 
 struct suspend_info {
@@ -185,7 +186,12 @@ struct shutdown_handler {
 static void do_poweroff(void)
 {
 	shutting_down = SHUTDOWN_POWEROFF;
-	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
+	if (system_state == SYSTEM_RUNNING)
+		orderly_poweroff(false);
+	else if (system_state == SYSTEM_BOOTING)
+		orderly_poweroff(true);
+	else
+		printk(KERN_WARNING "Ignorning shutdown request as poweroff in progress.\n");
 }
 
 static void do_reboot(void)
@@ -250,7 +256,29 @@ static void shutdown_handler(struct xenbus_watch *watch,
 
 	kfree(str);
 }
+/*
+ * This function is called when the system is being rebooted.
+ */
+static int
+xxen_system_reboot(struct notifier_block *nb, unsigned long event, void *unused)
+{
+	switch (event) {
+	case SYS_RESTART:
+	case SYS_HALT:
+	case SYS_POWER_OFF:
+	default:
+		mutex_lock(&xenwatch_mutex);
+		shutting_down = SHUTDOWN_POWEROFF;
+		mutex_unlock(&xenwatch_mutex);
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
 
+static struct notifier_block xen_shutdown_notifier = {
+        .notifier_call = xen_system_reboot,
+};
 #ifdef CONFIG_MAGIC_SYSRQ
 static void sysrq_handler(struct xenbus_watch *watch, const char **vec,
 			  unsigned int len)
@@ -308,7 +336,11 @@ static int setup_shutdown_watcher(void)
 		return err;
 	}
 #endif
-
+	err = register_reboot_notifier(&xen_shutdown_notifier);
+	if (err) {
+		pr_warn("Failed to register shutdown notifier\n");
+		return err;
+	}
 	return 0;
 }
 
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index b6d5fff..ac25752 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -122,7 +122,7 @@ static DEFINE_SPINLOCK(watch_events_lock);
  * carrying out work.
  */
 static pid_t xenwatch_pid;
-static DEFINE_MUTEX(xenwatch_mutex);
+DEFINE_MUTEX(xenwatch_mutex);
 static DECLARE_WAIT_QUEUE_HEAD(watch_events_waitq);
 
 static int get_error(const char *errorstring)
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 40abaf6..57b3370 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -125,7 +125,7 @@ struct xenbus_transaction
 {
 	u32 id;
 };
-
+extern struct mutex xenwatch_mutex;
 /* Nil transaction ID. */
 #define XBT_NIL ((struct xenbus_transaction) { 0 })
 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul.durrant@citrix.com>, xen-devel@lists.xen.org, Stefano Stabellini <stefano.stabellini@citrix.com>, Dave Scott <Dave.Scott@eu.citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, George Dunlap <george.dunlap@eu.citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Thu, 7 Nov 2013 11:24:45 +0000
Message-ID: <1383823485.26213.186.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

> > @@ -185,7 +185,7 @@ struct shutdown_handler {
> > >  static void do_poweroff(void)
> > >  {
> > >  	shutting_down = SHUTDOWN_POWEROFF;
> > > -	orderly_poweroff(false);
> > > +	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
> > 
> > Does this DTRT for systems in SYSTEM_{HALTED,POWEROFF}? I suppose under
> > those circumstances forcing is the desired action, insn't it
> 
> Yes. And there is also a guard (shutting_down) that gets set on
> drivers/xen/manage.c so that the 'do_poweroff' will only get called
> once. Which would guard against us powering off and then
> receiving another 'xl shutdown' when the the system_state is in
> HALTED or POWEROFF.
> 
> But I hadn't tested the case where the user does 'poweroff'
> and at the same time the system admin does 'xl shutdown'.
> 
> Depending on the race, the state will be SYSTEM_RUNNING or
> SYSTEM_POWER_OFF. If SYSTEM_RUNNING we just end up making
> a duplicate call to 'poweroff' (while it is running).
> 
> That will fail or execute (And if executed then it will be
> stuck in the reboot_mutex mutex). But nobody will care b/c the
> machine is in poweroff sequence.

Right.

> If the state is SYSTEM_POWER_OFF then we end up making
> a duplicate call to kernel_power_off. There is no locking
> there so we walk in the same steps as what 'poweroff'
> has been doing.
> 
> This code in kernel/reboot.c doesn't look that safe when it
> comes to a user-invoked 'poweroff' operation and a kernel
> 'orderly_poweroff' operation.

Yes.

> Perhaps what we should do is just:
> 
> 	if (system_state == SYSTEM_BOOTING)
> 		orderly_poweroff(true);
> 	else if (system_state == SYSTEM_RUNNING)
> 		orderly_poweroff(false);
> 	else
> 		printk("Shutdown in progress. Ignoring xl shutdown");

(nb: switch() ;-)). I would also avoiding saying xl since it may not be
true. "Ignoring Xen toolstack shutdown" or something

> But then 'system_state' is not guarded by a spinlock or such. Thought
> it is guarded by the xenwatch mutex.

system_state is a core global though, so it must surely also be touched
outside of xen code and therefore outside of xenwatch mutex.

maybe you meant s/system_state/shutting_down/?

> Perhaps to be extra safe we should add ourselves to the
> register_reboot_notifier like so (not compile tested)

I think this only makes sense if you did mean
s/system_state/shutting_down/ above, so I'll assume that to be the case.

It's a shame this has to expose the watch mutex outside of the core xs
code. Perhaps the core code could add the notifier itself and in turn
call a manage.c notification function with the lock already held?

> 
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index fe1c0a6..fb43db6 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -36,7 +36,8 @@ enum shutdown_state {
>  	 SHUTDOWN_HALT = 4,
>  };
>  
> -/* Ignore multiple shutdown requests. */
> +/* Ignore multiple shutdown requests. Our mutex for this is that
> + * shutdown handler is called with a mutex from xenwatch. */
>  static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
>  
>  struct suspend_info {
> @@ -185,7 +186,12 @@ struct shutdown_handler {
>  static void do_poweroff(void)
>  {
>  	shutting_down = SHUTDOWN_POWEROFF;
> -	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
> +	if (system_state == SYSTEM_RUNNING)
> +		orderly_poweroff(false);
> +	else if (system_state == SYSTEM_BOOTING)
> +		orderly_poweroff(true);
> +	else
> +		printk(KERN_WARNING "Ignorning shutdown request as poweroff in progress.\n");
>  }
>  
>  static void do_reboot(void)
> @@ -250,7 +256,29 @@ static void shutdown_handler(struct xenbus_watch *watch,
>  
>  	kfree(str);
>  }
> +/*
> + * This function is called when the system is being rebooted.
> + */
> +static int
> +xxen_system_reboot(struct notifier_block *nb, unsigned long event, void *unused)
> +{
> +	switch (event) {
> +	case SYS_RESTART:
> +	case SYS_HALT:
> +	case SYS_POWER_OFF:
> +	default:
> +		mutex_lock(&xenwatch_mutex);
> +		shutting_down = SHUTDOWN_POWEROFF;
> +		mutex_unlock(&xenwatch_mutex);
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
>  
> +static struct notifier_block xen_shutdown_notifier = {
> +        .notifier_call = xen_system_reboot,
> +};
>  #ifdef CONFIG_MAGIC_SYSRQ
>  static void sysrq_handler(struct xenbus_watch *watch, const char **vec,
>  			  unsigned int len)
> @@ -308,7 +336,11 @@ static int setup_shutdown_watcher(void)
>  		return err;
>  	}
>  #endif
> -
> +	err = register_reboot_notifier(&xen_shutdown_notifier);
> +	if (err) {
> +		pr_warn("Failed to register shutdown notifier\n");
> +		return err;
> +	}
>  	return 0;
>  }
>  
> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
> index b6d5fff..ac25752 100644
> --- a/drivers/xen/xenbus/xenbus_xs.c
> +++ b/drivers/xen/xenbus/xenbus_xs.c
> @@ -122,7 +122,7 @@ static DEFINE_SPINLOCK(watch_events_lock);
>   * carrying out work.
>   */
>  static pid_t xenwatch_pid;
> -static DEFINE_MUTEX(xenwatch_mutex);
> +DEFINE_MUTEX(xenwatch_mutex);
>  static DECLARE_WAIT_QUEUE_HEAD(watch_events_waitq);
>  
>  static int get_error(const char *errorstring)
> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> index 40abaf6..57b3370 100644
> --- a/include/xen/xenbus.h
> +++ b/include/xen/xenbus.h
> @@ -125,7 +125,7 @@ struct xenbus_transaction
>  {
>  	u32 id;
>  };
> -
> +extern struct mutex xenwatch_mutex;
>  /* Nil transaction ID. */
>  #define XBT_NIL ((struct xenbus_transaction) { 0 })
>  
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org, Paul Durrant <paul.durrant@citrix.com>, Anthony Perard <anthony.perard@citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>, Alex Bligh <alex@alex.org.uk>, Diana Crisan <dcrisan@flexiant.com>, Dave Scott <Dave.Scott@eu.citrix.com>
Subject: Re: [Xen-devel] Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU
Date: Fri, 8 Nov 2013 09:27:51 -0500
Message-ID: <20131108142751.GD24060@phenom.dumpdata.com>

[ Reply to this message; Retrieve Raw Message; Archives: gmane, marc.info ]

On Thu, Nov 07, 2013 at 11:24:45AM +0000, Ian Campbell wrote:
> > > @@ -185,7 +185,7 @@ struct shutdown_handler {
> > > >  static void do_poweroff(void)
> > > >  {
> > > >  	shutting_down = SHUTDOWN_POWEROFF;
> > > > -	orderly_poweroff(false);
> > > > +	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
> > > 
> > > Does this DTRT for systems in SYSTEM_{HALTED,POWEROFF}? I suppose under
> > > those circumstances forcing is the desired action, insn't it
> > 
> > Yes. And there is also a guard (shutting_down) that gets set on
> > drivers/xen/manage.c so that the 'do_poweroff' will only get called
> > once. Which would guard against us powering off and then
> > receiving another 'xl shutdown' when the the system_state is in
> > HALTED or POWEROFF.
> > 
> > But I hadn't tested the case where the user does 'poweroff'
> > and at the same time the system admin does 'xl shutdown'.
> > 
> > Depending on the race, the state will be SYSTEM_RUNNING or
> > SYSTEM_POWER_OFF. If SYSTEM_RUNNING we just end up making
> > a duplicate call to 'poweroff' (while it is running).
> > 
> > That will fail or execute (And if executed then it will be
> > stuck in the reboot_mutex mutex). But nobody will care b/c the
> > machine is in poweroff sequence.
> 
> Right.
> 
> > If the state is SYSTEM_POWER_OFF then we end up making
> > a duplicate call to kernel_power_off. There is no locking
> > there so we walk in the same steps as what 'poweroff'
> > has been doing.
> > 
> > This code in kernel/reboot.c doesn't look that safe when it
> > comes to a user-invoked 'poweroff' operation and a kernel
> > 'orderly_poweroff' operation.
> 
> Yes.
> 
> > Perhaps what we should do is just:
> > 
> > 	if (system_state == SYSTEM_BOOTING)
> > 		orderly_poweroff(true);
> > 	else if (system_state == SYSTEM_RUNNING)
> > 		orderly_poweroff(false);
> > 	else
> > 		printk("Shutdown in progress. Ignoring xl shutdown");
> 
> (nb: switch() ;-)). I would also avoiding saying xl since it may not be
> true. "Ignoring Xen toolstack shutdown" or something
> 
> > But then 'system_state' is not guarded by a spinlock or such. Thought
> > it is guarded by the xenwatch mutex.
> 
> system_state is a core global though, so it must surely also be touched
> outside of xen code and therefore outside of xenwatch mutex.
> 
> maybe you meant s/system_state/shutting_down/?

I think I meant shutting_down. Which is a guard variable we use
to guard against calling the orderly_poweroff multiple times.

But then in my mind the system_state and shutting_down melted
in one.

I blame the sleep deprevation on that.

> 
> > Perhaps to be extra safe we should add ourselves to the
> > register_reboot_notifier like so (not compile tested)
> 
> I think this only makes sense if you did mean
> s/system_state/shutting_down/ above, so I'll assume that to be the case.
> 
> It's a shame this has to expose the watch mutex outside of the core xs
> code. Perhaps the core code could add the notifier itself and in turn
> call a manage.c notification function with the lock already held?

We could also make the 'shutting_down' be an atomic. That way
it will always have the correct value and we don't have to depend on
mutexes.

And then we won't go in the orderly_shutdown when a 'poweroff'
has been done from user-space. Problem solved :-)

We will still need the notifier naturally (in the manage.c code).
> 
> > 
> > diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> > index fe1c0a6..fb43db6 100644
> > --- a/drivers/xen/manage.c
> > +++ b/drivers/xen/manage.c
> > @@ -36,7 +36,8 @@ enum shutdown_state {
> >  	 SHUTDOWN_HALT = 4,
> >  };
> >  
> > -/* Ignore multiple shutdown requests. */
> > +/* Ignore multiple shutdown requests. Our mutex for this is that
> > + * shutdown handler is called with a mutex from xenwatch. */
> >  static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
> >  
> >  struct suspend_info {
> > @@ -185,7 +186,12 @@ struct shutdown_handler {
> >  static void do_poweroff(void)
> >  {
> >  	shutting_down = SHUTDOWN_POWEROFF;
> > -	orderly_poweroff(system_state != SYSTEM_RUNNING ? true : false);
> > +	if (system_state == SYSTEM_RUNNING)
> > +		orderly_poweroff(false);
> > +	else if (system_state == SYSTEM_BOOTING)
> > +		orderly_poweroff(true);
> > +	else
> > +		printk(KERN_WARNING "Ignorning shutdown request as poweroff in progress.\n");
> >  }
> >  
> >  static void do_reboot(void)
> > @@ -250,7 +256,29 @@ static void shutdown_handler(struct xenbus_watch *watch,
> >  
> >  	kfree(str);
> >  }
> > +/*
> > + * This function is called when the system is being rebooted.
> > + */
> > +static int
> > +xxen_system_reboot(struct notifier_block *nb, unsigned long event, void *unused)
> > +{
> > +	switch (event) {
> > +	case SYS_RESTART:
> > +	case SYS_HALT:
> > +	case SYS_POWER_OFF:
> > +	default:
> > +		mutex_lock(&xenwatch_mutex);
> > +		shutting_down = SHUTDOWN_POWEROFF;
> > +		mutex_unlock(&xenwatch_mutex);
> > +		break;
> > +	}
> > +
> > +	return NOTIFY_DONE;
> > +}
> >  
> > +static struct notifier_block xen_shutdown_notifier = {
> > +        .notifier_call = xen_system_reboot,
> > +};
> >  #ifdef CONFIG_MAGIC_SYSRQ
> >  static void sysrq_handler(struct xenbus_watch *watch, const char **vec,
> >  			  unsigned int len)
> > @@ -308,7 +336,11 @@ static int setup_shutdown_watcher(void)
> >  		return err;
> >  	}
> >  #endif
> > -
> > +	err = register_reboot_notifier(&xen_shutdown_notifier);
> > +	if (err) {
> > +		pr_warn("Failed to register shutdown notifier\n");
> > +		return err;
> > +	}
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
> > index b6d5fff..ac25752 100644
> > --- a/drivers/xen/xenbus/xenbus_xs.c
> > +++ b/drivers/xen/xenbus/xenbus_xs.c
> > @@ -122,7 +122,7 @@ static DEFINE_SPINLOCK(watch_events_lock);
> >   * carrying out work.
> >   */
> >  static pid_t xenwatch_pid;
> > -static DEFINE_MUTEX(xenwatch_mutex);
> > +DEFINE_MUTEX(xenwatch_mutex);
> >  static DECLARE_WAIT_QUEUE_HEAD(watch_events_waitq);
> >  
> >  static int get_error(const char *errorstring)
> > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> > index 40abaf6..57b3370 100644
> > --- a/include/xen/xenbus.h
> > +++ b/include/xen/xenbus.h
> > @@ -125,7 +125,7 @@ struct xenbus_transaction
> >  {
> >  	u32 id;
> >  };
> > -
> > +extern struct mutex xenwatch_mutex;
> >  /* Nil transaction ID. */
> >  #define XBT_NIL ((struct xenbus_transaction) { 0 })
> >  
> > 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel