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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/16/2016 07:03 PM, Sebastian Parschauer wrote:
On 16.02.2016 18:41, Jes Sorensen wrote:
Sebastian Parschauer <sebastian.riemer@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
---
  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.

I strongly prefer removing the udev event generation altogether.
As this appears to be a carry-over from older kernels, it looks to me as being an incomplete conversion: At one point udev introduced 'ONLINE' and 'OFFLINE' events, which were supposed to be used for this kind of scenario. (ONLINE being a companion to 'ADD', and 'OFFLINE' being the companion to 'DELETE'). However, later the 'ONLINE' got modified to 'CHANGE', and the 'OFFLINE' got dropped completely.
Or that was the plan.
So it looks as if the conversion to 'CHANGE' got applied to the 'OFFLINE' event, too. Hence I strongly recommend to drop it completely, and let the kernel or the MD module decide if and when a uevent should be send.

Cheers,

Hannes
--
Dr. Hannes Reinecke		      zSeries & Storage
hare@xxxxxxx			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux