Skip to content

Commit 7f876e9

Browse files
committed
btrfs-progs: balance: add extra delay if converting with a missing device
[BUG] There is a reproducer that can trigger btrfs to flips RO: # mkfs.btrfs -f -mraid1 -draid1 /dev/sdd /dev/sde # mount /dev/sdd /mnt/btrfs # echo 1 > /sys/block/sde/device/delete # btrfs balance start -mconvert=dup -dconvert=single /mnt/btrfs ERROR: error during balancing '.': Input/output error There may be more info in syslog - try dmesg | tail Then btrfs will flip read-only with the following errors: btrfs: attempt to access beyond end of device sde: rw=6145, sector=21696, nr_sectors = 32 limit=0 btrfs: attempt to access beyond end of device sde: rw=6145, sector=21728, nr_sectors = 32 limit=0 btrfs: attempt to access beyond end of device sde: rw=6145, sector=21760, nr_sectors = 32 limit=0 BTRFS error (device sdd): bdev /dev/sde errs: wr 1, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device sdd): bdev /dev/sde errs: wr 2, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device sdd): bdev /dev/sde errs: wr 3, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device sdd): bdev /dev/sde errs: wr 3, rd 0, flush 1, corrupt 0, gen 0 btrfs: attempt to access beyond end of device sde: rw=145409, sector=128, nr_sectors = 8 limit=0 BTRFS warning (device sdd): lost super block write due to IO error on /dev/sde (-5) BTRFS error (device sdd): bdev /dev/sde errs: wr 4, rd 0, flush 1, corrupt 0, gen 0 btrfs: attempt to access beyond end of device sde: rw=14337, sector=131072, nr_sectors = 8 limit=0 BTRFS warning (device sdd): lost super block write due to IO error on /dev/sde (-5) BTRFS error (device sdd): bdev /dev/sde errs: wr 5, rd 0, flush 1, corrupt 0, gen 0 BTRFS error (device sdd): error writing primary super block to device 2 BTRFS info (device sdd): balance: start -dconvert=single -mconvert=dup -sconvert=dup BTRFS info (device sdd): relocating block group 1372585984 flags data|raid1 BTRFS error (device sdd): bdev /dev/sde errs: wr 5, rd 0, flush 2, corrupt 0, gen 0 BTRFS warning (device sdd): chunk 2446327808 missing 1 devices, max tolerance is 0 for writable mount BTRFS: error (device sdd) in write_all_supers:4044: errno=-5 IO failure (errors while submitting device barriers.) BTRFS info (device sdd state E): forced readonly BTRFS warning (device sdd state E): Skipping commit of aborted transaction. BTRFS error (device sdd state EA): Transaction aborted (error -5) BTRFS: error (device sdd state EA) in cleanup_transaction:2017: errno=-5 IO failure BTRFS info (device sdd state EA): balance: ended with status: -5 [CAUSE] The root cause is that, deleting devices using sysfs interface normally will trigger the shutdown callback for the fs. But btrfs doesn't handle that callback at all, thus it can not really know that device is no longer avaialble, thus btrfs will still try to do usual read/write on that device. This is fine if the user do nothing, as RAID1 can handle it properly. But if we try to convert to SINGLE/DUP, btrfs will still use that device to allocate new data/metadata chunks. And if a new metadata chunk is allocated to the removed device, all the write will be lost, and trigger the super block write/barrier errors above. [USER SPACE ENHANCEMENT] For now, add extra missing devices check at btrfs-balance command. If there is a missing devices, `btrfs balance` will add a 10 seconds delay and warn the possible dangerous. The root fix is to introduce a failing/removed device detection for btrfs, but that will be a pretty big feature and will take quite some time before landing it upstream. Reported-by: Jeff Siddall <[email protected]> Link: https://lore.kernel.org/linux-btrfs/[email protected]/ Signed-off-by: Qu Wenruo <[email protected]>
1 parent e162294 commit 7f876e9

File tree

1 file changed

+87
-0
lines changed

1 file changed

+87
-0
lines changed

Diff for: cmds/balance.c

+87
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,45 @@ static const char * const cmd_balance_start_usage[] = {
376376
NULL
377377
};
378378

379+
/*
380+
* Return 0 if no missing device found for the fs at @mnt.
381+
* Return >0 if there is any missing device for the fs at @mnt.
382+
* Return <0 if we hit other errors during the check.
383+
*/
384+
static int check_missing_devices(const char *mnt)
385+
{
386+
struct btrfs_ioctl_fs_info_args fs_info_arg;
387+
struct btrfs_ioctl_dev_info_args *dev_info_args = NULL;
388+
bool found_missing = false;
389+
int ret;
390+
391+
ret = get_fs_info(mnt, &fs_info_arg, &dev_info_args);
392+
if (ret < 0)
393+
return ret;
394+
395+
for (int i = 0; i < fs_info_arg.num_devices; i++) {
396+
struct btrfs_ioctl_dev_info_args *cur_dev_info;
397+
int fd;
398+
399+
cur_dev_info = (struct btrfs_ioctl_dev_info_args *)&dev_info_args[i];
400+
401+
/*
402+
* Kernel will report the device path even if we can no
403+
* longer access it anymore. So we have to manually check it.
404+
*/
405+
fd = open((char *)cur_dev_info->path, O_RDONLY);
406+
if (fd < 0) {
407+
found_missing = true;
408+
break;
409+
}
410+
close(fd);
411+
}
412+
free(dev_info_args);
413+
if (found_missing)
414+
return 1;
415+
return 0;
416+
}
417+
379418
static int cmd_balance_start(const struct cmd_struct *cmd,
380419
int argc, char **argv)
381420
{
@@ -387,6 +426,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
387426
bool enqueue = false;
388427
unsigned start_flags = 0;
389428
bool raid56_warned = false;
429+
bool convert_warned = false;
390430
int i;
391431

392432
memset(&args, 0, sizeof(args));
@@ -481,6 +521,53 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
481521
args.flags |= BTRFS_BALANCE_TYPE_MASK;
482522
}
483523

524+
/*
525+
* If we are using convert, and there is a missing/failed device at
526+
* runtime (e.g. mounted then remove a device using sysfs interface),
527+
* btrfs has no way to avoid using that failing/removed device.
528+
*
529+
* In that case converting the profile is very dangerous, e.g.
530+
* converting RAID1 to SINGLE/DUP, and new SINGLE/DUP chunks can
531+
* be allocated to that failing/removed device, and cause the
532+
* fs to flip RO due to failed metadata writes.
533+
*
534+
* Meanwhile the fs may work completely fine due to the extra
535+
* duplication (e.g. all RAID1 profiles).
536+
*
537+
* So here warn if one is trying to convert with missing devices.
538+
*/
539+
for (i = 0; ptrs[i]; i++) {
540+
int delay = 10;
541+
int ret;
542+
543+
if (!(ptrs[i]->flags & BTRFS_BALANCE_ARGS_CONVERT) || convert_warned)
544+
continue;
545+
546+
ret = check_missing_devices(argv[optind]);
547+
if (ret < 0) {
548+
errno = -ret;
549+
warning("skipping missing devices check due to failure: %m");
550+
break;
551+
}
552+
if (ret == 0)
553+
continue;
554+
convert_warned = true;
555+
printf("WARNING:\n\n");
556+
printf("\tConversion with missing device(s) can be dangerous.\n");
557+
printf("\tPlease use `btrfs replace` or `btrfs device remove` instead.\n");
558+
if (force) {
559+
printf("\tSafety timeout skipped due to --force\n\n");
560+
continue;
561+
}
562+
printf("\tThe operation will continue in %d seconds.\n", delay);
563+
printf("\tUse Ctrl-C to stop.\n");
564+
while (delay) {
565+
printf("%2d", delay--);
566+
fflush(stdout);
567+
sleep(1);
568+
}
569+
}
570+
484571
/* drange makes sense only when devid is set */
485572
for (i = 0; ptrs[i]; i++) {
486573
if ((ptrs[i]->flags & BTRFS_BALANCE_ARGS_DRANGE) &&

0 commit comments

Comments
 (0)