All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
@ 2011-07-27 23:13 Jan Beulich
  2011-07-28  5:25 ` Olaf Hering
  2011-07-28 11:37 ` Stefano Stabellini
  0 siblings, 2 replies; 18+ messages in thread
From: Jan Beulich @ 2011-07-27 23:13 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1965 bytes --]

>>> On 26.07.11 at 13:52, Olaf Hering <olaf@aepfle.de> wrote:
> Unregister the shutdown and sysrq watch during kexec.  The watches can
> not be re-registered in the kexec kernel because they are still seen as
> busy by xenstore.

This and subsequent patches don't look right to me from a conceptual
pov: If the kexec attempt is due to a crash, the dying kernel should be
doing as little as possible, and the new kernel should really do the
cleanup. The more logic gets added to the shutdown path of the old
kernel, the more likely it'll become that the kexec attempt will fail.

If this requires changes outside the kernel (e.g. state reset helpers
in hypervisor or tools) - so be it.

Jan

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  drivers/xen/manage.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> Index: linux-3.0/drivers/xen/manage.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/manage.c
> +++ linux-3.0/drivers/xen/manage.c
> @@ -320,6 +320,18 @@ static int shutdown_event(struct notifie
>  return NOTIFY_DONE;
>  }
>  
> +static void xenbus_disable_shutdown_watcher(void)
> +{
> +unregister_xenbus_watch(&shutdown_watch);
> +#ifdef CONFIG_MAGIC_SYSRQ
> +unregister_xenbus_watch(&sysrq_watch);
> +#endif
> +}
> +
> +static struct syscore_ops xenbus_watcher_syscore_ops = {
> +.shutdown = xenbus_disable_shutdown_watcher,
> +};
> +
>  int xen_setup_shutdown_event(void)
>  {
>  static struct notifier_block xenstore_notifier = {
> @@ -329,6 +341,7 @@ int xen_setup_shutdown_event(void)
>  if (!xen_domain())
>  return -ENODEV;
>  register_xenstore_notifier(&xenstore_notifier);
> +register_syscore_ops(&xenbus_watcher_syscore_ops);
>  
>  return 0;
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 



[-- Attachment #1.2: HTML --]
[-- Type: text/html, Size: 3447 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-27 23:13 [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot Jan Beulich
@ 2011-07-28  5:25 ` Olaf Hering
  2011-07-28 10:52   ` Ian Campbell
  2011-07-28 16:09   ` Jan Beulich
  2011-07-28 11:37 ` Stefano Stabellini
  1 sibling, 2 replies; 18+ messages in thread
From: Olaf Hering @ 2011-07-28  5:25 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

On Thu, Jul 28, Jan Beulich wrote:

> >>> On 26.07.11 at 13:52, Olaf Hering <olaf@aepfle.de> wrote:
> > Unregister the shutdown and sysrq watch during kexec.  The watches can
> > not be re-registered in the kexec kernel because they are still seen as
> > busy by xenstore.
> 
> This and subsequent patches don't look right to me from a conceptual
> pov: If the kexec attempt is due to a crash, the dying kernel should be
> doing as little as possible, and the new kernel should really do the
> cleanup. The more logic gets added to the shutdown path of the old
> kernel, the more likely it'll become that the kexec attempt will fail.

kexec is about reboot, kdump is about crash handling. Both use different
code paths.
The kexec code path is like a reboot without going through the firmware.
The kdump kernel runs in its own memory range, so memory corruption does
not appear to happen (with the sles11sp1 kernel + my kdump patch).

> 
> If this requires changes outside the kernel (e.g. state reset helpers
> in hypervisor or tools) - so be it.

Are you suggesting that there have to be ways for a domU to query the
state of its registered watches and shut them all down during very early
boot? And what about the event/irq handling? There is currently no way
to check what virq is bound to what port, other than looping through all
possible ports and see if one matches the requested virq.

Olaf

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28  5:25 ` Olaf Hering
@ 2011-07-28 10:52   ` Ian Campbell
  2011-07-28 11:00     ` Keir Fraser
                       ` (2 more replies)
  2011-07-28 16:09   ` Jan Beulich
  1 sibling, 3 replies; 18+ messages in thread
From: Ian Campbell @ 2011-07-28 10:52 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel, Jan Beulich

On Thu, 2011-07-28 at 01:25 -0400, Olaf Hering wrote:
> On Thu, Jul 28, Jan Beulich wrote:
> 
> > >>> On 26.07.11 at 13:52, Olaf Hering <olaf@aepfle.de> wrote:
> > > Unregister the shutdown and sysrq watch during kexec.  The watches can
> > > not be re-registered in the kexec kernel because they are still seen as
> > > busy by xenstore.
> > 
> > This and subsequent patches don't look right to me from a conceptual
> > pov: If the kexec attempt is due to a crash, the dying kernel should be
> > doing as little as possible, and the new kernel should really do the
> > cleanup. The more logic gets added to the shutdown path of the old
> > kernel, the more likely it'll become that the kexec attempt will fail.
> 
> kexec is about reboot, kdump is about crash handling. Both use different
> code paths.
> The kexec code path is like a reboot without going through the firmware.
> The kdump kernel runs in its own memory range, so memory corruption does
> not appear to happen (with the sles11sp1 kernel + my kdump patch).

Getting into the kdump kernel is a kexec like operation though and
shares many of the code paths, doesn't it?

> > If this requires changes outside the kernel (e.g. state reset helpers
> > in hypervisor or tools) - so be it.
> 
> Are you suggesting that there have to be ways for a domU to query the
> state of its registered watches and shut them all down during very early
> boot?

Perhaps the xenstore protocol could be enhanced with a mechanism to
clear all existing watches? A kernel could call that at start of day.

>  And what about the event/irq handling? There is currently no way
> to check what virq is bound to what port, other than looping through all
> possible ports and see if one matches the requested virq.

There is EVTCHNOP_reset but I'm not sure if it does too much. For
example I'm not sure how the guest could recover event channels which
are setup by the domain builder -- such as the xenstore event channel.

IIRC EVTCHNOP_reset was added to aid with kexec though -- so I must be
missing something.

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 10:52   ` Ian Campbell
@ 2011-07-28 11:00     ` Keir Fraser
  2011-07-28 12:52       ` Ian Campbell
  2011-07-28 11:02     ` Olaf Hering
  2011-07-28 14:07     ` Olaf Hering
  2 siblings, 1 reply; 18+ messages in thread
From: Keir Fraser @ 2011-07-28 11:00 UTC (permalink / raw)
  To: Ian Campbell, Olaf Hering; +Cc: xen-devel, Jan Beulich

On 28/07/2011 11:52, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

>>  And what about the event/irq handling? There is currently no way
>> to check what virq is bound to what port, other than looping through all
>> possible ports and see if one matches the requested virq.
> 
> There is EVTCHNOP_reset but I'm not sure if it does too much. For
> example I'm not sure how the guest could recover event channels which
> are setup by the domain builder -- such as the xenstore event channel.

EVTCHNOP_reset is currently only called from the toolstack, which is then
able to re-setup things like the xenstore event channel for the guest.

 -- Keir

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 10:52   ` Ian Campbell
  2011-07-28 11:00     ` Keir Fraser
@ 2011-07-28 11:02     ` Olaf Hering
  2011-07-28 12:56       ` Ian Campbell
  2011-07-28 14:07     ` Olaf Hering
  2 siblings, 1 reply; 18+ messages in thread
From: Olaf Hering @ 2011-07-28 11:02 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Jan Beulich

On Thu, Jul 28, Ian Campbell wrote:

> Getting into the kdump kernel is a kexec like operation though and
> shares many of the code paths, doesn't it?

The big difference is that kdump is entered in an unreliable state,
while kexec is a controlled reboot.

> > > If this requires changes outside the kernel (e.g. state reset helpers
> > > in hypervisor or tools) - so be it.
> > 
> > Are you suggesting that there have to be ways for a domU to query the
> > state of its registered watches and shut them all down during very early
> > boot?
> 
> Perhaps the xenstore protocol could be enhanced with a mechanism to
> clear all existing watches? A kernel could call that at start of day.

I wonder why xenstore knows that sysrq and shutdown nodes are busy,
while the device, backend and state files can be watched twice.

Olaf

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-27 23:13 [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot Jan Beulich
  2011-07-28  5:25 ` Olaf Hering
@ 2011-07-28 11:37 ` Stefano Stabellini
  1 sibling, 0 replies; 18+ messages in thread
From: Stefano Stabellini @ 2011-07-28 11:37 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Olaf Hering, xen-devel

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

On Thu, 28 Jul 2011, Jan Beulich wrote:
> >>> On 26.07.11 at 13:52, Olaf Hering <olaf@aepfle.de> wrote:
> > Unregister the shutdown and sysrq watch during kexec.  The watches can
> > not be re-registered in the kexec kernel because they are still seen as
> > busy by xenstore.
> 
> This and subsequent patches don't look right to me from a conceptual
> pov: If the kexec attempt is due to a crash, the dying kernel should be
> doing as little as possible, and the new kernel should really do the
> cleanup. The more logic gets added to the shutdown path of the old
> kernel, the more likely it'll become that the kexec attempt will fail.
> 
> If this requires changes outside the kernel (e.g. state reset helpers
> in hypervisor or tools) - so be it.

I agree.

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 11:00     ` Keir Fraser
@ 2011-07-28 12:52       ` Ian Campbell
  0 siblings, 0 replies; 18+ messages in thread
From: Ian Campbell @ 2011-07-28 12:52 UTC (permalink / raw)
  To: Keir Fraser; +Cc: Olaf Hering, xen-devel, Jan Beulich

On Thu, 2011-07-28 at 07:00 -0400, Keir Fraser wrote:
> On 28/07/2011 11:52, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
> 
> >>  And what about the event/irq handling? There is currently no way
> >> to check what virq is bound to what port, other than looping through all
> >> possible ports and see if one matches the requested virq.
> > 
> > There is EVTCHNOP_reset but I'm not sure if it does too much. For
> > example I'm not sure how the guest could recover event channels which
> > are setup by the domain builder -- such as the xenstore event channel.
> 
> EVTCHNOP_reset is currently only called from the toolstack, which is then
> able to re-setup things like the xenstore event channel for the guest.

Oh, right. And toolstack isn't involved in kexec so that's not terribly
helpful.

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 11:02     ` Olaf Hering
@ 2011-07-28 12:56       ` Ian Campbell
  0 siblings, 0 replies; 18+ messages in thread
From: Ian Campbell @ 2011-07-28 12:56 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel, Jan Beulich

On Thu, 2011-07-28 at 07:02 -0400, Olaf Hering wrote:
> On Thu, Jul 28, Ian Campbell wrote:
> 
> > Getting into the kdump kernel is a kexec like operation though and
> > shares many of the code paths, doesn't it?
> 
> The big difference is that kdump is entered in an unreliable state,
> while kexec is a controlled reboot.

Sure, but by handling the former you also solve the later, while the
opposite is not true.

> > > > If this requires changes outside the kernel (e.g. state reset helpers
> > > > in hypervisor or tools) - so be it.
> > > 
> > > Are you suggesting that there have to be ways for a domU to query the
> > > state of its registered watches and shut them all down during very early
> > > boot?
> > 
> > Perhaps the xenstore protocol could be enhanced with a mechanism to
> > clear all existing watches? A kernel could call that at start of day.
> 
> I wonder why xenstore knows that sysrq and shutdown nodes are busy,
> while the device, backend and state files can be watched twice.

Do the precise path's watched differ subtly?

Oh, I see, xenstored only calls a watch a duplicate if both the path
_and_ the token match. In the case of backend paths the token is a
reference to the dynamically allocated per-device data structure. In the
sysrq and shutdown case the token is a static global variable -- I
suppose you are kexec'ing an identical kernel? I expect that if you
kexec'd a subtly different kernel where these datastructures ended up at
a different address you wouldn't get the EEXIST.

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 10:52   ` Ian Campbell
  2011-07-28 11:00     ` Keir Fraser
  2011-07-28 11:02     ` Olaf Hering
@ 2011-07-28 14:07     ` Olaf Hering
  2011-07-28 14:13       ` Keir Fraser
  2 siblings, 1 reply; 18+ messages in thread
From: Olaf Hering @ 2011-07-28 14:07 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Jan Beulich

On Thu, Jul 28, Ian Campbell wrote:

> > Are you suggesting that there have to be ways for a domU to query the
> > state of its registered watches and shut them all down during very early
> > boot?
> 
> Perhaps the xenstore protocol could be enhanced with a mechanism to
> clear all existing watches? A kernel could call that at start of day.

I poked around in the xenstore sources, there is appearently nothing
that can be used to shutdown all. Or would calling XS_RELEASE do the trick?

Olaf

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 14:07     ` Olaf Hering
@ 2011-07-28 14:13       ` Keir Fraser
  2011-07-28 19:50         ` Olaf Hering
  0 siblings, 1 reply; 18+ messages in thread
From: Keir Fraser @ 2011-07-28 14:13 UTC (permalink / raw)
  To: Olaf Hering, Ian Campbell; +Cc: xen-devel, Jan Beulich

On 28/07/2011 15:07, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Thu, Jul 28, Ian Campbell wrote:
> 
>>> Are you suggesting that there have to be ways for a domU to query the
>>> state of its registered watches and shut them all down during very early
>>> boot?
>> 
>> Perhaps the xenstore protocol could be enhanced with a mechanism to
>> clear all existing watches? A kernel could call that at start of day.
> 
> I poked around in the xenstore sources, there is appearently nothing
> that can be used to shutdown all. Or would calling XS_RELEASE do the trick?

XS_INTRODUCE

 K.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28  5:25 ` Olaf Hering
  2011-07-28 10:52   ` Ian Campbell
@ 2011-07-28 16:09   ` Jan Beulich
  1 sibling, 0 replies; 18+ messages in thread
From: Jan Beulich @ 2011-07-28 16:09 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel

>>> On 28.07.11 at 07:25, Olaf Hering <olaf@aepfle.de> wrote:
> On Thu, Jul 28, Jan Beulich wrote:
> 
>> >>> On 26.07.11 at 13:52, Olaf Hering <olaf@aepfle.de> wrote:
>> > Unregister the shutdown and sysrq watch during kexec.  The watches can
>> > not be re-registered in the kexec kernel because they are still seen as
>> > busy by xenstore.
>> 
>> This and subsequent patches don't look right to me from a conceptual
>> pov: If the kexec attempt is due to a crash, the dying kernel should be
>> doing as little as possible, and the new kernel should really do the
>> cleanup. The more logic gets added to the shutdown path of the old
>> kernel, the more likely it'll become that the kexec attempt will fail.
> 
> kexec is about reboot, kdump is about crash handling. Both use different
> code paths.
> The kexec code path is like a reboot without going through the firmware.

Oh, I implied it would be kdump.

But then again xenstore state shouldn't matter wrt the purpose of
the secondary kernel.

> The kdump kernel runs in its own memory range, so memory corruption does
> not appear to happen (with the sles11sp1 kernel + my kdump patch).

It's also not clear to me what corruption there is - this would seem to
imply that there should be information on certain addresses that were
used in the old kernel to get passed to the new kernel, or get used to
access guest memory from outside the guest. All of which sounds
wrong.

Jan

>> If this requires changes outside the kernel (e.g. state reset helpers
>> in hypervisor or tools) - so be it.
> 
> Are you suggesting that there have to be ways for a domU to query the
> state of its registered watches and shut them all down during very early
> boot? And what about the event/irq handling? There is currently no way
> to check what virq is bound to what port, other than looping through all
> possible ports and see if one matches the requested virq.
> 
> Olaf

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 14:13       ` Keir Fraser
@ 2011-07-28 19:50         ` Olaf Hering
  2011-07-28 20:30           ` Keir Fraser
  0 siblings, 1 reply; 18+ messages in thread
From: Olaf Hering @ 2011-07-28 19:50 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Ian Campbell, Jan Beulich

On Thu, Jul 28, Keir Fraser wrote:

> On 28/07/2011 15:07, "Olaf Hering" <olaf@aepfle.de> wrote:
> 
> > On Thu, Jul 28, Ian Campbell wrote:
> > 
> >>> Are you suggesting that there have to be ways for a domU to query the
> >>> state of its registered watches and shut them all down during very early
> >>> boot?
> >> 
> >> Perhaps the xenstore protocol could be enhanced with a mechanism to
> >> clear all existing watches? A kernel could call that at start of day.
> > 
> > I poked around in the xenstore sources, there is appearently nothing
> > that can be used to shutdown all. Or would calling XS_RELEASE do the trick?
> 
> XS_INTRODUCE

Unfortunately do_introduce() is not preprared for that.

On enter, conn->id contains the domain_id, so it errors out early.
Later it compares domain->conn != conn and does not enter the correct path.

If I remove both checks the kexec appears to work ok.
Is it save to remove both checks?

The only issue I havent figured out:
How to get the domid from within the kernel?

Olaf

diff -r 42edf1481c57 tools/xenstore/xenstored_domain.c
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -326,7 +326,7 @@ void do_introduce(struct connection *con
 		return;
 	}
 
-	if (conn->id != 0 || !conn->can_write) {
+	if (!conn->can_write) {
 		send_error(conn, EACCES);
 		return;
 	}
@@ -365,7 +365,7 @@ void do_introduce(struct connection *con
 		talloc_steal(domain->conn, domain);
 
 		fire_watches(NULL, "@introduceDomain", false);
-	} else if ((domain->mfn == mfn) && (domain->conn != conn)) {
+	} else if (domain->mfn == mfn) {
 		/* Use XS_INTRODUCE for recreating the xenbus event-channel. */
 		if (domain->port)
 			xc_evtchn_unbind(xce_handle, domain->port);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 19:50         ` Olaf Hering
@ 2011-07-28 20:30           ` Keir Fraser
  2011-08-01 13:01             ` Olaf Hering
  0 siblings, 1 reply; 18+ messages in thread
From: Keir Fraser @ 2011-07-28 20:30 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel, Ian Campbell, Jan Beulich

[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]

On 28/07/2011 20:50, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Thu, Jul 28, Keir Fraser wrote:
> 
>> On 28/07/2011 15:07, "Olaf Hering" <olaf@aepfle.de> wrote:
>> 
>>> On Thu, Jul 28, Ian Campbell wrote:
>>> 
>>>>> Are you suggesting that there have to be ways for a domU to query the
>>>>> state of its registered watches and shut them all down during very early
>>>>> boot?
>>>> 
>>>> Perhaps the xenstore protocol could be enhanced with a mechanism to
>>>> clear all existing watches? A kernel could call that at start of day.
>>> 
>>> I poked around in the xenstore sources, there is appearently nothing
>>> that can be used to shutdown all. Or would calling XS_RELEASE do the trick?
>> 
>> XS_INTRODUCE
> 
> Unfortunately do_introduce() is not preprared for that.
> 
> On enter, conn->id contains the domain_id, so it errors out early.
> Later it compares domain->conn != conn and does not enter the correct path.

Oh of course you want to do it from *inside* the guest...

> If I remove both checks the kexec appears to work ok.
> Is it save to remove both checks?

Hmm, no. Attached patch would be safer, give it a try.

And note that it is *dangerous* to reset the domain xenstore connection if
there could be any other concurrent activity on the connection that could
confuse the guest kernel. So doing the reset during kernel bringup might be
safest, there you can do it in the kernel before the whole xenbus subsystem
is fully up.

 -- Keir

> The only issue I havent figured out:
> How to get the domid from within the kernel?
> 
> Olaf
> 
> diff -r 42edf1481c57 tools/xenstore/xenstored_domain.c
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -326,7 +326,7 @@ void do_introduce(struct connection *con
> return;
> }
>  
> - if (conn->id != 0 || !conn->can_write) {
> + if (!conn->can_write) {
> send_error(conn, EACCES);
> return;
> }
> @@ -365,7 +365,7 @@ void do_introduce(struct connection *con
> talloc_steal(domain->conn, domain);
>  
> fire_watches(NULL, "@introduceDomain", false);
> - } else if ((domain->mfn == mfn) && (domain->conn != conn)) {
> + } else if (domain->mfn == mfn) {
> /* Use XS_INTRODUCE for recreating the xenbus event-channel. */
> if (domain->port)
> xc_evtchn_unbind(xce_handle, domain->port);


[-- Attachment #2: 00-xenstore-introduce --]
[-- Type: application/octet-stream, Size: 890 bytes --]

diff -r 0f36c2eec2e1 tools/xenstore/xenstored_domain.c
--- a/tools/xenstore/xenstored_domain.c	Thu Jul 28 15:40:54 2011 +0100
+++ b/tools/xenstore/xenstored_domain.c	Thu Jul 28 21:26:49 2011 +0100
@@ -326,7 +326,7 @@ void do_introduce(struct connection *con
 		return;
 	}
 
-	if (conn->id != 0 || !conn->can_write) {
+	if (!conn->can_write) {
 		send_error(conn, EACCES);
 		return;
 	}
@@ -343,7 +343,16 @@ void do_introduce(struct connection *con
 
 	domain = find_domain_by_domid(domid);
 
-	if (domain == NULL) {
+	if (conn->id != 0) {
+		if ((domain == NULL) || (domain->conn != conn)) {
+			send_error(conn, EACCES);
+			return;
+		}
+		if ((domain->mfn != mfn) || (domain->remote_port != port)) {
+			send_error(conn, EINVAL);
+			return;
+		}
+	} else if (domain == NULL) {
 		interface = xc_map_foreign_range(
 			*xc_handle, domid,
 			getpagesize(), PROT_READ|PROT_WRITE, mfn);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-28 20:30           ` Keir Fraser
@ 2011-08-01 13:01             ` Olaf Hering
  2011-08-01 14:35               ` Keir Fraser
  0 siblings, 1 reply; 18+ messages in thread
From: Olaf Hering @ 2011-08-01 13:01 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Ian Campbell, Jan Beulich

On Thu, Jul 28, Keir Fraser wrote:

> > Unfortunately do_introduce() is not preprared for that.
> > 
> > On enter, conn->id contains the domain_id, so it errors out early.
> > Later it compares domain->conn != conn and does not enter the correct path.
> 
> Oh of course you want to do it from *inside* the guest...
> 
> > If I remove both checks the kexec appears to work ok.
> > Is it save to remove both checks?
> 
> Hmm, no. Attached patch would be safer, give it a try.
> 
> And note that it is *dangerous* to reset the domain xenstore connection if
> there could be any other concurrent activity on the connection that could
> confuse the guest kernel. So doing the reset during kernel bringup might be
> safest, there you can do it in the kernel before the whole xenbus subsystem
> is fully up.

I sent a different xenstored patch which takes DOMID_SELF into account.

Olaf

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-08-01 13:01             ` Olaf Hering
@ 2011-08-01 14:35               ` Keir Fraser
  0 siblings, 0 replies; 18+ messages in thread
From: Keir Fraser @ 2011-08-01 14:35 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel, Ian Campbell, Jan Beulich

On 01/08/2011 06:01, "Olaf Hering" <olaf@aepfle.de> wrote:

>> Hmm, no. Attached patch would be safer, give it a try.
>> 
>> And note that it is *dangerous* to reset the domain xenstore connection if
>> there could be any other concurrent activity on the connection that could
>> confuse the guest kernel. So doing the reset during kernel bringup might be
>> safest, there you can do it in the kernel before the whole xenbus subsystem
>> is fully up.
> 
> I sent a different xenstored patch which takes DOMID_SELF into account.

Saw it, I'll give it a think over. I think it looks okay, I wonder whether
DOMID_SELF handling should be pushed into find_domain_by_domid, but it's a
minor detail really. I certainly support the approach. Hopefully we can get
someone to similarly modify ocaml/xenstored too.

 -- Keir

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-26 14:17   ` Konrad Rzeszutek Wilk
@ 2011-07-26 14:28     ` Olaf Hering
  0 siblings, 0 replies; 18+ messages in thread
From: Olaf Hering @ 2011-07-26 14:28 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel

On Tue, Jul 26, Konrad Rzeszutek Wilk wrote:

> On Tue, Jul 26, 2011 at 01:52:11PM +0200, Olaf Hering wrote:
> > Unregister the shutdown and sysrq watch during kexec.  The watches can
> > not be re-registered in the kexec kernel because they are still seen as
> > busy by xenstore.
> 
> So this is the PV or HVM guest doing the kexec?

Its for HVM guests with PV drivers.

Olaf

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-26 11:52 ` [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot Olaf Hering
@ 2011-07-26 14:17   ` Konrad Rzeszutek Wilk
  2011-07-26 14:28     ` Olaf Hering
  0 siblings, 1 reply; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-07-26 14:17 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel

On Tue, Jul 26, 2011 at 01:52:11PM +0200, Olaf Hering wrote:
> Unregister the shutdown and sysrq watch during kexec.  The watches can
> not be re-registered in the kexec kernel because they are still seen as
> busy by xenstore.

So this is the PV or HVM guest doing the kexec?

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  drivers/xen/manage.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> Index: linux-3.0/drivers/xen/manage.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/manage.c
> +++ linux-3.0/drivers/xen/manage.c
> @@ -320,6 +320,18 @@ static int shutdown_event(struct notifie
>  	return NOTIFY_DONE;
>  }
>  
> +static void xenbus_disable_shutdown_watcher(void)
> +{
> +	unregister_xenbus_watch(&shutdown_watch);
> +#ifdef CONFIG_MAGIC_SYSRQ
> +	unregister_xenbus_watch(&sysrq_watch);
> +#endif
> +}
> +
> +static struct syscore_ops xenbus_watcher_syscore_ops = {
> +	.shutdown = xenbus_disable_shutdown_watcher,
> +};
> +
>  int xen_setup_shutdown_event(void)
>  {
>  	static struct notifier_block xenstore_notifier = {
> @@ -329,6 +341,7 @@ int xen_setup_shutdown_event(void)
>  	if (!xen_domain())
>  		return -ENODEV;
>  	register_xenstore_notifier(&xenstore_notifier);
> +	register_syscore_ops(&xenbus_watcher_syscore_ops);
>  
>  	return 0;
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot
  2011-07-26 11:52 [PATCH 0/6] misc changes for kexec in pv-on-hvm guests Olaf Hering
@ 2011-07-26 11:52 ` Olaf Hering
  2011-07-26 14:17   ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 18+ messages in thread
From: Olaf Hering @ 2011-07-26 11:52 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: xen.syscore_ops.manage.shutdown_event.patch --]
[-- Type: text/plain, Size: 1156 bytes --]

Unregister the shutdown and sysrq watch during kexec.  The watches can
not be re-registered in the kexec kernel because they are still seen as
busy by xenstore.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/manage.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

Index: linux-3.0/drivers/xen/manage.c
===================================================================
--- linux-3.0.orig/drivers/xen/manage.c
+++ linux-3.0/drivers/xen/manage.c
@@ -320,6 +320,18 @@ static int shutdown_event(struct notifie
 	return NOTIFY_DONE;
 }
 
+static void xenbus_disable_shutdown_watcher(void)
+{
+	unregister_xenbus_watch(&shutdown_watch);
+#ifdef CONFIG_MAGIC_SYSRQ
+	unregister_xenbus_watch(&sysrq_watch);
+#endif
+}
+
+static struct syscore_ops xenbus_watcher_syscore_ops = {
+	.shutdown = xenbus_disable_shutdown_watcher,
+};
+
 int xen_setup_shutdown_event(void)
 {
 	static struct notifier_block xenstore_notifier = {
@@ -329,6 +341,7 @@ int xen_setup_shutdown_event(void)
 	if (!xen_domain())
 		return -ENODEV;
 	register_xenstore_notifier(&xenstore_notifier);
+	register_syscore_ops(&xenbus_watcher_syscore_ops);
 
 	return 0;
 }

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2011-08-01 14:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-27 23:13 [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot Jan Beulich
2011-07-28  5:25 ` Olaf Hering
2011-07-28 10:52   ` Ian Campbell
2011-07-28 11:00     ` Keir Fraser
2011-07-28 12:52       ` Ian Campbell
2011-07-28 11:02     ` Olaf Hering
2011-07-28 12:56       ` Ian Campbell
2011-07-28 14:07     ` Olaf Hering
2011-07-28 14:13       ` Keir Fraser
2011-07-28 19:50         ` Olaf Hering
2011-07-28 20:30           ` Keir Fraser
2011-08-01 13:01             ` Olaf Hering
2011-08-01 14:35               ` Keir Fraser
2011-07-28 16:09   ` Jan Beulich
2011-07-28 11:37 ` Stefano Stabellini
  -- strict thread matches above, loose matches on Subject: below --
2011-07-26 11:52 [PATCH 0/6] misc changes for kexec in pv-on-hvm guests Olaf Hering
2011-07-26 11:52 ` [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot Olaf Hering
2011-07-26 14:17   ` Konrad Rzeszutek Wilk
2011-07-26 14:28     ` Olaf Hering

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.