[dm-devel] IBM request to allow unprivledged ioctls [Was: Revert "dm mpath: fix stalls when handling invalid ioctls"]

[Hannes Reinecke]

On 11/02/2015 02:31 PM, Mike Snitzer wrote:
> On Mon, Nov 02 2015 at  2:28am -0500,
> Hannes Reinecke <hare at suse.de> wrote:
> 
>> On 10/31/2015 11:47 PM, Mike Snitzer wrote:
>>>
>>> For Hannes, and in my head, it didn't matter if a future bdev satisfies
>>> the length condition.  I don't think Hannes was trying to guard against
>>> dangerous partition ioctls being issued by udev... but now I _do_
>>> question what exactly _is_ the point of his commit 21a2807bc3f.  Which
>>> invalid ioctls, from udev, did Hannes' change actually disallow?
>>>
> 
> FYI, I meant s/21a2807bc3f/a1989b33/
> 
>> The reasoning is thus:
>>
>> With the original code we would need to wait for path activation
>> before we would be able to figure out if the ioctl is valid.
>> However, the callback to verify the ioctl is sd_ioctl(), which as
>> a first step calls scsi_verify_ioctl().
>> So my reasoning was that we can as well call scsi_verify_ioctl()
>> early, and allow it to filter out known invalid ioctls.
>> Then we wouldn't need to wait for path activation and return
>> immediately.
> 
> Right, I understood that to be your intent.  And it seemed reasonible.
> Unfortuntely, as we now know, scsi_verify_blk_ioctl() only really cares
> to filter out ioctls that are invalid for partitions.
> 
> I'm still curious: which ioctls were being issued by udev that your
> commit a1989b33 "fixed" (so udev workers didn't hang)?  Is the right fix
> to modify udev to not issue such ioctls?  What was invalid about the
> udev issued ioctls?
> 
Thing is, it's not udev itself which issues ioctl. It's the programs
started by udev via the various rules (PROGRAM or IMPORT keywords).
They might (and occasionally, will) issue ioctls, and we have no
means of controlling them.

>> Incidentally, in the 'r == -ENOTCONN' case, we're waiting
>> for path activation, but then just bail out with -ENOTCONN.
>> As we're not resetting -ENOTCONN, where's the point in activate the
>> paths here?
>> Shouldn't we retry to figure out if -ENOTCONN is still true?
> 
> We do, in DM core, see _your_ commit that added this ;)
> 6c182cd88d ("dm mpath: fix ioctl deadlock when no paths")
> 
Indeed.

But then the real question remains:

What is the 'correct' behaviour for ioctls when no path retry
is active (or when no paths are present)?

Should we start path activation?
If so, should we wait for path activation to finish, risking udev
killing the worker for that event (udev has a built-in timeout of
120 seconds, which we might easily exceed when we need to activate
paths for large installations or slow path activation ... just
thinking of NetApp takeover/giveback cycle)?
If we're not waiting for path activation, where would be the point
in starting it, seeing that we're not actually interested in the result?
And if we shouldn't start a path activation, what is the point of
having code for it in the first place?
Couldn't we just rip it out altogether, and avoid much of the
current unpleasantness?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)