Browse code

devmapper: ensure that UdevWait is called after calls to setCookie

Recent changes to devmapper broke the implicit requirement that UdevWait be
called after every call to task.setCookie. Failure to do so results in leaks of
semaphores in the LVM code, eventually leading to semaphore exhaustion.
Previously this was handled by calling UdevWait in a ubiquitous defer function.
While there was initially some concern with deferring the UdevWait function
would cause some amount of race possibiliy, the fact that we never return the
cookie value or any value used to find it, makes that possibility seem unlikely,
so lets go back to that method

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Neil Horman authored on 2017/06/20 01:11:48
Showing 1 changed files
... ...
@@ -333,6 +333,7 @@ func RemoveDevice(name string) error {
333 333
 	if err := task.setCookie(cookie, 0); err != nil {
334 334
 		return fmt.Errorf("devicemapper: Can not set cookie: %s", err)
335 335
 	}
336
+	defer UdevWait(cookie)
336 337
 
337 338
 	dmSawBusy = false // reset before the task is run
338 339
 	if err = task.run(); err != nil {
... ...
@@ -342,7 +343,7 @@ func RemoveDevice(name string) error {
342 342
 		return fmt.Errorf("devicemapper: Error running RemoveDevice %s", err)
343 343
 	}
344 344
 
345
-	return UdevWait(cookie)
345
+	return nil
346 346
 }
347 347
 
348 348
 // RemoveDeviceDeferred is a useful helper for cleaning up a device, but deferred.
... ...
@@ -368,10 +369,6 @@ func RemoveDeviceDeferred(name string) error {
368 368
 		return fmt.Errorf("devicemapper: Can not set cookie: %s", err)
369 369
 	}
370 370
 
371
-	if err = task.run(); err != nil {
372
-		return fmt.Errorf("devicemapper: Error running RemoveDeviceDeferred %s", err)
373
-	}
374
-
375 371
 	// libdevmapper and udev relies on System V semaphore for synchronization,
376 372
 	// semaphores created in `task.setCookie` will be cleaned up in `UdevWait`.
377 373
 	// So these two function call must come in pairs, otherwise semaphores will
... ...
@@ -381,8 +378,13 @@ func RemoveDeviceDeferred(name string) error {
381 381
 	// this call will not wait for the deferred removal's final executing, since no
382 382
 	// udev event will be generated, and the semaphore's value will not be incremented
383 383
 	// by udev, what UdevWait is just cleaning up the semaphore.
384
+	defer UdevWait(cookie)
385
+
386
+	if err = task.run(); err != nil {
387
+		return fmt.Errorf("devicemapper: Error running RemoveDeviceDeferred %s", err)
388
+	}
384 389
 
385
-	return UdevWait(cookie)
390
+	return nil
386 391
 }
387 392
 
388 393
 // CancelDeferredRemove cancels a deferred remove for a device.
... ...
@@ -476,12 +478,13 @@ func CreatePool(poolName string, dataFile, metadataFile *os.File, poolBlockSize
476 476
 	if err := task.setCookie(cookie, flags); err != nil {
477 477
 		return fmt.Errorf("devicemapper: Can't set cookie %s", err)
478 478
 	}
479
+	defer UdevWait(cookie)
479 480
 
480 481
 	if err := task.run(); err != nil {
481 482
 		return fmt.Errorf("devicemapper: Error running deviceCreate (CreatePool) %s", err)
482 483
 	}
483 484
 
484
-	return UdevWait(cookie)
485
+	return nil
485 486
 }
486 487
 
487 488
 // ReloadPool is the programmatic example of "dmsetup reload".
... ...
@@ -661,12 +664,13 @@ func ResumeDevice(name string) error {
661 661
 	if err := task.setCookie(cookie, 0); err != nil {
662 662
 		return fmt.Errorf("devicemapper: Can't set cookie %s", err)
663 663
 	}
664
+	defer UdevWait(cookie)
664 665
 
665 666
 	if err := task.run(); err != nil {
666 667
 		return fmt.Errorf("devicemapper: Error running deviceResume %s", err)
667 668
 	}
668 669
 
669
-	return UdevWait(cookie)
670
+	return nil
670 671
 }
671 672
 
672 673
 // CreateDevice creates a device with the specified poolName with the specified device id.
... ...
@@ -759,11 +763,13 @@ func activateDevice(poolName string, name string, deviceID int, size uint64, ext
759 759
 		return fmt.Errorf("devicemapper: Can't set cookie %s", err)
760 760
 	}
761 761
 
762
+	defer UdevWait(cookie)
763
+
762 764
 	if err := task.run(); err != nil {
763 765
 		return fmt.Errorf("devicemapper: Error running deviceCreate (ActivateDevice) %s", err)
764 766
 	}
765 767
 
766
-	return UdevWait(cookie)
768
+	return nil
767 769
 }
768 770
 
769 771
 // CreateSnapDeviceRaw creates a snapshot device. Caller needs to suspend and resume the origin device if it is active.