Re: [PATCH] btrfs-prog: scrub: print message when scrubbing a read-only filesystem without r option

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/7/4 03:17, Li Zhang 写道:
> issue:666

It's better to move the issue tag before your SOB line.

And with something like "Issue: #829" so that github can detect it.

Check commit ab2260355afb ("btrfs-progs: subvol list: fix accidental
trimming of subvolume name") for a proper example.

>
> [enhancement]
> When scrubbing a filesystem mounted read-only and without r
> option, it aborts and there is no message associated with it.
> So we need to print an error message when scrubbing a read-only
> filesystem to tell the user what is going on here.
>
> [implementation]
> Move the error message from the main thread to each scrub thread,
> previously the message was printed after all scrub threads
> finished and without background mode.
>
> [test]
> Mount dev in read-only mode, then scrub it without r option
>
> $ sudo mount /dev/vdb -o ro /mnt/
> $ sudo btrfs scrub start  /mnt/
> scrub started on /mnt/, fsid f7c26803-a68d-4a96-91c4-a629b57b7f64 (pid=2892)
> Starting scrub on devid 1
> Starting scrub on devid 2
> Starting scrub on devid 3
> scrub on devid 1 failed ret=-1 errno=30 (Read-only file system)
> scrub on devid 2 failed ret=-1 errno=30 (Read-only file system)
> scrub on devid 3 failed ret=-1 errno=30 (Read-only file system)
>
> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> ---
>   cmds/scrub.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/cmds/scrub.c b/cmds/scrub.c
> index d54e11f..e0400dd 100644
> --- a/cmds/scrub.c
> +++ b/cmds/scrub.c
> @@ -957,7 +957,10 @@ static void *scrub_one_dev(void *ctx)
>   		warning("setting ioprio failed: %m (ignored)");
>
>   	ret = ioctl(sp->fd, BTRFS_IOC_SCRUB, &sp->scrub_args);
> -        pr_verbose(LOG_DEFAULT, "scrub ioctl devid:%llu ret:%d errno:%d\n",sp->scrub_args.devid, ret, errno);
> +	if (ret){
> +		pr_verbose(LOG_DEFAULT, "scrub on devid %llu failed ret=%d errno=%d (%m)\n",
> +			sp->scrub_args.devid, ret, errno);
> +	}

Doing direct output inside a thread can lead to race of the output.

And we do not know if the scrub ioctl would return error early or run
for a long long time, that's the dilemma.

I'm wondering if it's possible to do a blkid/mountinfo based probe.
Then it would avoid the possible output race and error out earilier.

Thanks,
Qu

>   	gettimeofday(&tv, NULL);
>   	sp->ret = ret;
>   	sp->stats.duration = tv.tv_sec - sp->stats.t_start;
> @@ -1596,13 +1599,6 @@ static int scrub_start(const struct cmd_struct *cmd, int argc, char **argv,
>   				++err;
>   				break;
>   			default:
> -				if (do_print) {
> -					errno = sp[i].ioctl_errno;
> -					error(
> -		"scrubbing %s failed for device id %lld: ret=%d, errno=%d (%m)",
> -						path, devid, sp[i].ret,
> -						sp[i].ioctl_errno);
> -				}
>   				++err;
>   				continue;
>   			}