[PATCH] discover/devmapper: Retry dm-device remove if busy

[Samuel Mendoza-Jonas]

On Tue, 2018-12-04 at 13:34 +1100, Samuel Mendoza-Jonas wrote:
> Buildroot's libdm is not built with --enable-udev_sync, so device-mapper
> actions are not able to sync or wait for udev events.
> (see 185676316, "discover/devmapper: Disable libdm udev sync support")
> 
> This can cause an issue when tearing down a snapshot in
> devmapper_destroy_snapshot() which performs a DM_DEVICE_REMOVE task
> against the snapshot, origin, and base devices one after the other. In
> some cases if the interval between these actions is too short the action
> can fail as the preceding device hasn't disappeared yet and the device
> being removed is still busy.
> 
> Since we don't yet have a way to tell exactly when the device is ready,
> pause for a short time and retry the action, letting
> devmapper_destroy_snapshot() continue and, for example, letting
> mount_device() fall back to the physical device.
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam at mendozajonas.com>

Merged as 89cde533

> ---
> In the future we'll enable udev_sync in Buildroot but this should help
> us fallback properly on existing systems. Using usleep() isn't perfect,
> however it's similar to what existing lvm tools & libraries do in the
> same situation, and allows us to fall back to the physical device
> quickly.
> 
>  discover/devmapper.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/discover/devmapper.c b/discover/devmapper.c
> index f7407b77..0a4d320e 100644
> --- a/discover/devmapper.c
> +++ b/discover/devmapper.c
> @@ -457,24 +457,32 @@ static int destroy_device(const char *dm_name)
>  {
>  	struct dm_task *task;
>  	uint32_t cookie;
> -	int rc = -1;
> +	int rc, retries = 0;
>  
> +retry:
>  	task = dm_task_create(DM_DEVICE_REMOVE);
>  	if (!task) {
> -		pb_log_fn("could not create dm_task\n");
> +		pb_debug_fn("could not create dm_task\n");
>  		return -1;
>  	}
>  
>  	if (!dm_task_set_name(task, dm_name)) {
> -		pb_log("No dm device named '%s'\n", dm_name);
> +		rc = dm_task_get_errno(task);
> +		pb_debug_fn("Couldn't set name for %s, %s (%d)\n",
> +				dm_name, strerror(rc), rc);
>  		goto out;
>  	}
>  
> -	if (!set_cookie(task, &cookie))
> +	if (!set_cookie(task, &cookie)) {
> +		rc = dm_task_get_errno(task);
> +		pb_debug_fn("set_cookie failed, %s (%d)\n", strerror(rc), rc);
>  		goto out;
> +	}
>  
>  	if (!dm_task_run(task)) {
> -		pb_log("Unable to remove device '%s'\n", dm_name);
> +		rc = dm_task_get_errno(task);
> +		pb_debug_fn("Unable to remove device '%s', %s (%d)\n",
> +				dm_name, strerror(rc), rc);
>  		goto out;
>  	}
>  
> @@ -485,6 +493,12 @@ static int destroy_device(const char *dm_name)
>  
>  out:
>  	dm_task_destroy(task);
> +	if (rc == EBUSY && retries < 5) {
> +		pb_log_fn("Device busy, retry %d..\n", retries);
> +		usleep(100000);
> +		retries++;
> +		goto retry;
> +	}
>  	return rc;
>  }
>