[PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Andrew Cooper]

This is a regression introduced by

"Switch from select() to poll() in xenconsoled's IO loop."
  hg c/s 26405:7359c3122c5d
  git cc5434c933153c4b8812d1df901f8915c22830a8

which results in reliable segfaults during VM power operations.

Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

---

These segfaults are 100% reproducible in a XenServer system when rebooting a
PV domain. I am at a loss to explain why the OSS testing has not picked them up.

diff -r 94ece33caae2 -r f66d74cbf492 tools/console/daemon/io.c
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -669,6 +669,7 @@ static struct domain *create_domain(int 
 	strcat(dom->conspath, "/console");
 
 	dom->master_fd = -1;
+	dom->master_pollfd = NULL;
 	dom->slave_fd = -1;
 	dom->log_fd = -1;
 
@@ -687,6 +688,7 @@ static struct domain *create_domain(int 
 	dom->remote_port = -1;
 	dom->interface = NULL;
 	dom->xce_handle = NULL;
+	dom->xce_pollfd = NULL;
 
 	if (!watch_domain(dom, true))
 		goto out;

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Olaf Hering]

On Thu, Mar 07, Andrew Cooper wrote:

> This is a regression introduced by
> 
> "Switch from select() to poll() in xenconsoled's IO loop."
>   hg c/s 26405:7359c3122c5d
>   git cc5434c933153c4b8812d1df901f8915c22830a8
> 
> which results in reliable segfaults during VM power operations.

What about switching to calloc(3), given that many members are zeroed
anyway?

Olaf

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Andrew Cooper]

On 07/03/13 15:28, Olaf Hering wrote:
> On Thu, Mar 07, Andrew Cooper wrote:
>
>> This is a regression introduced by
>>
>> "Switch from select() to poll() in xenconsoled's IO loop."
>>   hg c/s 26405:7359c3122c5d
>>   git cc5434c933153c4b8812d1df901f8915c22830a8
>>
>> which results in reliable segfaults during VM power operations.
> What about switching to calloc(3), given that many members are zeroed
> anyway?
>
> Olaf

That also works, and is better going forwards.  V2 on its way.

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Wei Liu]

On Thu, 2013-03-07 at 15:13 +0000, Andrew Cooper wrote:
> This is a regression introduced by
> 
> "Switch from select() to poll() in xenconsoled's IO loop."
>   hg c/s 26405:7359c3122c5d
>   git cc5434c933153c4b8812d1df901f8915c22830a8
> 
> which results in reliable segfaults during VM power operations.
> 
> Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 

Good catch. Thanks.


Wei.

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Andrew Cooper]

On 07/03/13 16:20, Wei Liu wrote:
> On Thu, 2013-03-07 at 15:13 +0000, Andrew Cooper wrote:
>> This is a regression introduced by
>>
>> "Switch from select() to poll() in xenconsoled's IO loop."
>>   hg c/s 26405:7359c3122c5d
>>   git cc5434c933153c4b8812d1df901f8915c22830a8
>>
>> which results in reliable segfaults during VM power operations.
>>
>> Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
> Good catch. Thanks.
>
>
> Wei.
>

Sadly, after fixing these segfaults, the code as currently is will cause
xenconsoled to exit gracefully as soon as you try and boot the 128th
domain.  We are currently investigating the issue.

~Andrew

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Wei Liu]

On Thu, 2013-03-07 at 16:22 +0000, Andrew Cooper wrote:
> On 07/03/13 16:20, Wei Liu wrote:
> > On Thu, 2013-03-07 at 15:13 +0000, Andrew Cooper wrote:
> >> This is a regression introduced by
> >>
> >> "Switch from select() to poll() in xenconsoled's IO loop."
> >>   hg c/s 26405:7359c3122c5d
> >>   git cc5434c933153c4b8812d1df901f8915c22830a8
> >>
> >> which results in reliable segfaults during VM power operations.
> >>
> >> Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>
> > Good catch. Thanks.
> >
> >
> > Wei.
> >
> 
> Sadly, after fixing these segfaults, the code as currently is will cause
> xenconsoled to exit gracefully as soon as you try and boot the 128th
> domain.  We are currently investigating the issue.
> 

Odd. So you were implying if you didn't fix this bug, you succeeded in
booting up >128 guests?

All the exit paths have "dolog" which outputs to stderr, so maybe run
xenconsoled in foreground can help.


Wei.

> ~Andrew

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Andrew Cooper]

On 07/03/13 17:15, Wei Liu wrote:
> On Thu, 2013-03-07 at 16:22 +0000, Andrew Cooper wrote:
>> On 07/03/13 16:20, Wei Liu wrote:
>>> On Thu, 2013-03-07 at 15:13 +0000, Andrew Cooper wrote:
>>>> This is a regression introduced by
>>>>
>>>> "Switch from select() to poll() in xenconsoled's IO loop."
>>>>   hg c/s 26405:7359c3122c5d
>>>>   git cc5434c933153c4b8812d1df901f8915c22830a8
>>>>
>>>> which results in reliable segfaults during VM power operations.
>>>>
>>>> Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>> Good catch. Thanks.
>>>
>>>
>>> Wei.
>>>
>> Sadly, after fixing these segfaults, the code as currently is will cause
>> xenconsoled to exit gracefully as soon as you try and boot the 128th
>> domain.  We are currently investigating the issue.
>>
> Odd. So you were implying if you didn't fix this bug, you succeeded in
> booting up >128 guests?
>
> All the exit paths have "dolog" which outputs to stderr, so maybe run
> xenconsoled in foreground can help.
>
>
> Wei.

After fixing the segfault bugs, xenconsoled intermittently fails and
exits with:

Mar  7 16:45:49 localhost /usr/sbin/xenconsoled: Failure in poll
xs_handle: 3 (No such process)

The test case is attempting to sequentially boot 1000 PV VMs on the same
host.

So far, the common theme of failures appear to be at multiples of 128
VMs.  This is usually at the 128th VM, but also seen at the 384th VM.

~Andrew

Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] tools/xenconsoled: Initialise pointers before trying to use them"):
> On 07/03/13 16:20, Wei Liu wrote:
> > On Thu, 2013-03-07 at 15:13 +0000, Andrew Cooper wrote:
> >> This is a regression introduced by
> >>
> >> "Switch from select() to poll() in xenconsoled's IO loop."
> >>   hg c/s 26405:7359c3122c5d
> >>   git cc5434c933153c4b8812d1df901f8915c22830a8
> >>
> >> which results in reliable segfaults during VM power operations.
> >>
> >> Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
...
> Sadly, after fixing these segfaults, the code as currently is will cause
> xenconsoled to exit gracefully as soon as you try and boot the 128th
> domain.  We are currently investigating the issue.

Right.  Nevertheless, I've applied your patch to be going on with.

Ian.