Re: [PATCH 4/4] btrfs-progs: Enqueue command if it can't be performed immediately

[David Sterba <dsterba@suse.cz>]

On Tue, Aug 25, 2020 at 10:03:38AM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> Wait for the current exclusive operation to finish before issuing the
> command ioctl, so we have a better chance of success.
> 
> Q: The resize argument parsing is hackish. Is there a better way to do
> this?

You mean parsing in kernel? Progs pass the 1st non-option parameter
without changes, so if you add a new option, the -- separator needs to
be used to make sure the relative size update (eg. -1G) is properly
recognized. This is built in already and should not require anything
special on the option parsing side.

> --- a/cmds/device.c
> +++ b/cmds/device.c
> @@ -49,6 +49,7 @@ static const char * const cmd_device_add_usage[] = {
>  	"",
>  	"-K|--nodiscard    do not perform whole device TRIM on devices that report such capability",
>  	"-f|--force        force overwrite existing filesystem on the disk",
> +	"-q|--enqueue	   enqueue if an exclusive operation is running",

Short for -q should not be used due to confusion with --quiet. Also I
think that --enqueue is not a common action that would need a short
option, the long option is always safe.

> --- a/common/sysfs.c
> +++ b/common/sysfs.c
> @@ -50,3 +50,29 @@ int get_exclusive_operation(int mp_fd, char *val)
>  	val[n - 1] = '\0';
>  	return n;
>  }
> +
> +int sysfs_wait(int fd, int seconds)
> +{
> +	fd_set fds;
> +	struct timeval tv;
> +
> +	FD_ZERO(&fds);
> +	FD_SET(fd, &fds);
> +
> +	tv.tv_sec = seconds;
> +	tv.tv_usec = 0;
> +
> +	return select(fd+1, NULL, NULL, &fds, &tv);

With the short sleep times, do we need to wait using select? Yes this
would return once the notification event is sent but as the sleep time
is 1 second, it could simply be sleep(1) unconditionally.

> +}
> +
> +void wait_for_exclusive_operation(int dirfd)
> +{
> +        char exop[BTRFS_SYSFS_EXOP_SIZE];
> +	int fd;
> +
> +        fd = sysfs_open(dirfd, "exclusive_operation");
> +        while ((sysfs_get_str_fd(fd, exop, BTRFS_SYSFS_EXOP_SIZE) > 0) &&
> +		strncmp(exop, "none", 4))
> +			sysfs_wait(fd, 1);
> +	close(fd);
> +}
> diff --git a/common/utils.h b/common/utils.h
> index be8aab58..f141edb6 100644
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -155,5 +155,6 @@ int btrfs_warn_multiple_profiles(int fd);
>  
>  #define BTRFS_SYSFS_EXOP_SIZE		16
>  int get_exclusive_operation(int fd, char *val);
> +void wait_for_exclusive_operation(int fd);
>  
>  #endif
> -- 
> 2.26.2