Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command

[Ming Lei <ming.lei@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5559 bytes --]

On Tue, Jan 06, 2026 at 10:33:33PM +0200, Yoav Cohen wrote:
> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
> device if there are no active openers for the ublk block device.
> If the device is busy, the command returns -EBUSY instead of
> disrupting active clients. This allows safe, non-destructive stopping.
> 
> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
> feature flag.
> 
> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> ---
>  drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
>  include/uapi/linux/ublk_cmd.h        |  9 +++++-
>  tools/testing/selftests/ublk/kublk.c |  1 +
>  3 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 2d5602ef05cc..fc8b87902f8f 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -54,6 +54,7 @@
>  #define UBLK_CMD_DEL_DEV_ASYNC	_IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
>  #define UBLK_CMD_UPDATE_SIZE	_IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
>  #define UBLK_CMD_QUIESCE_DEV	_IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
> +#define UBLK_CMD_TRY_STOP_DEV	_IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
>  
>  #define UBLK_IO_REGISTER_IO_BUF		_IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
>  #define UBLK_IO_UNREGISTER_IO_BUF	_IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
> @@ -73,7 +74,8 @@
>  		| UBLK_F_AUTO_BUF_REG \
>  		| UBLK_F_QUIESCE \
>  		| UBLK_F_PER_IO_DAEMON \
> -		| UBLK_F_BUF_REG_OFF_DAEMON)
> +		| UBLK_F_BUF_REG_OFF_DAEMON \
> +		| UBLK_F_SAFE_STOP_DEV)
>  
>  #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>  		| UBLK_F_USER_RECOVERY_REISSUE \
> @@ -239,6 +241,8 @@ struct ublk_device {
>  	struct delayed_work	exit_work;
>  	struct work_struct	partition_scan_work;
>  
> +	bool			block_open; /* protected by open_mutex */
> +
>  	struct ublk_queue       *queues[];
>  };
>  
> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
>  			return -EPERM;
>  	}
>  
> +	if (ub->block_open)
> +		return -EBUSY;
> +
>  	return 0;
>  }
>  
> @@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>  	ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
>  		UBLK_F_URING_CMD_COMP_IN_TASK |
>  		UBLK_F_PER_IO_DAEMON |
> -		UBLK_F_BUF_REG_OFF_DAEMON;
> +		UBLK_F_BUF_REG_OFF_DAEMON |
> +		UBLK_F_SAFE_STOP_DEV;
>  
>  	/* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
>  	if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
> @@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
>  	ublk_stop_dev(ub);
>  }
>  
> +static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
> +{
> +	struct gendisk *disk;
> +	int ret = 0;
> +
> +	disk = ublk_get_disk(ub);
> +	if (!disk)
> +		return -ENODEV;
> +
> +	mutex_lock(&disk->open_mutex);
> +	if (disk_openers(disk) > 0) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +	ub->block_open = true;
> +	/* release open_mutex as del_gendisk() will reacquire it */
> +	mutex_unlock(&disk->open_mutex);
> +
> +	ublk_ctrl_stop_dev(ub);
> +	goto out;
> +
> +unlock:
> +	mutex_unlock(&disk->open_mutex);
> +out:
> +	ublk_put_disk(disk);
> +	return ret;
> +}
> +
>  static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
>  		const struct ublksrv_ctrl_cmd *header)
>  {
> @@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
>  	case UBLK_CMD_END_USER_RECOVERY:
>  	case UBLK_CMD_UPDATE_SIZE:
>  	case UBLK_CMD_QUIESCE_DEV:
> +	case UBLK_CMD_TRY_STOP_DEV:
>  		mask = MAY_READ | MAY_WRITE;
>  		break;
>  	default:
> @@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
>  	case UBLK_CMD_QUIESCE_DEV:
>  		ret = ublk_ctrl_quiesce_dev(ub, header);
>  		break;
> +	case UBLK_CMD_TRY_STOP_DEV:
> +		ret = ublk_ctrl_try_stop_dev(ub);
> +		break;
>  	default:
>  		ret = -EOPNOTSUPP;
>  		break;
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index ec77dabba45b..9daa8ab372f0 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -55,7 +55,8 @@
>  	_IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
>  #define UBLK_U_CMD_QUIESCE_DEV		\
>  	_IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
> -
> +#define UBLK_U_CMD_TRY_STOP_DEV		\
> +	_IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
>  /*
>   * 64bits are enough now, and it should be easy to extend in case of
>   * running out of feature flags
> @@ -311,6 +312,12 @@
>   */
>  #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
>  
> +/*
> + * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
> + * allows stopping the device only if there are no openers.
> + */
> +#define UBLK_F_SAFE_STOP_DEV	(1ULL << 17)
> +
>  /* device state */
>  #define UBLK_S_DEV_DEAD	0
>  #define UBLK_S_DEV_LIVE	1
> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
> index 185ba553686a..6739e28c4059 100644
> --- a/tools/testing/selftests/ublk/kublk.c
> +++ b/tools/testing/selftests/ublk/kublk.c
> @@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
>  		FEAT_NAME(UBLK_F_QUIESCE),
>  		FEAT_NAME(UBLK_F_PER_IO_DAEMON),
>  		FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
> +		FEAT_NAME(UBLK_F_SAFE_STOP_DEV),

We usually split driver and test code into different patches, and the patch
looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

But we usually separate test code from driver change, also new feature needs
selftest code for verifying, can you move `+FEAT_NAME(UBLK_F_SAFE_STOP_DEV)` into
the attached selftest patch and post all 3 in v5?



Thanks,
Ming

[-- Attachment #2: 0001-selftests-ublk-add-stop-command-with-safe-option.patch --]
[-- Type: text/plain, Size: 6230 bytes --]

From 3574f73d3098dc56f63b7c2b1097c0f090c4ad3c Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Wed, 7 Jan 2026 13:01:39 +0000
Subject: [PATCH] selftests: ublk: add stop command with --safe option

Add 'stop' subcommand to kublk utility that uses the new
UBLK_CMD_TRY_STOP_DEV command when --safe option is specified.
This allows stopping a device only if it has no active openers,
returning -EBUSY otherwise.

Also add test_generic_16.sh to test the new functionality.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 tools/testing/selftests/ublk/Makefile         |  1 +
 tools/testing/selftests/ublk/kublk.c          | 52 +++++++++++++++++
 tools/testing/selftests/ublk/kublk.h          |  1 +
 .../testing/selftests/ublk/test_generic_16.sh | 57 +++++++++++++++++++
 4 files changed, 111 insertions(+)
 create mode 100755 tools/testing/selftests/ublk/test_generic_16.sh

diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index 06ba6fde098d..e9614f77b809 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -23,6 +23,7 @@ TEST_PROGS += test_generic_12.sh
 TEST_PROGS += test_generic_13.sh
 TEST_PROGS += test_generic_14.sh
 TEST_PROGS += test_generic_15.sh
+TEST_PROGS += test_generic_16.sh
 
 TEST_PROGS += test_null_01.sh
 TEST_PROGS += test_null_02.sh
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 6739e28c4059..1a563705caa0 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -107,6 +107,15 @@ static int ublk_ctrl_stop_dev(struct ublk_dev *dev)
 	return __ublk_ctrl_cmd(dev, &data);
 }
 
+static int ublk_ctrl_try_stop_dev(struct ublk_dev *dev)
+{
+	struct ublk_ctrl_cmd_data data = {
+		.cmd_op	= UBLK_U_CMD_TRY_STOP_DEV,
+	};
+
+	return __ublk_ctrl_cmd(dev, &data);
+}
+
 static int ublk_ctrl_start_dev(struct ublk_dev *dev,
 		int daemon_pid)
 {
@@ -1392,6 +1401,42 @@ static int cmd_dev_del(struct dev_ctx *ctx)
 	return 0;
 }
 
+static int cmd_dev_stop(struct dev_ctx *ctx)
+{
+	int number = ctx->dev_id;
+	struct ublk_dev *dev;
+	int ret;
+
+	if (number < 0) {
+		ublk_err("%s: device id is required\n", __func__);
+		return -EINVAL;
+	}
+
+	dev = ublk_ctrl_init();
+	dev->dev_info.dev_id = number;
+
+	ret = ublk_ctrl_get_info(dev);
+	if (ret < 0)
+		goto fail;
+
+	if (ctx->safe_stop) {
+		ret = ublk_ctrl_try_stop_dev(dev);
+		if (ret < 0)
+			ublk_err("%s: try_stop dev %d failed ret %d\n",
+					__func__, number, ret);
+	} else {
+		ret = ublk_ctrl_stop_dev(dev);
+		if (ret < 0)
+			ublk_err("%s: stop dev %d failed ret %d\n",
+					__func__, number, ret);
+	}
+
+fail:
+	ublk_ctrl_deinit(dev);
+
+	return ret;
+}
+
 static int __cmd_dev_list(struct dev_ctx *ctx)
 {
 	struct ublk_dev *dev = ublk_ctrl_init();
@@ -1582,6 +1627,8 @@ static int cmd_dev_help(char *exe)
 
 	printf("%s del [-n dev_id] -a \n", exe);
 	printf("\t -a delete all devices -n delete specified device\n\n");
+	printf("%s stop -n dev_id [--safe]\n", exe);
+	printf("\t --safe only stop if device has no active openers\n\n");
 	printf("%s list [-n dev_id] -a \n", exe);
 	printf("\t -a list all devices, -n list specified device, default -a \n\n");
 	printf("%s features\n", exe);
@@ -1613,6 +1660,7 @@ int main(int argc, char *argv[])
 		{ "nthreads",		1,	NULL,  0 },
 		{ "per_io_tasks",	0,	NULL,  0 },
 		{ "no_ublk_fixed_fd",	0,	NULL,  0 },
+		{ "safe",		0,	NULL,  0 },
 		{ 0, 0, 0, 0 }
 	};
 	const struct ublk_tgt_ops *ops = NULL;
@@ -1697,6 +1745,8 @@ int main(int argc, char *argv[])
 				ctx.per_io_tasks = 1;
 			if (!strcmp(longopts[option_idx].name, "no_ublk_fixed_fd"))
 				ctx.no_ublk_fixed_fd = 1;
+			if (!strcmp(longopts[option_idx].name, "safe"))
+				ctx.safe_stop = 1;
 			break;
 		case '?':
 			/*
@@ -1764,6 +1814,8 @@ int main(int argc, char *argv[])
 		}
 	} else if (!strcmp(cmd, "del"))
 		ret = cmd_dev_del(&ctx);
+	else if (!strcmp(cmd, "stop"))
+		ret = cmd_dev_stop(&ctx);
 	else if (!strcmp(cmd, "list")) {
 		ctx.all = 1;
 		ret = cmd_dev_list(&ctx);
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 8a83b90ec603..4b6b4992f78e 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -78,6 +78,7 @@ struct dev_ctx {
 	unsigned int	auto_zc_fallback:1;
 	unsigned int	per_io_tasks:1;
 	unsigned int	no_ublk_fixed_fd:1;
+	unsigned int	safe_stop:1;
 
 	int _evtfd;
 	int _shmid;
diff --git a/tools/testing/selftests/ublk/test_generic_16.sh b/tools/testing/selftests/ublk/test_generic_16.sh
new file mode 100755
index 000000000000..e08af7b685c9
--- /dev/null
+++ b/tools/testing/selftests/ublk/test_generic_16.sh
@@ -0,0 +1,57 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
+
+TID="generic_16"
+ERR_CODE=0
+
+_prep_test "null" "stop --safe command"
+
+# Check if SAFE_STOP_DEV feature is supported
+if ! _have_feature "SAFE_STOP_DEV"; then
+	_cleanup_test "null"
+	exit "$UBLK_SKIP_CODE"
+fi
+
+# Test 1: stop --safe on idle device should succeed
+dev_id=$(_add_ublk_dev -t null -q 2 -d 32)
+_check_add_dev $TID $?
+
+# Device is idle (no openers), stop --safe should succeed
+if ! ${UBLK_PROG} stop -n "${dev_id}" --safe; then
+	echo "stop --safe on idle device failed unexpectedly!"
+	ERR_CODE=255
+fi
+
+# Clean up device
+${UBLK_PROG} del -n "${dev_id}" > /dev/null 2>&1
+udevadm settle
+
+# Test 2: stop --safe on device with active opener should fail
+dev_id=$(_add_ublk_dev -t null -q 2 -d 32)
+_check_add_dev $TID $?
+
+# Open device in background (dd reads indefinitely)
+dd if=/dev/ublkb${dev_id} of=/dev/null bs=4k iflag=direct > /dev/null 2>&1 &
+dd_pid=$!
+
+# Give dd time to start
+sleep 0.2
+
+# Device has active opener, stop --safe should fail with -EBUSY
+if ${UBLK_PROG} stop -n "${dev_id}" --safe 2>/dev/null; then
+	echo "stop --safe on busy device succeeded unexpectedly!"
+	ERR_CODE=255
+fi
+
+# Kill dd and clean up
+kill $dd_pid 2>/dev/null
+wait $dd_pid 2>/dev/null
+
+# Now device should be idle, regular delete should work
+${UBLK_PROG} del -n "${dev_id}"
+udevadm settle
+
+_cleanup_test "null"
+_show_result $TID $ERR_CODE
-- 
2.47.1