Re: [PATCH 2/2] reap the blktapctl thread and notify the tapdisk backend driver to release resource like memory..

["Jan Beulich" <JBeulich@novell.com>]

>>> Ian Jackson <Ian.Jackson@eu.citrix.com> 06.05.10 18:01 >>>
>James Song writes ("[Xen-devel] [PATCH 2/2] reap the blktapctl thread and notify the tapdisk backend driver to release resource like memory.."):
>> I write a new patch for this issue, which modified qemu code.  So
>> Ian, could you take a look this patch,too.
>
>As far as I can see the effect here is to catch SIGINT in order to do
>some kind of cleanup.  I don't think that is right.  qemu could quite
>easily crash (and does!) so it is not right for cleanup to happen in
>qemu.
>
>If there is any cleanup that needs doing it needs to be done by qemu's
>caller.
>
>Reading the message you refer to, surely it should be the job of the
>toolstack (xend or libxl) to ensure that the backends are instructed
>to do all necessary releasing ?

No (or not only): The cleanup done here is to close open file handles
and/or mmap-s associated with blktap. You may have seen the kernel
side patches to allow the system as a whole to recover from that
state (particularly when qemu-dm crashes), but in general I consider
it bad practice for an application to keep open huge amounts of
mapped memory when getting orderly terminated.

"Orderly" in the qemu-dm case unfortunately means being terminated
by a signal, hence the signal should be intercepted by qemu
(otherwise, i.e. in the current state) the design seems broken to me.

Having said that doesn't mean that I agree to the blktap-centric
approach taken by the patch. Imo global cleanup should be
performed by qemu-dm upon being terminated - the question just is
whether such code already exists (and just needs to be hooked up),
or whether that part is missing altogether and needs to be written
from scratch.

Jan