[dm-devel] [lvm-devel] master - multipathd: fix fd leak when iscsi device logs in

[Martin Wilck]

Hi Zdenek,

On Mon, 2020-07-13 at 11:56 +0200, Zdenek Kabelac wrote:
> Dne 13. 07. 20 v 11:21 Martin Wilck napsal(a):
> > Hi Lixiaokeng,
> > 
> > 
> > @Zdenek, do we have to protect every libdm call, or is it
> > sufficient
> > to protect only dm_task_run(), as lixiaokeng suggested?
> > 
> 
> Hi
> 
> It's actually hard to answer it in a simple way.
> Several properties are held in library static variables. So
> converting libdm
> into a fully threaded 'version' would basically require to duplicate
> all API 
> functions with extended 'context' structure passed in - where all
> buffers can 
> be maintained properly (and it's getting more complicated with signal
> handling 
> and debug logging).

Sure. I don't think this is where we're going for. But we need to
figure out how to handle this properly in libmultipath.
I take it that we must really protect *every* libdm call in
libmultipath and multipathd if we want to do it right.

> ATM it doesn't look like there is a big need for threaded support of
> DM usage
> as majority of tools spends most of their time outside thus
> 'serialization'
> of lvm2 or dmeventd on libdm access look doesn't look like a big
> issue
> (let's say there are far bigger fishes to hunt).

Agreed.

> As for the issue of keeping control_fd open - there should be a
> synchronized 
> call of dm_hold_control_dev(0/1) -  see the codebase of  dmeventd.c
> in lvm2 
> code base - how we solve the control_fd handling.

Ben has already added support for dm_hold_control_dev() in libmultipath
(e24d8b1 ("libmutipath: don't close fd on dm_lib_release")). But this
doesn't protect us from calling _open_control() simultaneously in
separate code paths, as Lixiaokeng has pointed out.

I don't see how it would, as dm_hold_control_dev() also just changes
a static variable.

Thanks for your quick answer,
Martin