Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Diana Crisan]

This is problem 3 of 3 problems we are having with live migration and/or ACPI on Xen-4.3 and Xen-4.2.

Any help would be appreciated.

Detailed description of problem:

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.

This is replicable every time.

How to replicate:
1. Take a machines running xen-4.3-rc1 version of Xen on Ubuntu Precise with 3.5.0-23-generic kernel.
2. Use the xl.conf below as a configuration file.
3. Create a VM using Ubuntu Precise and 3.5.0-23 generic.
4. Start the VM
5. Immediately (before boot is completed) run 'xl shutdown' or 'xl reboot'. As ACPI is not initialised, this will have no effect.
6. Wait until domU has completed booting.
7. Run 'xl shutdown' or 'xl reboot' again.

Expected results:

At step 7, domU will shutdown or reboot as appropriate

Actual results:

domU does not shutdown or reboot and it is necessary to 'xl des' the domU to stop it.

Notes:

On xen-4.2, a similar thing happens.

--xl.conf--

builder='hvm'
memory = 512
name = "416-vm"
vcpus=1
disk = [ 'tap:qcow2:/root/diana.qcow2,xvda,w' ]
vif = ['mac=00:16:3f:1d:6a:c0, bridge=defaultbr']
sdl=0
opengl=1
vnc=1
vnclisten="0.0.0.0"
vncdisplay=0
vncunused=0
vncpasswd='p'
stdvga=0
serial='pty'

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

CCing the guys who know about qemu.

On Tue, 2013-05-14 at 14:13 +0100, Diana Crisan wrote:
> This is problem 3 of 3 problems we are having with live migration
> and/or ACPI on Xen-4.3 and Xen-4.2.
> 
> Any help would be appreciated.
> 
> Detailed description of problem:
> 
> 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.

> 
> This is replicable every time.
> 
> How to replicate:
> 1. Take a machines running xen-4.3-rc1 version of Xen on Ubuntu Precise with 3.5.0-23-generic kernel.
> 2. Use the xl.conf below as a configuration file.
> 3. Create a VM using Ubuntu Precise and 3.5.0-23 generic.
> 4. Start the VM
> 5. Immediately (before boot is completed) run 'xl shutdown' or 'xl reboot'. As ACPI is not initialised, this will have no effect.
> 6. Wait until domU has completed booting.
> 7. Run 'xl shutdown' or 'xl reboot' again.
> 
> Expected results:
> 
> At step 7, domU will shutdown or reboot as appropriate
> 
> Actual results:
> 
> domU does not shutdown or reboot and it is necessary to 'xl des' the domU to stop it.
> 
> Notes:
> 
> On xen-4.2, a similar thing happens.
> 
> --xl.conf--
> 
> builder='hvm'
> memory = 512
> name = "416-vm"
> vcpus=1
> disk = [ 'tap:qcow2:/root/diana.qcow2,xvda,w' ]
> vif = ['mac=00:16:3f:1d:6a:c0, bridge=defaultbr']
> sdl=0
> opengl=1
> vnc=1
> vnclisten="0.0.0.0"
> vncdisplay=0
> vncunused=0
> vncpasswd='p'
> stdvga=0
> serial='pty'
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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.

-- 
Alex Bligh

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

 -George

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

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.

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

George,

--On 21 May 2013 14:39:55 +0100 George Dunlap <George.Dunlap@eu.citrix.com> 
wrote:

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

OK. But doesn't the power thing yank the power rather than send a
clean shutdown?

We are using libxl here (admittedly having looked carefully at
the xl code for guidance) and get the same problem.

-- 
Alex Bligh

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

On 05/21/2013 04:16 PM, Alex Bligh wrote:
> George,
>
> --On 21 May 2013 14:39:55 +0100 George Dunlap
> <George.Dunlap@eu.citrix.com> wrote:
>
>> 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.
>
> OK. But doesn't the power thing yank the power rather than send a
> clean shutdown?

No -- if you push the button just once on most modern hardware it will 
send an ACPI "poweroff" event that the OS handles gracefully.  That's 
what gets sent when you do "xl trigger [domain] power".  If the OS 
ignores it (either on real hardware or virtual hardware) nothing 
happens.  On real hardware you have to then hold down the button for 5 
seconds for a hard-shutdown, with xl you have to do "xl destroy".

> We are using libxl here (admittedly having looked carefully at
> the xl code for guidance) and get the same problem.

The relevant xl code is here:

     rc=libxl_domain_shutdown(ctx, domid);
     if (rc == ERROR_NOPARAVIRT) {
         if (fallback_trigger) {
             fprintf(stderr, "PV control interface not available:"
                     " sending ACPI power button event.\n");
             rc = libxl_send_trigger(ctx, domid, LIBXL_TRIGGER_POWER, 0);
         } else {
             fprintf(stderr, "PV control interface not available:"
                     " external graceful shutdown not possible.\n");
             fprintf(stderr, "Use \"-F\" to fallback to ACPI power 
event.\n");
         }
     }

It looks like libxl_domain_shutdown() will only use the PV shutdown 
method.  If that method is not available, then it will call 
libxl_send_trigger().

What does your code do?  Does it call libxl_domain_shutdown(), or 
libxl_send_trigger() (or one then the other, like xl)?

  -George

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

George,

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

> On 05/21/2013 04:16 PM, Alex Bligh wrote:
>> George,
>>
>> --On 21 May 2013 14:39:55 +0100 George Dunlap
>> <George.Dunlap@eu.citrix.com> wrote:
>>
>>> 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.
>>
>> OK. But doesn't the power thing yank the power rather than send a
>> clean shutdown?
>
> No -- if you push the button just once on most modern hardware it will
> send an ACPI "poweroff" event that the OS handles gracefully.  That's
> what gets sent when you do "xl trigger [domain] power".  If the OS
> ignores it (either on real hardware or virtual hardware) nothing happens.
> On real hardware you have to then hold down the button for 5 seconds for
> a hard-shutdown, with xl you have to do "xl destroy".

OK, great. I am guessing the reason why 'xl shutdown' doesn't do
that is to cope with (a) non-HVM domains, and (b) old fashioned
HVM domains with PV support but not ACPI support. Correct?

>> We are using libxl here (admittedly having looked carefully at
>> the xl code for guidance) and get the same problem.
>
> The relevant xl code is here:
>
>      rc=libxl_domain_shutdown(ctx, domid);
>      if (rc == ERROR_NOPARAVIRT) {
>          if (fallback_trigger) {
>              fprintf(stderr, "PV control interface not available:"
>                      " sending ACPI power button event.\n");
>              rc = libxl_send_trigger(ctx, domid, LIBXL_TRIGGER_POWER, 0);
>          } else {
>              fprintf(stderr, "PV control interface not available:"
>                      " external graceful shutdown not possible.\n");
>              fprintf(stderr, "Use \"-F\" to fallback to ACPI power
> event.\n");
>          }
>      }
>
> It looks like libxl_domain_shutdown() will only use the PV shutdown
> method.  If that method is not available, then it will call
> libxl_send_trigger().

OK. Well if that's the case, why is it not working? i.e. how
is using 'xl trigger power' a workaround? As our testing was
on xl (as well as libxl). Perhaps libxl_domain_shutdown
is returning an error other than ERROR_NOPARAVIRT or no error
at all?

The major problem for us isn't that the initial domain
shutdown doesn't work (that's just a bit sad), it's that it blocks
subsequent shutdowns from working.

> What does your code do?  Does it call libxl_domain_shutdown(), or
> libxl_send_trigger() (or one then the other, like xl)?

When Diana reported the bug, she was using xl (and thus the
code you are using above).

Our libxl code currently just calls libxl_domain_shutdown.
Apparently we didn't look too closely at xl :-)

Does xl trigger reset do the same thing (i.e. a graceful shutdown)?
If not we'd also (I think) have to recode reboots to do a power trigger
then a restart.

-- 
Alex Bligh

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

On 05/21/2013 04:59 PM, Alex Bligh wrote:
> George,
>
> --On 21 May 2013 16:23:00 +0100 George Dunlap
> <george.dunlap@eu.citrix.com> wrote:
>
>> On 05/21/2013 04:16 PM, Alex Bligh wrote:
>>> George,
>>>
>>> --On 21 May 2013 14:39:55 +0100 George Dunlap
>>> <George.Dunlap@eu.citrix.com> wrote:
>>>
>>>> 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.
>>>
>>> OK. But doesn't the power thing yank the power rather than send a
>>> clean shutdown?
>>
>> No -- if you push the button just once on most modern hardware it will
>> send an ACPI "poweroff" event that the OS handles gracefully.  That's
>> what gets sent when you do "xl trigger [domain] power".  If the OS
>> ignores it (either on real hardware or virtual hardware) nothing happens.
>> On real hardware you have to then hold down the button for 5 seconds for
>> a hard-shutdown, with xl you have to do "xl destroy".
>
> OK, great. I am guessing the reason why 'xl shutdown' doesn't do
> that is to cope with (a) non-HVM domains, and (b) old fashioned
> HVM domains with PV support but not ACPI support. Correct?
>
>>> We are using libxl here (admittedly having looked carefully at
>>> the xl code for guidance) and get the same problem.
>>
>> The relevant xl code is here:
>>
>>      rc=libxl_domain_shutdown(ctx, domid);
>>      if (rc == ERROR_NOPARAVIRT) {
>>          if (fallback_trigger) {
>>              fprintf(stderr, "PV control interface not available:"
>>                      " sending ACPI power button event.\n");
>>              rc = libxl_send_trigger(ctx, domid, LIBXL_TRIGGER_POWER, 0);
>>          } else {
>>              fprintf(stderr, "PV control interface not available:"
>>                      " external graceful shutdown not possible.\n");
>>              fprintf(stderr, "Use \"-F\" to fallback to ACPI power
>> event.\n");
>>          }
>>      }
>>
>> It looks like libxl_domain_shutdown() will only use the PV shutdown
>> method.  If that method is not available, then it will call
>> libxl_send_trigger().
>
> OK. Well if that's the case, why is it not working? i.e. how
> is using 'xl trigger power' a workaround? As our testing was
> on xl (as well as libxl). Perhaps libxl_domain_shutdown
> is returning an error other than ERROR_NOPARAVIRT or no error
> at all?

xl/libxl is working as designed. xl tried libxl_domain_shutdown() first. 
  libxl saw that there were PV drivers available, so it sent the PV 
shutdown signal over xenstore and returned success.  libxl and xl have 
no way of knowing that the signal was never received, so it never falls 
back to ACPI.

"xl trigger power" is a work-around because when you detect that a guest 
is stuck, you should be able to issue an ACPI shutdown and get a clean 
shutdown, even when the PV shutdown path is stuck.

(In fact, as a short-term solution, you might consider just replacing 
"xl shutdown $d" with "xl trigger $d power" in your control framework.)

> Does xl trigger reset do the same thing (i.e. a graceful shutdown)?
> If not we'd also (I think) have to recode reboots to do a power trigger
> then a restart.

Hmm, this is what I get:

# xl trigger h0 reset
libxl: error: libxl.c:4639:libxl_send_trigger: Send trigger 'reset' 
failed: Function not implemented

YMMV...

  -George

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

George,

> xl/libxl is working as designed. xl tried libxl_domain_shutdown() first.
> libxl saw that there were PV drivers available, so it sent the PV
> shutdown signal over xenstore and returned success.  libxl and xl have no
> way of knowing that the signal was never received, so it never falls back
> to ACPI.
>
> "xl trigger power" is a work-around because when you detect that a guest
> is stuck, you should be able to issue an ACPI shutdown and get a clean
> shutdown, even when the PV shutdown path is stuck.
>
> (In fact, as a short-term solution, you might consider just replacing "xl
> shutdown $d" with "xl trigger $d power" in your control framework.)

Yup. I think this is the best option, save for the fact that 'xl trigger
power' itself appears unreliable. We apparently need to send it multiple
times. Diana will send more details.

>> Does xl trigger reset do the same thing (i.e. a graceful shutdown)?
>> If not we'd also (I think) have to recode reboots to do a power trigger
>> then a restart.
>
> Hmm, this is what I get:
>
># xl trigger h0 reset
> libxl: error: libxl.c:4639:libxl_send_trigger: Send trigger 'reset'
> failed: Function not implemented
>
> YMMV...

Indeed. So presumably 'xl trigger power' (+/- repeats to get it
to work), wait until the domain is dead, then power it back up
again.

-- 
Alex Bligh

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Konrad Rzeszutek Wilk]

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.

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Diana Crisan]

George,

On 21/05/13 17:09, George Dunlap wrote:
> On 05/21/2013 04:59 PM, Alex Bligh wrote:
>> George,
>>
>> --On 21 May 2013 16:23:00 +0100 George Dunlap
>> <george.dunlap@eu.citrix.com> wrote:
>>
>>> On 05/21/2013 04:16 PM, Alex Bligh wrote:
>>>> George,
>>>>
>>>> --On 21 May 2013 14:39:55 +0100 George Dunlap
>>>> <George.Dunlap@eu.citrix.com> wrote:
>>>>
>>>>> 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.
>>>>
>>>> OK. But doesn't the power thing yank the power rather than send a
>>>> clean shutdown?
>>>
>>> No -- if you push the button just once on most modern hardware it will
>>> send an ACPI "poweroff" event that the OS handles gracefully. That's
>>> what gets sent when you do "xl trigger [domain] power".  If the OS
>>> ignores it (either on real hardware or virtual hardware) nothing 
>>> happens.
>>> On real hardware you have to then hold down the button for 5 seconds 
>>> for
>>> a hard-shutdown, with xl you have to do "xl destroy".
>>
>> OK, great. I am guessing the reason why 'xl shutdown' doesn't do
>> that is to cope with (a) non-HVM domains, and (b) old fashioned
>> HVM domains with PV support but not ACPI support. Correct?
>>
>>>> We are using libxl here (admittedly having looked carefully at
>>>> the xl code for guidance) and get the same problem.
>>>
>>> The relevant xl code is here:
>>>
>>>      rc=libxl_domain_shutdown(ctx, domid);
>>>      if (rc == ERROR_NOPARAVIRT) {
>>>          if (fallback_trigger) {
>>>              fprintf(stderr, "PV control interface not available:"
>>>                      " sending ACPI power button event.\n");
>>>              rc = libxl_send_trigger(ctx, domid, 
>>> LIBXL_TRIGGER_POWER, 0);
>>>          } else {
>>>              fprintf(stderr, "PV control interface not available:"
>>>                      " external graceful shutdown not possible.\n");
>>>              fprintf(stderr, "Use \"-F\" to fallback to ACPI power
>>> event.\n");
>>>          }
>>>      }
>>>
>>> It looks like libxl_domain_shutdown() will only use the PV shutdown
>>> method.  If that method is not available, then it will call
>>> libxl_send_trigger().
>>
>> OK. Well if that's the case, why is it not working? i.e. how
>> is using 'xl trigger power' a workaround? As our testing was
>> on xl (as well as libxl). Perhaps libxl_domain_shutdown
>> is returning an error other than ERROR_NOPARAVIRT or no error
>> at all?
>
> xl/libxl is working as designed. xl tried libxl_domain_shutdown() 
> first.  libxl saw that there were PV drivers available, so it sent the 
> PV shutdown signal over xenstore and returned success.  libxl and xl 
> have no way of knowing that the signal was never received, so it never 
> falls back to ACPI.
>
> "xl trigger power" is a work-around because when you detect that a 
> guest is stuck, you should be able to issue an ACPI shutdown and get a 
> clean shutdown, even when the PV shutdown path is stuck.
>
> (In fact, as a short-term solution, you might consider just replacing 
> "xl shutdown $d" with "xl trigger $d power" in your control framework.)
>

I am having problems replicating the short-term solution reliably on our 
end, even with xl/libxl. It seems that issuing an "xl trigger dom power" 
does not make the domain shutdown, even after a 10 minute wait. 
Subsequent ones tend to do the same and after issuing quite a few I 
managed to get the domU to power off.

This happens even if I do not issue the shutdown early in the boot 
process, but rather issue directly an xl trigger dom power.

>> Does xl trigger reset do the same thing (i.e. a graceful shutdown)?
>> If not we'd also (I think) have to recode reboots to do a power trigger
>> then a restart.
>
> Hmm, this is what I get:
>
> # xl trigger h0 reset
> libxl: error: libxl.c:4639:libxl_send_trigger: Send trigger 'reset' 
> failed: Function not implemented
>
> YMMV...
>
>  -George

--
Diana Crisan

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Dave Scott]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Sander Eikelenboom]

Tuesday, May 21, 2013, 6:09:33 PM, you wrote:

> On 05/21/2013 04:59 PM, Alex Bligh wrote:
>> George,
>>
>> --On 21 May 2013 16:23:00 +0100 George Dunlap
>> <george.dunlap@eu.citrix.com> wrote:
>>
>>> On 05/21/2013 04:16 PM, Alex Bligh wrote:
>>>> George,
>>>>
>>>> --On 21 May 2013 14:39:55 +0100 George Dunlap
>>>> <George.Dunlap@eu.citrix.com> wrote:
>>>>
>>>>> 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.
>>>>
>>>> OK. But doesn't the power thing yank the power rather than send a
>>>> clean shutdown?
>>>
>>> No -- if you push the button just once on most modern hardware it will
>>> send an ACPI "poweroff" event that the OS handles gracefully.  That's
>>> what gets sent when you do "xl trigger [domain] power".  If the OS
>>> ignores it (either on real hardware or virtual hardware) nothing happens.
>>> On real hardware you have to then hold down the button for 5 seconds for
>>> a hard-shutdown, with xl you have to do "xl destroy".
>>
>> OK, great. I am guessing the reason why 'xl shutdown' doesn't do
>> that is to cope with (a) non-HVM domains, and (b) old fashioned
>> HVM domains with PV support but not ACPI support. Correct?
>>
>>>> We are using libxl here (admittedly having looked carefully at
>>>> the xl code for guidance) and get the same problem.
>>>
>>> The relevant xl code is here:
>>>
>>>      rc=libxl_domain_shutdown(ctx, domid);
>>>      if (rc == ERROR_NOPARAVIRT) {
>>>          if (fallback_trigger) {
>>>              fprintf(stderr, "PV control interface not available:"
>>>                      " sending ACPI power button event.\n");
>>>              rc = libxl_send_trigger(ctx, domid, LIBXL_TRIGGER_POWER, 0);
>>>          } else {
>>>              fprintf(stderr, "PV control interface not available:"
>>>                      " external graceful shutdown not possible.\n");
>>>              fprintf(stderr, "Use \"-F\" to fallback to ACPI power
>>> event.\n");
>>>          }
>>>      }
>>>
>>> It looks like libxl_domain_shutdown() will only use the PV shutdown
>>> method.  If that method is not available, then it will call
>>> libxl_send_trigger().
>>
>> OK. Well if that's the case, why is it not working? i.e. how
>> is using 'xl trigger power' a workaround? As our testing was
>> on xl (as well as libxl). Perhaps libxl_domain_shutdown
>> is returning an error other than ERROR_NOPARAVIRT or no error
>> at all?

> xl/libxl is working as designed. xl tried libxl_domain_shutdown() first. 
>   libxl saw that there were PV drivers available, so it sent the PV 
> shutdown signal over xenstore and returned success.  libxl and xl have 
> no way of knowing that the signal was never received, so it never falls 
> back to ACPI.

At a sidenote ...

This reminds me that there still is the issue that a HVM domain without PV drivers doesn't shutdown properly with the standard init.d scripts.
It was discussed last year with IanC and IanJ, to make the fallback to acpi automatical (at present it isn't it just prints a warning)
IanJ was in favor, IanC wasn't convinced that it wouldn't hurt in some cases. At some point the discussion resulted in a stalemate.
(thread is here: http://lists.xen.org/archives/html/xen-devel/2012-10/msg01973.html )

There are a few options:
- always invoking acpi method if pv method fails in libxl
- add -F to the shutdown line in /etc/(sysconfig|default)/xendomains, so automatically invoking the shutdown is only done from the shutdown script.
- adding a option to the domains cfg (f.e. shutdown="shutdown_acpi" instead of shutdown="shutdown")

This issue also causes the "guest-stop fail   never pass" results in the auto test system.

--
Sander


> "xl trigger power" is a work-around because when you detect that a guest 
> is stuck, you should be able to issue an ACPI shutdown and get a clean 
> shutdown, even when the PV shutdown path is stuck.

> (In fact, as a short-term solution, you might consider just replacing 
> "xl shutdown $d" with "xl trigger $d power" in your control framework.)

>> Does xl trigger reset do the same thing (i.e. a graceful shutdown)?
>> If not we'd also (I think) have to recode reboots to do a power trigger
>> then a restart.

> Hmm, this is what I get:

> # xl trigger h0 reset
> libxl: error: libxl.c:4639:libxl_send_trigger: Send trigger 'reset' 
> failed: Function not implemented

> YMMV...

>   -George

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Konrad Rzeszutek Wilk]

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.

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

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.

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Diana Crisan]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Alex Bligh]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Konrad Rzeszutek Wilk]

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.

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[George Dunlap]

On 21/05/13 18:31, Sander Eikelenboom wrote:
> Tuesday, May 21, 2013, 6:09:33 PM, you wrote:
>
>> On 05/21/2013 04:59 PM, Alex Bligh wrote:
>>> George,
>>>
>>> --On 21 May 2013 16:23:00 +0100 George Dunlap
>>> <george.dunlap@eu.citrix.com> wrote:
>>>
>>>> On 05/21/2013 04:16 PM, Alex Bligh wrote:
>>>>> George,
>>>>>
>>>>> --On 21 May 2013 14:39:55 +0100 George Dunlap
>>>>> <George.Dunlap@eu.citrix.com> wrote:
>>>>>
>>>>>> 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.
>>>>> OK. But doesn't the power thing yank the power rather than send a
>>>>> clean shutdown?
>>>> No -- if you push the button just once on most modern hardware it will
>>>> send an ACPI "poweroff" event that the OS handles gracefully.  That's
>>>> what gets sent when you do "xl trigger [domain] power".  If the OS
>>>> ignores it (either on real hardware or virtual hardware) nothing happens.
>>>> On real hardware you have to then hold down the button for 5 seconds for
>>>> a hard-shutdown, with xl you have to do "xl destroy".
>>> OK, great. I am guessing the reason why 'xl shutdown' doesn't do
>>> that is to cope with (a) non-HVM domains, and (b) old fashioned
>>> HVM domains with PV support but not ACPI support. Correct?
>>>
>>>>> We are using libxl here (admittedly having looked carefully at
>>>>> the xl code for guidance) and get the same problem.
>>>> The relevant xl code is here:
>>>>
>>>>       rc=libxl_domain_shutdown(ctx, domid);
>>>>       if (rc == ERROR_NOPARAVIRT) {
>>>>           if (fallback_trigger) {
>>>>               fprintf(stderr, "PV control interface not available:"
>>>>                       " sending ACPI power button event.\n");
>>>>               rc = libxl_send_trigger(ctx, domid, LIBXL_TRIGGER_POWER, 0);
>>>>           } else {
>>>>               fprintf(stderr, "PV control interface not available:"
>>>>                       " external graceful shutdown not possible.\n");
>>>>               fprintf(stderr, "Use \"-F\" to fallback to ACPI power
>>>> event.\n");
>>>>           }
>>>>       }
>>>>
>>>> It looks like libxl_domain_shutdown() will only use the PV shutdown
>>>> method.  If that method is not available, then it will call
>>>> libxl_send_trigger().
>>> OK. Well if that's the case, why is it not working? i.e. how
>>> is using 'xl trigger power' a workaround? As our testing was
>>> on xl (as well as libxl). Perhaps libxl_domain_shutdown
>>> is returning an error other than ERROR_NOPARAVIRT or no error
>>> at all?
>> xl/libxl is working as designed. xl tried libxl_domain_shutdown() first.
>>    libxl saw that there were PV drivers available, so it sent the PV
>> shutdown signal over xenstore and returned success.  libxl and xl have
>> no way of knowing that the signal was never received, so it never falls
>> back to ACPI.
> At a sidenote ...
>
> This reminds me that there still is the issue that a HVM domain without PV drivers doesn't shutdown properly with the standard init.d scripts.

BTW, if you want this to get attention, you should probably either reply 
to that thread, or start a new thread.  Starting a new topic on an 
unrelated thread is generally frowned upon. :-)

  -George

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Konrad Rzeszutek Wilk]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

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.

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Jan Beulich]

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

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Konrad Rzeszutek Wilk]

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 })

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Ian Campbell]

> > @@ -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 })
>  
> 

Re: Early ACPI events prevent subsequent ACPI functionality on xen 4.3 + HVM domU

[Konrad Rzeszutek Wilk]

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 })
> >  
> > 
> 
>