On Wed, Apr 27, 2016 at 01:10:37PM +0200, Hannes Reinecke wrote: > Instead of generating the cookie internally we should be > passing in the cookie to dm_addmap().
These look like they could actually cause problems. With dm_addmap_create and dm_addmap_reload, you could be in a situation where you get a cookie back on your first call to dm_task_set_cookie, wait on it, so that cookie is no longer in use, and then use that same cookie id again. It's possible that after you first waited on the cookie, someone else could have been given that cookie id (without looking into the dm cookie code more, I'm not sure how likely this is). If that is so, the second call to dm_addmap would be adding its command to the list of other commands for that cookie (since you are allowed to use a cookie for multiple dm commands). Calling dm_udev_wait would make it wait of all the commands. We can use a cookie multiple times and then wait on it once. But we can't keep reusing the cookie after we've waited on it, because someone else could be using the same cookie. -Ben > > Signed-off-by: Hannes Reinecke <h...@suse.de> > --- > libmultipath/devmapper.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > index e6156f7..a96cc0e 100644 > --- a/libmultipath/devmapper.c > +++ b/libmultipath/devmapper.c > @@ -274,11 +274,10 @@ dm_device_remove (const char *name, int needsync, int > deferred_remove) { > > extern int > dm_addmap (int task, const char *target, struct multipath *mpp, > - char * params, int ro) { > + char * params, int ro, uint32_t *cookie) { > int r = 0; > struct dm_task *dmt; > char *prefixed_uuid = NULL; > - uint32_t cookie = 0; > > if (!(dmt = dm_task_create (task))) > return 0; > @@ -319,18 +318,18 @@ dm_addmap (int task, const char *target, struct > multipath *mpp, > dm_task_no_open_count(dmt); > > if (task == DM_DEVICE_CREATE && > - !dm_task_set_cookie(dmt, &cookie, > + !dm_task_set_cookie(dmt, cookie, > DM_UDEV_DISABLE_LIBRARY_FALLBACK)) { > - dm_udev_complete(cookie); > + dm_udev_complete(*cookie); > goto freeout; > } > r = dm_task_run (dmt); > > if (task == DM_DEVICE_CREATE) { > if (!r) > - dm_udev_complete(cookie); > + dm_udev_complete(*cookie); > else > - dm_udev_wait(cookie); > + dm_udev_wait(*cookie); > } > freeout: > if (prefixed_uuid) > @@ -345,11 +344,13 @@ dm_addmap (int task, const char *target, struct > multipath *mpp, > extern int > dm_addmap_create (struct multipath *mpp, char * params) { > int ro; > + uint32_t cookie = 0; > > for (ro = 0; ro <= 1; ro++) { > int err; > > - if (dm_addmap(DM_DEVICE_CREATE, TGT_MPATH, mpp, params, ro)) > + if (dm_addmap(DM_DEVICE_CREATE, TGT_MPATH, > + mpp, params, ro, &cookie)) > return 1; > /* > * DM_DEVICE_CREATE is actually DM_DEV_CREATE + DM_TABLE_LOAD. > @@ -374,11 +375,15 @@ dm_addmap_create (struct multipath *mpp, char * params) > { > > extern int > dm_addmap_reload (struct multipath *mpp, char *params) { > - if (dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, ADDMAP_RW)) > + uint32_t cookie = 0; > + > + if (dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, > + ADDMAP_RW, &cookie)) > return 1; > if (errno != EROFS) > return 0; > - return dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, ADDMAP_RO); > + return dm_addmap(DM_DEVICE_RELOAD, TGT_MPATH, mpp, params, > + ADDMAP_RO, &cookie); > } > > extern int > -- > 2.6.6 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel