[dm-devel] [PATCH] libmultipath: don't lock block device but use lock files

[Christian Seiler]

Thanks for your quick response!

On 05/24/2015 09:07 AM, Hannes Reinecke wrote:
> On 05/23/2015 11:00 PM, Christian Seiler wrote:
>> In recent versions udev uses flock() on the device node itself,
>> breaking libmultipath's current locking logic. Since libmultipath
>> doesn't actually modify anything on the device itself (and hence does
>> not actually need an exclusive lock on the device node, unlike e.g.
>> tools that create partitions etc.), and the reason for the lock is to
>> guard against multipath interfering with multipathd, use lock files
>> (named after the devices) in /run/lock/multipath instead.
>>
>> See the discussion under:
>> https://www.redhat.com/archives/dm-devel/2014-December/msg00083.html
>> Especially:
>> https://www.redhat.com/archives/dm-devel/2014-December/msg00117.html
>>
>> Signed-off-by: Christian Seiler <christian at iwakd.de>
>>
>> ---
> Well ... couldn't you just use a shared lock here?

I think you misunderstand the nature of the lock here: the lock is in
place to serialize different libmultipath instances against each other,
i.e. two multipath programs running at the same time or multipath
called at the same time multipathd is running. As was mentioned in the
mail I linked in the commit message of my patch: it was introduced in
commit [1] with a description of the problem.

So you definitely need an exclusive lock to serialize libmultipath
against other instances of it. Because with a shared lock, you don't
gain anything, that can always be taken, so it will always succeed. But
since udev uses a shared lock itself nowadays, taking it out on the
device node itself is not going to work, so that's why my patch
resorts to using lock files.

Of course, _additionally_ taking out a shared lock on the device might
be advisable if you want to protect against tools that modify devices,
such as partitioning tools etc. (This is the reason why udev takes out
a shared lock: to make sure that when processing events and determining
the contents of the device, problems with tools that modify the
partition table etc. don't occur.) But that's not really necessary for
multipath, because libmultipath only opens the device to do some
information gathering on the device (INQUIRY command, determine
geometry, etc.), but doesn't really care about the contents (there's
never a see/read done on the fd).

That said, I've noticed that my patch might cause multipathd to have
the lock file fds open unnecessarily, so I've updated it to close them
on regular unlocking (but not on failure, because that means there will
be a retry). Version 2 is attached.

> I thought to have send this patch already; hmm.

I didn't see that, sorry. (I started looking at this problem in-depth
yesterday, found the thread in December, noticed no followup there in
the archive's web interface, noticed no commit in git regarding this
issue and hence checked out the git source code for the first time and
started working on the patch.)

But as I elaborated: I don't think that helps.

Christian

[1] http://git.opensvc.com/gitweb.cgi?p=multipath-tools/.git;a=commit;h=4bc295cef7a50ea46bfc7b98c65848babf56c822
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libmultipath-don-t-lock-block-device-but-use-lock-fi.patch
Type: text/x-diff
Size: 5048 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20150524/a27b3714/attachment.bin>