From edd1c9e3255ddd80c97313bd596812dbb3470a5b Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 9 May 2017 13:42:01 -0400 Subject: [PATCH] Make cookies for devicemapper operations unique Currently, the devicemapper library sets cookies to correlate wait operations, which must be unique (as the lvm2 library doesn't detect duplicate cookies). The current method for cookie generation is to take the address of a cookie variable. However, because the variable is declared on the stack, execution patterns can lead to the cookie variable being declared at the same stack location, which results in a high likelyhood of duplicate cookie use, which in turn can lead to various odd lvm behaviors, which can be hard to track down (object use before create, duplicate completions, etc). Lets guarantee that the cookie we generate is unique by declaring it on the heap instead. This guarantees that the address of the variable won't be reused until such time as the UdevWait operation completes, and drops its reference to it, at which time the gc can reclaim it. Signed-off-by: Neil Horman --- pkg/devicemapper/devmapper.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/devicemapper/devmapper.go b/pkg/devicemapper/devmapper.go index e97d166e78..cc02562c16 100644 --- a/pkg/devicemapper/devmapper.go +++ b/pkg/devicemapper/devmapper.go @@ -328,11 +328,11 @@ 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) + defer UdevWait(cookie) dmSawBusy = false // reset before the task is run if err = task.run(); err != nil { @@ -361,10 +361,10 @@ 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) } @@ -377,7 +377,7 @@ 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) + defer UdevWait(cookie) if err = task.run(); err != nil { return fmt.Errorf("devicemapper: Error running RemoveDeviceDeferred %s", err) @@ -471,13 +471,13 @@ 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) + defer UdevWait(cookie) if err := task.run(); err != nil { return fmt.Errorf("devicemapper: Error running deviceCreate (CreatePool) %s", err) @@ -659,11 +659,11 @@ 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) + defer UdevWait(cookie) if err := task.run(); err != nil { return fmt.Errorf("devicemapper: Error running deviceResume %s", err) @@ -757,12 +757,12 @@ 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) + defer UdevWait(cookie) if err := task.run(); err != nil { return fmt.Errorf("devicemapper: Error running deviceCreate (ActivateDevice) %s", err)