[lvm-devel] [PATCH] Avoid recursive calls from dmeventd to itself

[Petr Rockai]

Alasdair G Kergon <agk at redhat.com> writes:

> On Thu, Sep 01, 2011 at 03:08:03AM +0200, Peter Rockai wrote:
>> -		lvm2_run(_lvm_handle, "_memlock_dec");
>> +		lvm2_run(_lvm_handle, "_dmeventd_leave");
>
> I'm not keen on overloading it like that.
>
> If it's being used with cmdlib and lvm2_run, use of existing cmdline parameters
> ought to be sufficient to achieve this.

While that is true, I doubt it is a good idea. If we say that dmeventd
is not supposed to call itself recursively (which I believe is the
correct approach), we should enforce that globally. Asking every call
site to take care of this individually is error prone and bound to
introduce the bug sooner or later (especially with the rampant
copy-and-paste programming found in LVM :\).

> It might be a little more code, but I'd rather see it a property of the handle
> never to perform dmeventd monitoring calls.  We never fixed handle init to 
> take multiple settings, so maybe call
>   void lvm2_disable_dmeventd_monitoring(void *handle)
> after handle initialisation to set DMEVENTD_MONITOR_DISABLED and have that take
> precedence over any later attempt to enable monitoring.

This sounds like a better option. Now that raises the question why we
aren't doing the same thing for memlock_{inc,dec}? We should certainly
unify these two things. I'll have a look, and unless I run into a
stumbling block will submit a patch to do that.

(Of course, it would be much better if we could instead move dmeventd
plugins over to using liblvm2api (replacing liblvm2cmd), but I suspect
there's a lot of stuff to resolve before we can hit that mark.)

Petr

-- 
id' Ash = Ash; id' Dust = Dust; id' _ = undefined