Move UdevWait from defer to inline

All LVM actions in the devicemapper library are asyncronous, involving a call to
a task enqueue function (dm_run_task) and a wait on a resultant udev event
(UdevWait).  Currently devmapper.go defers all calls to UdevWait, which discards
the return value.  While it still generates an error message in the log (if
debugging is enabled), the calling thread is still allowed to continue as if no
error has occured, leading to subsequent errors, and significant confusion when
debugging, due to those subsequent errors.  Given that there is no risk of panic
between the task submission and the wait operation, it seems more reasonable to
preform the UdevWait inline at the end of any given lvm action so that errors
can be caught and returned before docker can continue and create additional
failures.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
This commit is contained in:
Neil Horman 2017-05-09 13:30:10 -04:00 committed by root
parent edd1c9e325
commit 5206d45e70
1 changed files with 9 additions and 15 deletions

View File

@ -332,7 +332,6 @@ func RemoveDevice(name string) error {
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.
@ -368,6 +367,10 @@ func RemoveDeviceDeferred(name string) error {
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.
@ -477,13 +475,12 @@ func CreatePool(poolName string, dataFile, metadataFile *os.File, poolBlockSize
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".
@ -663,13 +660,12 @@ func ResumeDevice(name string) error {
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.
@ -762,13 +758,11 @@ func activateDevice(poolName string, name string, deviceID int, size uint64, ext
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.