Merge pull request #33119 from nhorman/devmapper-fixes

Devmapper fixes for cookie generation and error reporting (fixes  #33050)

pkg/devicemapper/devmapper.go

@@ -257,7 +257,7 @@ func (t *Task) getNextTarget(next unsafe.Pointer) (nextPtr unsafe.Pointer, start
 // UdevWait waits for any processes that are waiting for udev to complete the specified cookie.
 func UdevWait(cookie *uint) error {
 	if res := DmUdevWait(*cookie); res != 1 {
-		logrus.Debugf("devicemapper: Failed to wait on udev cookie %d", *cookie)
+		logrus.Debugf("devicemapper: Failed to wait on udev cookie %d, %d", *cookie, res)
 		return ErrUdevWait
 	}
 	return nil
@@ -328,11 +328,10 @@ func RemoveDevice(name string) error {
 		return err
 	}
 
-	var cookie uint
-	if err := task.setCookie(&cookie, 0); err != nil {
+	cookie := new(uint)
+	if err := task.setCookie(cookie, 0); err != nil {
 		return fmt.Errorf("devicemapper: Can not set cookie: %s", err)
 	}
-	defer UdevWait(&cookie)
 
 	dmSawBusy = false // reset before the task is run
 	if err = task.run(); err != nil {
@@ -342,7 +341,7 @@ func RemoveDevice(name string) error {
 		return fmt.Errorf("devicemapper: Error running RemoveDevice %s", err)
 	}
 
-	return nil
+	return UdevWait(cookie)
 }
 
 // RemoveDeviceDeferred is a useful helper for cleaning up a device, but deferred.
@@ -361,13 +360,17 @@ func RemoveDeviceDeferred(name string) error {
 	// set a task cookie and disable library fallback, or else libdevmapper will
 	// disable udev dm rules and delete the symlink under /dev/mapper by itself,
 	// even if the removal is deferred by the kernel.
-	var cookie uint
+	cookie := new(uint)
 	var flags uint16
 	flags = DmUdevDisableLibraryFallback
-	if err := task.setCookie(&cookie, flags); err != nil {
+	if err := task.setCookie(cookie, flags); err != nil {
 		return fmt.Errorf("devicemapper: Can not set cookie: %s", err)
 	}
 
+	if err = task.run(); err != nil {
+		return fmt.Errorf("devicemapper: Error running RemoveDeviceDeferred %s", err)
+	}
+
 	// libdevmapper and udev relies on System V semaphore for synchronization,
 	// semaphores created in `task.setCookie` will be cleaned up in `UdevWait`.
 	// So these two function call must come in pairs, otherwise semaphores will
@@ -377,13 +380,8 @@ func RemoveDeviceDeferred(name string) error {
 	// this call will not wait for the deferred removal's final executing, since no
 	// udev event will be generated, and the semaphore's value will not be incremented
 	// by udev, what UdevWait is just cleaning up the semaphore.
-	defer UdevWait(&cookie)
-
-	if err = task.run(); err != nil {
-		return fmt.Errorf("devicemapper: Error running RemoveDeviceDeferred %s", err)
-	}
 
-	return nil
+	return UdevWait(cookie)
 }
 
 // CancelDeferredRemove cancels a deferred remove for a device.
@@ -471,19 +469,18 @@ func CreatePool(poolName string, dataFile, metadataFile *os.File, poolBlockSize
 		return fmt.Errorf("devicemapper: Can't add target %s", err)
 	}
 
-	var cookie uint
+	cookie := new(uint)
 	var flags uint16
 	flags = DmUdevDisableSubsystemRulesFlag | DmUdevDisableDiskRulesFlag | DmUdevDisableOtherRulesFlag
-	if err := task.setCookie(&cookie, flags); err != nil {
+	if err := task.setCookie(cookie, flags); err != nil {
 		return fmt.Errorf("devicemapper: Can't set cookie %s", err)
 	}
-	defer UdevWait(&cookie)
 
 	if err := task.run(); err != nil {
 		return fmt.Errorf("devicemapper: Error running deviceCreate (CreatePool) %s", err)
 	}
 
-	return nil
+	return UdevWait(cookie)
 }
 
 // ReloadPool is the programmatic example of "dmsetup reload".
@@ -659,17 +656,16 @@ func ResumeDevice(name string) error {
 		return err
 	}
 
-	var cookie uint
-	if err := task.setCookie(&cookie, 0); err != nil {
+	cookie := new(uint)
+	if err := task.setCookie(cookie, 0); err != nil {
 		return fmt.Errorf("devicemapper: Can't set cookie %s", err)
 	}
-	defer UdevWait(&cookie)
 
 	if err := task.run(); err != nil {
 		return fmt.Errorf("devicemapper: Error running deviceResume %s", err)
 	}
 
-	return nil
+	return UdevWait(cookie)
 }
 
 // CreateDevice creates a device with the specified poolName with the specified device id.
@@ -757,18 +753,16 @@ func activateDevice(poolName string, name string, deviceID int, size uint64, ext
 		return fmt.Errorf("devicemapper: Can't add node %s", err)
 	}
 
-	var cookie uint
-	if err := task.setCookie(&cookie, 0); err != nil {
+	cookie := new(uint)
+	if err := task.setCookie(cookie, 0); err != nil {
 		return fmt.Errorf("devicemapper: Can't set cookie %s", err)
 	}
 
-	defer UdevWait(&cookie)
-
 	if err := task.run(); err != nil {
 		return fmt.Errorf("devicemapper: Error running deviceCreate (ActivateDevice) %s", err)
 	}
 
-	return nil
+	return UdevWait(cookie)
 }
 
 // CreateSnapDeviceRaw creates a snapshot device. Caller needs to suspend and resume the origin device if it is active.