All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Sebastian Chlad <sebastianchlad@gmail.com>
Cc: linux-block@vger.kernel.org,  Sebastian Chlad <sebastian.chlad@suse.com>
Subject: Re: [PATCH 1/2 blktests] src/miniublk: switch to ioctl-encoded ublk commands
Date: Fri, 19 Jun 2026 12:26:11 +0900	[thread overview]
Message-ID: <ajSud4Y4PmCu2X_5@shinmob> (raw)
In-Reply-To: <20260617072516.6238-2-sebastian.chlad@suse.com>

Hi Sebastian,

Thanks for the patches. I agree that this direction is good: it's the better
shift away from the legacy interface.

One point I noticed is that src/miniublk.c can no longer be built with the
kernel headers of the LTS kernel version v6.1.y, probably (v5.15.y does not have
ublk and v6.6.y supports the new interface). This is a rather small window, and
may be acceptable but I wonder what you think about it

If we drop the miniublk build with v6.1.y kernel headers, it might be the better
to check before building miniublk. I quickly created a Makefile change [1] for
that purpose.

Also, please find a comment in line below.

On Jun 17, 2026 / 09:25, Sebastian Chlad wrote:
> Kernels built without CONFIG_BLKDEV_UBLK_LEGACY_OPCODES reject the
> legacy raw UBLK_CMD_* and UBLK_IO_* opcodes. Switch miniublk to use
> the ioctl-encoded UBLK_U_CMD_* and UBLK_U_IO_* variants defined in
> linux/ublk_cmd.h instead.
> 
> For IO commands, the ioctl-encoded opcode is used for submission while
> _IOC_NR() extracts the raw NR bits for build_user_data(), keeping the
> user_data tag encoding intact.
> 
> Signed-off-by: Sebastian Chlad <sebastian.chlad@suse.com>
> ---
>  src/miniublk.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/src/miniublk.c b/src/miniublk.c
> index f98f850..5a35ca7 100644
> --- a/src/miniublk.c
> +++ b/src/miniublk.c
[...]
> @@ -624,9 +624,9 @@ static int ublk_queue_io_cmd(struct ublk_queue *q,
>  		return 0;
>  
>  	if (io->flags & UBLKSRV_NEED_COMMIT_RQ_COMP)
> -		cmd_op = UBLK_IO_COMMIT_AND_FETCH_REQ;
> -	else if (io->flags & UBLKSRV_NEED_FETCH_RQ)
> -		cmd_op = UBLK_IO_FETCH_REQ;
> +		cmd_op = UBLK_U_IO_COMMIT_AND_FETCH_REQ;
> +	else
> +		cmd_op = UBLK_U_IO_FETCH_REQ;

The hunk above changes the "else if" part, is this intentional?


[1]

diff --git a/src/Makefile b/src/Makefile
index d8833bf..adfe3ef 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -8,6 +8,10 @@ HAVE_C_MACRO = $(shell if echo "$(H)include <$(1)>" |	\
 		$(CC) $(CFLAGS) -E - 2>&1 /dev/null | grep $(2) > /dev/null 2>&1; \
 		then echo 1;else echo 0; fi)
 
+HAVE_C_DEF = $(shell if echo -e "$(H)include <$(1)>\n#ifdef $(2)\nHAVE_$(2)\n#endif" | \
+		$(CC) $(CFLAGS) -E - 2>&1 /dev/null | grep HAVE_$(2) > /dev/null 2>&1; \
+		then echo 1;else echo 0; fi)
+
 C_TARGETS := \
 	dio-offsets \
 	loblksize \
@@ -27,6 +31,7 @@ C_UBLK_TARGETS := miniublk
 
 HAVE_LIBURING := $(call HAVE_C_MACRO,liburing.h,IORING_OP_URING_CMD)
 HAVE_UBLK_HEADER := $(call HAVE_C_HEADER,linux/ublk_cmd.h,1)
+HAVE_NEW_UBLK_INTF := $(call HAVE_C_DEF,linux/ublk_cmd.h,UBLK_U_CMD_START_DEV)
 
 CXX_TARGETS := \
 	discontiguous-io
@@ -37,8 +42,12 @@ SYZKALLER_TARGETS := \
 TARGETS := $(C_TARGETS) $(CXX_TARGETS) $(SYZKALLER_TARGETS)
 
 ifeq ($(HAVE_UBLK_HEADER), 1)
+ifeq ($(HAVE_NEW_UBLK_INTF), 1)
 C_URING_TARGETS += $(C_UBLK_TARGETS)
 else
+$(info Skip $(C_UBLK_TARGETS) build due to missing new ublk interface(v6.4+))
+endif
+else
 $(info Skip $(C_UBLK_TARGETS) build due to missing kernel header(v6.0+))
 endif
 


  reply	other threads:[~2026-06-19  3:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-17  7:25 [PATCH 0/2 blktests] Update the miniublk to use ioctl opcodes Sebastian Chlad
2026-06-17  7:25 ` [PATCH 1/2 blktests] src/miniublk: switch to ioctl-encoded ublk commands Sebastian Chlad
2026-06-19  3:26   ` Shin'ichiro Kawasaki [this message]
2026-06-22 13:34     ` Sebastian Chlad
2026-06-22 22:21       ` Shin'ichiro Kawasaki
2026-06-17  7:25 ` [PATCH 2/2 blktests] src/miniublk: fall back to legacy opcodes on older kernels Sebastian Chlad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajSud4Y4PmCu2X_5@shinmob \
    --to=shinichiro.kawasaki@wdc.com \
    --cc=linux-block@vger.kernel.org \
    --cc=sebastian.chlad@suse.com \
    --cc=sebastianchlad@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.