On 05/23/2012 01:21 PM, Goffredo Baroncelli wrote:

> Hi
> 
> On 05/17/2012 02:08 PM, Liu Bo wrote:
>> This patch fixes two bugs:
>>
>> When we do not assigne a device id for the resizer,
>> - it will only take one device to resize, which is supposed to apply on
>>   all available devices.
>> - it will take 'id 1' device as default, and this will cause a bug as we
>>   may have removed the 'id 1' device from the filesystem.
>>
>> After this patch, we can find all available devices by searching the
>> chunk tree and resize them:
> 
> 
> I am not sure that this is a sane default for all resizing.
> 
> If the user want to resize to MAX, I agree that it is a sane default,
> but when the user want to shrink or enlarge of a fixed quantity, the
> user should specific the dev id. Because the shrinking and or the
> enlarging should be evaluated case by case.
> 
> My suggestion is to change the code at kernel level so in case of
> multi-volume file-system the user *has* to specify the device to shrink
> and/or enlarge.
> Should be the user space btrfs tool to handle the check and the growing
> (i.e: if the new size is max, automatically grow all the device up to
> max; otherwise the user should specific the device to shrink and/or
> enlarge).
> 


Hi,

It is quite easy to do what you expect, but IMO that will confuse users a lot...

With this patch, you can still assign a specific dev id to resize what you want 
:)

thanks,
liubo

> BR
> Goffredo
> 
> 
>> $ mkfs.btrfs /dev/sdb7
>> $ mount /dev/sdb7 /mnt/btrfs/
>> $ btrfs dev add /dev/sdb8 /mnt/btrfs/
>>
>> $ btrfs fi resize -100m /mnt/btrfs/
>> then we can get from dmesg:
>> btrfs: new size for /dev/sdb7 is 980844544
>> btrfs: new size for /dev/sdb8 is 980844544
>>
>> $ btrfs fi resize max /mnt/btrfs
>> then we can get from dmesg:
>> btrfs: new size for /dev/sdb7 is 1085702144
>> btrfs: new size for /dev/sdb8 is 1085702144
>>
>> $ btrfs fi resize 1:-100m /mnt/btrfs
>> then we can get from dmesg:
>> btrfs: resizing devid 1
>> btrfs: new size for /dev/sdb7 is 980844544
>>
>> $ btrfs fi resize 1:-100m /mnt/btrfs
>> then we can get from dmesg:
>> btrfs: resizing devid 2
>> btrfs: new size for /dev/sdb8 is 980844544
>>
>> Signed-off-by: Liu Bo <liubo2...@cn.fujitsu.com>
>> ---
>>  fs/btrfs/ioctl.c |  101 
>> ++++++++++++++++++++++++++++++++++++++++++++----------
>>  1 files changed, 83 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index ec2245d..d9a4fa8 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -1250,12 +1250,51 @@ out_ra:
>>      return ret;
>>  }
>>  
>> +static struct btrfs_device *get_avail_device(struct btrfs_root *root, u64 
>> devid)
>> +{
>> +    struct btrfs_key key;
>> +    struct btrfs_path *path;
>> +    struct btrfs_dev_item *dev_item;
>> +    struct btrfs_device *device = NULL;
>> +    int ret;
>> +
>> +    path = btrfs_alloc_path();
>> +    if (!path)
>> +            return ERR_PTR(-ENOMEM);
>> +
>> +    key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
>> +    key.offset = devid;
>> +    key.type = BTRFS_DEV_ITEM_KEY;
>> +
>> +    ret = btrfs_search_slot(NULL, root->fs_info->chunk_root, &key,
>> +                            path, 0, 0);
>> +    if (ret < 0) {
>> +            device = ERR_PTR(ret);
>> +            goto out;
>> +    }
>> +    btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +    if (key.objectid != BTRFS_DEV_ITEMS_OBJECTID ||
>> +        key.type != BTRFS_DEV_ITEM_KEY) {
>> +            device = NULL;
>> +            goto out;
>> +    }
>> +    dev_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +                              struct btrfs_dev_item);
>> +    devid = btrfs_device_id(path->nodes[0], dev_item);
>> +
>> +    device = btrfs_find_device(root, devid, NULL, NULL);
>> +out:
>> +    btrfs_free_path(path);
>> +    return device;
>> +}
>> +
>>  static noinline int btrfs_ioctl_resize(struct btrfs_root *root,
>>                                      void __user *arg)
>>  {
>> -    u64 new_size;
>> +    u64 new_size = 0;
>>      u64 old_size;
>> -    u64 devid = 1;
>> +    u64 orig_new_size = 0;
>> +    u64 devid = (-1ULL);
>>      struct btrfs_ioctl_vol_args *vol_args;
>>      struct btrfs_trans_handle *trans;
>>      struct btrfs_device *device = NULL;
>> @@ -1263,6 +1302,8 @@ static noinline int btrfs_ioctl_resize(struct 
>> btrfs_root *root,
>>      char *devstr = NULL;
>>      int ret = 0;
>>      int mod = 0;
>> +    int scan_all = 1;
>> +    int use_max = 0;
>>  
>>      if (root->fs_info->sb->s_flags & MS_RDONLY)
>>              return -EROFS;
>> @@ -1295,8 +1336,31 @@ static noinline int btrfs_ioctl_resize(struct 
>> btrfs_root *root,
>>              devid = simple_strtoull(devstr, &end, 10);
>>              printk(KERN_INFO "btrfs: resizing devid %llu\n",
>>                     (unsigned long long)devid);
>> +            scan_all = 0;
>>      }
>> -    device = btrfs_find_device(root, devid, NULL, NULL);
>> +
>> +    if (!strcmp(sizestr, "max")) {
>> +            use_max = 1;
>> +    } else {
>> +            if (sizestr[0] == '-') {
>> +                    mod = -1;
>> +                    sizestr++;
>> +            } else if (sizestr[0] == '+') {
>> +                    mod = 1;
>> +                    sizestr++;
>> +            }
>> +            orig_new_size = memparse(sizestr, NULL);
>> +            if (orig_new_size == 0) {
>> +                    ret = -EINVAL;
>> +                    goto out_free;
>> +            }
>> +    }
>> +
>> +    if (devid < (-1ULL))
>> +            device = btrfs_find_device(root, devid, NULL, NULL);
>> +    else
>> +            device = get_avail_device(root, 0);
>> +again:
>>      if (!device) {
>>              printk(KERN_INFO "btrfs: resizer unable to find device %llu\n",
>>                     (unsigned long long)devid);
>> @@ -1310,22 +1374,10 @@ static noinline int btrfs_ioctl_resize(struct 
>> btrfs_root *root,
>>              goto out_free;
>>      }
>>  
>> -    if (!strcmp(sizestr, "max"))
>> +    if (use_max)
>>              new_size = device->bdev->bd_inode->i_size;
>> -    else {
>> -            if (sizestr[0] == '-') {
>> -                    mod = -1;
>> -                    sizestr++;
>> -            } else if (sizestr[0] == '+') {
>> -                    mod = 1;
>> -                    sizestr++;
>> -            }
>> -            new_size = memparse(sizestr, NULL);
>> -            if (new_size == 0) {
>> -                    ret = -EINVAL;
>> -                    goto out_free;
>> -            }
>> -    }
>> +    else
>> +            new_size = orig_new_size;
>>  
>>      old_size = device->total_bytes;
>>  
>> @@ -1365,7 +1417,20 @@ static noinline int btrfs_ioctl_resize(struct 
>> btrfs_root *root,
>>      } else if (new_size < old_size) {
>>              ret = btrfs_shrink_device(device, new_size);
>>      }
>> +    if (ret)
>> +            goto out_free;
>>  
>> +    if (scan_all) {
>> +            /* next available device */
>> +            device = get_avail_device(root, device->devid + 1);
>> +            if (!device)
>> +                    goto out_free;
>> +            if (IS_ERR(device)) {
>> +                    ret = PTR_ERR(device);
>> +                    goto out_free;
>> +            }
>> +            goto again;
>> +    }
>>  out_free:
>>      kfree(vol_args);
>>  out:
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to