On Wed, May 04, 2016 at 07:57:27AM +0200, Hannes Reinecke wrote:
> The 'udev_sync' attribute is pointless without a cookie, so we
> can as well use the existence of the 'cookie' argument for
> the same function.
Well, I've already said that we should leave the cookies inside
dm_simplecmd, and if we do that, we need the udev_sync code.
One reason to do this is that cookies aren't just used for
synchronization. They are the mechanism by which udev flags are sent to
device-mapper. Now it just so happens that we currently aren't sending
any udev_flags when we are not asking for synchronization, but that
doesn't mean it will always be the case. So there could be a use
knowing whether or not we need synchronization, even in cases where we
know that we defintely need a cookie to pass flags to udev.
While again, we could do this by checking udev_flags as well and then
passing in a cookie and it wouldn't break anything, I just don't see the
point of passing in a pointer to a value that must be zero when we pass
it, and we're not allowed to use the result when we get it back.
-Ben
>
> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> ---
> libmultipath/devmapper.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index ab71fda..9facbb9 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -207,12 +207,10 @@ dm_prereq (void)
> #define do_deferred(x) ((x) == DEFERRED_REMOVE_ON || (x) == DEFERRED_REMOVE_IN_PROGRESS)
>
> static int
> -dm_simplecmd (int task, const char *name, int no_flush, int need_sync,
> +dm_simplecmd (int task, const char *name, int no_flush,
> uint16_t udev_flags, int deferred_remove, uint32_t *cookie)
> {
> int r = 0;
> - int udev_wait_flag = (need_sync && (task == DM_DEVICE_RESUME ||
> - task == DM_DEVICE_REMOVE));
> struct dm_task *dmt;
>
> if (!(dmt = dm_task_create (task)))
> @@ -231,7 +229,7 @@ dm_simplecmd (int task, const char *name, int no_flush, int need_sync,
> if (do_deferred(deferred_remove))
> dm_task_deferred_remove(dmt);
> #endif
> - if (udev_wait_flag &&
> + if (cookie &&
> !dm_task_set_cookie(dmt, cookie,
> DM_UDEV_DISABLE_LIBRARY_FALLBACK | udev_flags)) {
> dm_udev_complete(*cookie);
> @@ -239,7 +237,7 @@ dm_simplecmd (int task, const char *name, int no_flush, int need_sync,
> }
> r = dm_task_run (dmt);
>
> - if (udev_wait_flag) {
> + if (cookie) {
> if (!r)
> dm_udev_complete(*cookie);
> else
> @@ -254,22 +252,27 @@ extern int
> dm_simplecmd_flush (int task, const char *name, uint16_t udev_flags) {
> uint32_t cookie = 0;
>
> - return dm_simplecmd(task, name, 0, 1, udev_flags, 0, &cookie);
> + if (task == DM_DEVICE_RESUME ||
> + task == DM_DEVICE_REMOVE)
> + return dm_simplecmd(task, name, 0, udev_flags, 0, &cookie);
> + else
> + return dm_simplecmd(task, name, 0, udev_flags, 0, NULL);
> }
>
> extern int
> dm_simplecmd_noflush (int task, const char *name, int needsync, uint16_t udev_flags) {
> uint32_t cookie = 0;
>
> - return dm_simplecmd(task, name, 1, needsync, udev_flags, 0, &cookie);
> + return dm_simplecmd(task, name, 1, udev_flags, 0,
> + needsync ? &cookie : NULL);
> }
>
> static int
> dm_device_remove (const char *name, int needsync, int deferred_remove) {
> uint32_t cookie = 0;
>
> - return dm_simplecmd(DM_DEVICE_REMOVE, name, 0, needsync, 0,
> - deferred_remove, &cookie);
> + return dm_simplecmd(DM_DEVICE_REMOVE, name, 0, 0,
> + deferred_remove, needsync ? &cookie : NULL);
> }
>
> static int
> --
> 2.6.6
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
