-
Notifications
You must be signed in to change notification settings - Fork 257
btrfs-progs: offline filesystem resize feature #1007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Changes from all commits
9c1d53d
9dd0e80
97c5435
b5460cd
4113e89
b57d93e
e1ed04d
05718fb
a40d9ca
be7f415
5378bb9
7a8c4e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,10 +38,12 @@ | |
#include "kernel-lib/sizes.h" | ||
#include "kernel-lib/list_sort.h" | ||
#include "kernel-lib/overflow.h" | ||
#include "kernel-shared/accessors.h" | ||
#include "kernel-shared/ctree.h" | ||
#include "kernel-shared/compression.h" | ||
#include "kernel-shared/volumes.h" | ||
#include "kernel-shared/disk-io.h" | ||
#include "kernel-shared/transaction.h" | ||
#include "common/defs.h" | ||
#include "common/internal.h" | ||
#include "common/messages.h" | ||
|
@@ -1287,9 +1289,241 @@ | |
"[kK] means KiB, which denotes 1KiB = 1024B, 1MiB = 1024KiB, etc.", | ||
"", | ||
OPTLINE("--enqueue", "wait if there's another exclusive operation running, otherwise continue"), | ||
OPTLINE("--offline", "resize an offline filesystem, only increases are allowed"), | ||
NULL | ||
}; | ||
|
||
static int check_offline_resize_args(const char *path, const char *amount, | ||
const struct btrfs_fs_info *fs_info, | ||
struct btrfs_device **device_ret, | ||
u64 *new_size_ret) | ||
{ | ||
int ret = 0; | ||
char amount_dup[BTRFS_VOL_NAME_MAX]; | ||
struct btrfs_device *device = NULL; | ||
struct btrfs_device *mindev = NULL; | ||
bool dev_found = false; | ||
u64 devid = 1; | ||
u64 mindevid = (u64)-1; | ||
char *devstr = NULL; | ||
struct stat stat_buf; | ||
char *sizestr = NULL; | ||
u64 new_size = 0, old_size = 0, diff = 0, device_size = 0; | ||
int mod = 0; | ||
|
||
if (check_mounted(path)) { | ||
error("%s must not be mounted to use --offline", path); | ||
return 1; | ||
} | ||
|
||
if (!fs_info->fs_devices->num_devices) { | ||
error("no devices found"); | ||
return 1; | ||
} | ||
|
||
ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%s", amount); | ||
if (strlen(amount) != ret) { | ||
error("newsize argument is too long"); | ||
return 1; | ||
} | ||
|
||
if (strcmp(amount, "cancel") == 0) { | ||
error("cannot cancel offline resize since the operation is synchronous"); | ||
return 1; | ||
} | ||
|
||
/* Parse device id. */ | ||
sizestr = amount_dup; | ||
devstr = strchr(sizestr, ':'); | ||
if (devstr) { | ||
sizestr = devstr + 1; | ||
*devstr = 0; | ||
devstr = amount_dup; | ||
|
||
errno = 0; | ||
devid = strtoull(devstr, NULL, 10); | ||
|
||
if (errno) { | ||
error("failed to parse devid %s: %m", devstr); | ||
return 1; | ||
} | ||
} | ||
|
||
/* Find device matching device id */ | ||
list_for_each_entry(device, &fs_info->fs_devices->devices, dev_list) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this list guaranteed to be in order? If it comes from one of the b-trees it will be. If it's in order, you won't need to scan for the minimum devid, you can just use the devid of the first item in the list |
||
if (device->devid < mindevid) { | ||
mindevid = device->devid; | ||
mindev = device; | ||
} | ||
if (device->devid == devid) { | ||
dev_found = true; | ||
break; | ||
} | ||
} | ||
|
||
if (devstr && !dev_found) { | ||
/* Devid specified but not found. */ | ||
error("cannot find devid: %lld", devid); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. devid is unsigned, this should be %llu. The -Wformat-signedness warning will diagnose this |
||
return 1; | ||
} else if (!devstr && devid == 1 && !dev_found) { | ||
/* | ||
* No device specified, assuming implicit 1 but it does not | ||
* exist. Use minimum device as fallback. | ||
*/ | ||
warning("no devid specified means devid 1 which does not exist, using\n" | ||
"\t lowest devid %llu as a fallback", | ||
mindevid); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guessing at what the user meant seems dangerous - I'd error out here. |
||
devid = mindevid; | ||
device = mindev; | ||
} | ||
if (!device) { | ||
error("unable to find device"); | ||
return 1; | ||
} | ||
*device_ret = device; | ||
old_size = device->total_bytes; | ||
|
||
if (strcmp(sizestr, "max") == 0) { | ||
if (path_is_block_device(device->name)) { | ||
ret = device_get_partition_size(device->name, &new_size); | ||
if (ret) | ||
new_size = 0; | ||
} else if (path_is_reg_file(device->name)) { | ||
ret = stat(device->name, &stat_buf); | ||
if (ret) { | ||
new_size = 0; | ||
} else { | ||
new_size = stat_buf.st_size; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit-picking, but the usual kernel style is that you wouldn't need curly brackets here |
||
} | ||
|
||
if (new_size == 0) { | ||
error("unable to get size for device: %s", | ||
device->name); | ||
return 1; | ||
} | ||
} else { | ||
if (sizestr[0] == '-') { | ||
error("offline resize does not support shrinking"); | ||
return 1; | ||
} else if (sizestr[0] == '+') { | ||
mod = 1; | ||
sizestr++; | ||
} | ||
ret = parse_u64_with_suffix(sizestr, &diff); | ||
if (ret < 0) { | ||
error("failed to parse size %s", sizestr); | ||
return 1; | ||
} | ||
|
||
/* For target sizes without +/- sign prefix (e.g. 1:150g) */ | ||
if (mod == 0) { | ||
new_size = diff; | ||
} else if (mod > 0) { | ||
|
||
if (diff > ULLONG_MAX - old_size) { | ||
error("increasing %s is out of range", | ||
pretty_size_mode(diff, UNITS_DEFAULT)); | ||
return 1; | ||
} | ||
new_size = old_size + diff; | ||
} | ||
} | ||
new_size = round_down(new_size, fs_info->sectorsize); | ||
if (new_size < old_size) { | ||
error("offline resize does not support shrinking"); | ||
return 1; | ||
} | ||
*new_size_ret = new_size; | ||
|
||
ret = device_get_partition_size(device->name, &device_size); | ||
if (ret) { | ||
error("unable to get size for device: %s", device->name); | ||
return -1; | ||
} | ||
if (path_is_block_device(device->name) && new_size > device_size) { | ||
error("unable to resize '%s': not enough free space", | ||
device->name); | ||
return 1; | ||
} | ||
|
||
if (new_size < 256 * SZ_1M) | ||
warning("the new size %lld (%s) is < 256MiB, this may be rejected by kernel", | ||
new_size, pretty_size_mode(new_size, UNITS_DEFAULT)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. %llu |
||
|
||
pr_verbose(LOG_DEFAULT, "Resize device id %lld from %s to %s\n", devid, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. %llu |
||
pretty_size_mode(old_size, UNITS_DEFAULT), | ||
pretty_size_mode(new_size, UNITS_DEFAULT)); | ||
return 0; | ||
} | ||
|
||
static int offline_resize(const char *path, const char *amount) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe just return bool, if you don't particularly care about the actual return value? It'd simplify some of the code. |
||
{ | ||
int ret = 0; | ||
struct btrfs_root *root; | ||
struct btrfs_fs_info *fs_info; | ||
struct btrfs_device *device; | ||
struct btrfs_super_block *super; | ||
struct btrfs_trans_handle *trans; | ||
u64 new_size; | ||
u64 old_total; | ||
u64 diff; | ||
char dev_name[BTRFS_VOL_NAME_MAX]; | ||
|
||
root = open_ctree(path, 0, OPEN_CTREE_WRITES | OPEN_CTREE_CHUNK_ROOT_ONLY); | ||
if (!root) { | ||
error("could not open file at %s\n" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might not even need to print an error here, it looks like open_ctree is already quite chatty. But if you did keep it I think you need to make it clear it works with block devices as well as files |
||
"offline resize works on a file containing a btrfs image.", | ||
path); | ||
return 1; | ||
} | ||
fs_info = root->fs_info; | ||
super = fs_info->super_copy; | ||
|
||
ret = check_offline_resize_args(path, amount, fs_info, &device, | ||
&new_size); | ||
if (ret) { | ||
ret = 1; | ||
goto close; | ||
} | ||
|
||
ret = snprintf(dev_name, BTRFS_VOL_NAME_MAX, "%s", device->name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sprintf with %s is a code smell - that's just a more complicated strcpy. strncpy is the particular function you're looking for here. |
||
if (strlen(device->name) != ret) { | ||
error("device name too long %s", device->name); | ||
ret = 1; | ||
goto close; | ||
} | ||
ret = 0; | ||
|
||
trans = btrfs_start_transaction(root, 1); | ||
if (IS_ERR(trans)) { | ||
ret = PTR_ERR(trans); | ||
errno = -ret; | ||
error_msg(ERROR_MSG_START_TRANS, "%m"); | ||
return ret; | ||
} | ||
old_total = btrfs_super_total_bytes(super); | ||
diff = round_down(new_size - device->total_bytes, fs_info->sectorsize); | ||
btrfs_set_super_total_bytes(super, round_down(old_total + diff, | ||
fs_info->sectorsize)); | ||
device->total_bytes = new_size; | ||
ret = btrfs_update_device(trans, device); | ||
if (ret) { | ||
btrfs_abort_transaction(trans, ret); | ||
goto close; | ||
} | ||
ret |= btrfs_commit_transaction(trans, root); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for |=, ret is guaranteed to be 0 here |
||
close: | ||
ret |= close_ctree(root); | ||
if (ret) | ||
return ret; | ||
|
||
if (path_is_reg_file(dev_name)) | ||
ret = truncate(dev_name, new_size); | ||
if (ret) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd move this check into the scope of the if on line 1520 |
||
error("failed to truncate %s", device->name); | ||
return ret; | ||
} | ||
|
||
static int check_resize_args(const char *amount, const char *path, u64 *devid_ret) | ||
{ | ||
struct btrfs_ioctl_fs_info_args fi_args; | ||
|
@@ -1449,6 +1683,7 @@ | |
u64 devid; | ||
int ret; | ||
bool enqueue = false; | ||
bool offline = false; | ||
bool cancel = false; | ||
|
||
/* | ||
|
@@ -1458,6 +1693,8 @@ | |
for (optind = 1; optind < argc; optind++) { | ||
if (strcmp(argv[optind], "--enqueue") == 0) { | ||
enqueue = true; | ||
} else if (strcmp(argv[optind], "--offline") == 0) { | ||
offline = true; | ||
} else if (strcmp(argv[optind], "--") == 0) { | ||
/* Separator: options -- non-options */ | ||
} else if (strncmp(argv[optind], "--", 2) == 0) { | ||
|
@@ -1472,6 +1709,12 @@ | |
if (check_argc_exact(argc - optind, 2)) | ||
return 1; | ||
|
||
if (offline && enqueue) { | ||
error("--enqueue is not compatible with --offline\n" | ||
"since offline resizing is synchronous"); | ||
return 1; | ||
} | ||
|
||
amount = argv[optind]; | ||
path = argv[optind + 1]; | ||
|
||
|
@@ -1481,6 +1724,9 @@ | |
return 1; | ||
} | ||
|
||
if (offline) | ||
return offline_resize(path, amount); | ||
|
||
cancel = (strcmp("cancel", amount) == 0); | ||
|
||
fd = btrfs_open_dir(path); | ||
|
@@ -1490,7 +1736,8 @@ | |
error( | ||
"resize works on mounted filesystems and accepts only\n" | ||
"directories as argument. Passing file containing a btrfs image\n" | ||
"would resize the underlying filesystem instead of the image.\n"); | ||
"would resize the underlying filesystem instead of the image.\n" | ||
"To resize a file containing a btrfs image please use the --offline flag.\n"); | ||
} | ||
return 1; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need to copy amount_dup - can't you use amount directly?