Re: [PATCH 2/2] Manage: Inform udev about device removal when stopping

From: Sebastian Parschauer <sebastian.riemer@profitbricks.com>
To: Jes Sorensen <Jes.Sorensen@redhat.com>
Cc: Brassow Jonathan <jbrassow@redhat.com>,
	NeilBrown <neilb@suse.com>,
	linux-raid <linux-raid@vger.kernel.org>,
	Artur Paszkiewicz <artur.paszkiewicz@intel.com>,
	systemd-devel@freedesktop.org, Shaohua Li <shli@kernel.org>
Subject: Re: [PATCH 2/2] Manage: Inform udev about device removal when stopping
Date: Tue, 16 Feb 2016 19:03:51 +0100	[thread overview]
Message-ID: <56C36487.3060201@profitbricks.com> (raw)
In-Reply-To: <wrfjio1oeemz.fsf@redhat.com>

On 16.02.2016 18:41, Jes Sorensen wrote:
> Sebastian Parschauer <sebastian.riemer@profitbricks.com> writes:
>> When stopping an MD device, then its device node /dev/mdX may still
>> exist afterwards or it is recreated by udev. The next open() call
>> can lead to creation of an inoperable MD device. The reason for
>> this is that a change event (KOBJ_CHANGE) is announced to udev.
>> So announce a removal event (KOBJ_REMOVE) to udev instead.
>>
>> This also overrides the change event sent by the kernel.
>>
>> Signed-off-by: Sebastian Parschauer <sebastian.riemer@profitbricks.com>
>> ---
>>  Manage.c |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Manage.c b/Manage.c
>> index 7e1b94b..bc89764 100644
>> --- a/Manage.c
>> +++ b/Manage.c
>> @@ -494,13 +494,13 @@ done:
>>  		goto out;
>>  	}
>>  	/* prior to 2.6.28, KOBJ_CHANGE was not sent when an md array
>> -	 * was stopped, so We'll do it here just to be sure.  Drop any
>> -	 * partitions as well...
>> +	 * was stopped, it should be KOBJ_REMOVE instead, so we set the
>> +	 * remove event here just to be sure. Drop any partitions as well...
>>  	 */
>>  	if (fd >= 0)
>>  		ioctl(fd, BLKRRPART, 0);
>>  	if (mdi)
>> -		sysfs_uevent(mdi, "change");
>> +		sysfs_uevent(mdi, "remove");
> 
> I am a little concerned about this change. You assume the kernel and
> mdadm will be updated in sync, which is unlikely to happen. I believe
> you need to match the kernel version and send the corresponding event
> currectly for this to work correctly?

The worst thing that can happen is that the kernel sends the change
event after the remove event. Then it is the current situation again.
From my tests mdadm does enough other stuff in between. Udev is able to
handle receiving two remove events from my testing. Multiple mdadm
instances can't run in parallel any ways. So userspace around it needs
some serialization for it any ways. So also stopping an MD device and
assembling a new one with the same minor number shouldn't race.

I still prefer this solution here. But if you decide to drop the udev
event sending in mdadm, then I'm also fine with that.

Cheers,
Sebastian
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel