| Message ID | 20210628194000.org5zuvytk34yvwy@fiona (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: Correct check_running_fs_exclop() return value | expand |
On Mon, Jun 28, 2021 at 02:40:00PM -0500, Goldwyn Rodrigues wrote: > check_running_fs_exclop() can return 1 when exclop is changed to "none" > The ret is set by the return value of the select() operation. Checking > the exclusive op changes just the exclop variable while ret is still > set to 1. > > Set ret = 0 if exclop is set to BTRFS_EXCL_NONE or BTRFS_EXCL_UNKNOWN. > Remove unnecessary continue statement at the end of the block. That's describing what the code does in words, but there must be some user visible effects like failed command. Do you have some reproducer? I've applied patch as it's a fix but would still like to update the changelog.
On 21:21 01/07, David Sterba wrote: > On Mon, Jun 28, 2021 at 02:40:00PM -0500, Goldwyn Rodrigues wrote: > > check_running_fs_exclop() can return 1 when exclop is changed to "none" > > The ret is set by the return value of the select() operation. Checking > > the exclusive op changes just the exclop variable while ret is still > > set to 1. > > > > Set ret = 0 if exclop is set to BTRFS_EXCL_NONE or BTRFS_EXCL_UNKNOWN. > > Remove unnecessary continue statement at the end of the block. > > That's describing what the code does in words, but there must be some > user visible effects like failed command. Do you have some reproducer? > > I've applied patch as it's a fix but would still like to update the > changelog. If check_running_fs_exclop() returns anything but zero, it would exit immediately and the ioctl, for which the program is called, is not executed, without any error notice. IOW, the command appears to have executed, but does not. This was found when balance which typically reports chunks relocated did not print anything on screen.
diff --git a/common/utils.c b/common/utils.c index 1627913a..3c562247 100644 --- a/common/utils.c +++ b/common/utils.c @@ -1771,7 +1771,8 @@ int check_running_fs_exclop(int fd, enum exclusive_operation start, bool enqueue tv.tv_sec /= 2; ret = select(sysfs_fd + 1, NULL, NULL, &fds, &tv); exclop = get_fs_exclop(fd); - continue; + if (exclop <= 0) + ret = 0; } } out:
check_running_fs_exclop() can return 1 when exclop is changed to "none" The ret is set by the return value of the select() operation. Checking the exclusive op changes just the exclop variable while ret is still set to 1. Set ret = 0 if exclop is set to BTRFS_EXCL_NONE or BTRFS_EXCL_UNKNOWN. Remove unnecessary continue statement at the end of the block. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> --- common/utils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)