[PATCH] Add timeout to xenconsole to fix race condition in xm create -c

[PATCH] Add timeout to xenconsole to fix race condition in xm create -c

[Anthony Liguori]

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

This should address the problems people are having now.

Wait a little bit for tty to appear.  There is a race condition that 
occurs after xend creates a domain.  Since no event triggers consoled to 
re-examine existing domains, we'll often not see the new pty by the time 
we're here.  Since consoled sleeps for 2 second periods, a 5 second 
timeout should keep us covered.

A xenstore watch isn't much better since we don't want to block forever 
if given an invalid domain or worse yes, a domain that someone else has 
connected to.

Signed-off-by: Anthony Liguori

Regards,

Anthony Liguori

[-- Attachment #2: 6482_consoled.diff --]
[-- Type: text/x-patch, Size: 1992 bytes --]

# HG changeset patch
# User Anthony Liguori <aliguori@us.ibm.com>
# Node ID fe6c5ecea53aabedc6b53988da25910e108eafe9
# Parent  551870a55f240791695d30fd7fa92a1bf4e48387
Wait for domain tty to become available.

diff -r 551870a55f24 -r fe6c5ecea53a tools/console/client/main.c
--- a/tools/console/client/main.c	Tue Aug 30 17:53:49 2005
+++ b/tools/console/client/main.c	Tue Aug 30 22:01:01 2005
@@ -176,6 +176,7 @@
 	unsigned int len = 0;
 	struct xs_handle *xs;
 	char *end;
+	time_t now;
 
 	while((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
 		switch(ch) {
@@ -215,13 +216,37 @@
 
 	snprintf(path, sizeof(path), "/console/%d/tty", domid);
 	str_pty = xs_read(xs, path, &len);
+
 	/* FIXME consoled currently does not assume domain-0 doesn't have a
 	   console which is good when we break domain-0 up.  To keep us
 	   user friendly, we'll bail out here since no data will ever show
 	   up on domain-0. */
-	if (domid == 0 || str_pty == NULL) {
+	if (domid == 0) {
 		err(errno, "Could not read tty from store");
 	}
+
+	/* FIXME wait a little bit for tty to appear.  There is a race
+	   condition that occurs after xend creates a domain.  Since no event
+           triggers consoled to re-examine existing domains, we'll often not
+           see the new pty by the time we're here.  Since consoled sleeps for
+	   2 second periods, a 5 second timeout should keep us covered.
+
+	   A xenstore watch isn't much better since we don't want to block
+           forever if given an invalid domain or worse yes, a domain that
+           someone else has connected to. */
+
+	now = time(0);
+	while (str_pty == NULL && (now + 5) > time(0)) {
+		struct timeval tv = { 0, 500 };
+		select(0, NULL, NULL, NULL, &tv); /* pause briefly */
+
+		str_pty = xs_read(xs, path, &len);
+	}
+
+	if (str_pty == NULL) {
+		err(errno, "Could not read tty from store");
+	}
+
 	spty = open(str_pty, O_RDWR | O_NOCTTY);
 	if (spty == -1) {
 		err(errno, "Could not open tty `%s'", str_pty);

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

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

RE: [PATCH] Add timeout to xenconsole to fix race condition in xm create -c

[Nakajima, Jun]

Anthony Liguori wrote:
> This should address the problems people are having now.
> 
Anthony, Hi

It cleanly fixed the problem I had. Thanks a lot for the quick fix!

> Wait a little bit for tty to appear.  There is a race condition that
> occurs after xend creates a domain.  Since no event triggers consoled
> to re-examine existing domains, we'll often not see the new pty by
> the time we're here.  Since consoled sleeps for 2 second periods, a 5
> second timeout should keep us covered.
> 
> A xenstore watch isn't much better since we don't want to block
> forever if given an invalid domain or worse yes, a domain that
> someone else has connected to.
> 
> Signed-off-by: Anthony Liguori
> 
> Regards,
> 
> Anthony Liguori


Jun
---
Intel Open Source Technology Center

Re: [PATCH] Add timeout to xenconsole to fix race condition in xm create -c

[Christian Limpach]

On 8/30/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This should address the problems people are having now.
> 
> Wait a little bit for tty to appear.  There is a race condition that
> occurs after xend creates a domain.  Since no event triggers consoled to
> re-examine existing domains, we'll often not see the new pty by the time
> we're here.  Since consoled sleeps for 2 second periods, a 5 second
> timeout should keep us covered.

I've applied this with slighly changed comments since xenconsoled now
uses watches and should setup the tty almost immediately.  The race
still exists though.

> A xenstore watch isn't much better since we don't want to block forever
> if given an invalid domain or worse yes, a domain that someone else has
> connected to.

It would improve the responsiveness but a timeout is still needed for
the reasons you pointed out.

Finally, there's still an issue with storing the console tty
information in /console/<domid> -- there might be stale data from a
previous domain having the same domid and the client might run before
the console daemon has rewritten the tty entry.  I think this is best
fixed by moving the console tty information into the domain's /domain
tree.

    christian

Re: [PATCH] Add timeout to xenconsole to fix race condition in xm create -c

[Anthony Liguori]

Christian Limpach wrote:

>On 8/30/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
>  
>
>>This should address the problems people are having now.
>>
>>Wait a little bit for tty to appear.  There is a race condition that
>>occurs after xend creates a domain.  Since no event triggers consoled to
>>re-examine existing domains, we'll often not see the new pty by the time
>>we're here.  Since consoled sleeps for 2 second periods, a 5 second
>>timeout should keep us covered.
>>    
>>
>
>I've applied this with slighly changed comments since xenconsoled now
>uses watches and should setup the tty almost immediately.  The race
>still exists though.
>  
>
Yes, what we really want is a event-dispatch mechanism.  It's something 
we've talked about a bit here but without flushing out any sort of 
actual design.

>>A xenstore watch isn't much better since we don't want to block forever
>>if given an invalid domain or worse yes, a domain that someone else has
>>connected to.
>>    
>>
>
>It would improve the responsiveness but a timeout is still needed for
>the reasons you pointed out.
>
>Finally, there's still an issue with storing the console tty
>information in /console/<domid> -- there might be stale data from a
>previous domain having the same domid and the client might run before
>the console daemon has rewritten the tty entry.  I think this is best
>fixed by moving the console tty information into the domain's /domain
>tree.
>  
>
Rusty had some objections to this originally--this is why its not there 
now.  Rusty?

Regards,

Anthony Liguori

>    christian
>
>  
>