Re: [PATCH ubdsrv] tgt_null: Return number of sectors read/written

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

On Tue, Jun 21, 2022 at 06:48:39PM -0400, Gabriel Krisman Bertazi wrote:
> Hi Ming,
> 
> I wrote this against your devel-v3 branch.  I'm wondering if you plan to
> send a new version of the kernel patch soon? From the latest

Yeah, that is on my todo list:

https://github.com/ming1/linux/commits/my_for-5.19-ubd-devel_v3

there has lots cleanup & improvement.

> discussions, I don't think there were major issues found on review. :)

One problem is the driver name, and Christoph thought we have
'arch/um/drivers/ubd*.c'. Not thought of one good candidate yet.

> 
> I hope people don't mind I cc'd linux-block about this userspace code.
> Please, let me know if I shouldn't do that.
> 
> -- >8 --
> 
> The number of sectors read/written is used to verify forward progress of
> the request inside the kernel.  If we return 0 here, the kernel
> understands that as an IO failure (see first check in ubd_complete_rq),
> and will reissue the request, causing an infinite loop of unfullfilled
> requests.  This can be reproduced with:
> 
>   ubdsrv/ubd add -t null -n0 -q1 -d1
>   dd if=/dev/vda of=/dev/ubdb0 count=1 bs=4k
> 
> The approach minics nullblk, which returns the total IO size.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> ---
>  tgt_null.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tgt_null.c b/tgt_null.c
> index 85636c405f0c..61850a2cd046 100644
> --- a/tgt_null.c
> +++ b/tgt_null.c
> @@ -20,7 +20,9 @@ static int null_init_tgt(struct ubdsrv_tgt_info *tgt, int type, int argc,
>  static int null_handle_io_async(struct ubdsrv_queue *q, struct ubd_io *io,
>  		int tag)
>  {
> -	ubdsrv_mark_io_done(io, 0);
> +	const struct ubdsrv_io_desc *iod = ubdsrv_get_iod(q, tag);
> +
> +	ubdsrv_mark_io_done(io, iod->nr_sectors << 9);

This issue was actually fixed in master branch, and there are actually lots
of change in master:

- switch to liburing
- switch to c++, and use c++20 coroutine for ->handle_io_async()
- all kinds of cleanup

I am actually working on ubd-qcow2 with above, but that may take a while and
the code isn't posted yet. The motivation is that the whole framework can
get verified well enough with one complicated target implementation.

Thanks,
Ming