Yosuke Iwamatsu
2009-Jan-28 08:46 UTC
[Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model
When we destroy a domain, xend sends SIGTERM to the device model and wait by waitpid() until the device model process disappears. If we restarted xend during the lifetime of the domain, waitpid() fails because the device model is no longer a child of xend, and in that case xend gives up waiting for the shutdown of process and just send it SIGKILL immediately. This is problematic because most of the case the device model will be forcibly killed by xend before shutting itself down. This patch adds time.sleep before sending SIGKILL to the device model. On my test box shutdown of a device model usually takes about 0.5 sec, so waiting two seconds should be enough in most cases. Regards, ----------------------- Yosuke Iwamatsu NEC Corporation Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2009-Jan-28 11:14 UTC
Re: [Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model
Yosuke Iwamatsu writes ("[Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model"):> When we destroy a domain, xend sends SIGTERM to the device model and > wait by waitpid() until the device model process disappears. > If we restarted xend during the lifetime of the domain, waitpid() fails > because the device model is no longer a child of xend, and in that case > xend gives up waiting for the shutdown of process and just send it > SIGKILL immediately. This is problematic because most of the case the > device model will be forcibly killed by xend before shutting itself > down.The code already has a timeout to forcibly kill the device model after (I think) 10 seconds. Surely we should reuse that code path (and the same timeout value) ? Restarting xend is not a usual thing to do and I think it''s OK if shutting down a domain started by a previous xend involves waiting for such a longer timeout. It''s better to err on the side of safety. Also, your patch was: Content-Type: all/allfiles; This is not a recognised content type and prevented both of my mailreaders from displaying it to me. Can you please fix your MUA ? Alternatively, just include the patch in the body of the mail rather than attaching it. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yosuke Iwamatsu
2009-Jan-29 08:40 UTC
[Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model
Ian Jackson wrote:> The code already has a timeout to forcibly kill the device model after > (I think) 10 seconds. Surely we should reuse that code path (and the > same timeout value) ? > > Restarting xend is not a usual thing to do and I think it''s OK if > shutting down a domain started by a previous xend involves waiting for > such a longer timeout. It''s better to err on the side of safety.O.K. Attached is a revised patch which reuses the existing code path. 10 seconds seems to me a bit too long, but I can agree we had better keep on the safe side.> Also, your patch was: > Content-Type: all/allfiles; > This is not a recognised content type and prevented both of my > mailreaders from displaying it to me. Can you please fix your MUA ?Sorry for the inconvenience. This time your mail client can recognize it, I think. -- Yosuke Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2009-Jan-29 11:12 UTC
Re: [Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model
Yosuke Iwamatsu writes ("[Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model"):> O.K. Attached is a revised patch which reuses the existing code path. > 10 seconds seems to me a bit too long, but I can agree we had better > keep on the safe side.That looks reasonable to me, thanks. (Although I haven''t tested it.)> Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com>Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yosuke Iwamatsu
2009-Feb-04 06:14 UTC
[Xen-devel] [PATCH] xend: Sleep before sending SIGKILL to device model
Keir, Would you mind applying this? -- Yosuke Yosuke Iwamatsu wrote:> Ian Jackson wrote: >> The code already has a timeout to forcibly kill the device model after >> (I think) 10 seconds. Surely we should reuse that code path (and the >> same timeout value) ? >> >> Restarting xend is not a usual thing to do and I think it''s OK if >> shutting down a domain started by a previous xend involves waiting for >> such a longer timeout. It''s better to err on the side of safety. > > O.K. Attached is a revised patch which reuses the existing code path. > 10 seconds seems to me a bit too long, but I can agree we had better > keep on the safe side. > >> Also, your patch was: >> Content-Type: all/allfiles; >> This is not a recognised content type and prevented both of my >> mailreaders from displaying it to me. Can you please fix your MUA ? > > Sorry for the inconvenience. > This time your mail client can recognize it, I think. > > -- Yosuke > > Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com> > > > > > ------------------------------------------------------------------------ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel