On Fri, Sep 06, 2013 at 05:37:53PM +0800, Anand Jain wrote:
Further, to scan for the disks this patch will use
lblkid, so that we don't have to manually scan the
/dev or /dev/mapper which means we don't need the
all-devices options.
Thanks for implementing it! I found a few things to fix, comments below.
I wonder if we should keep --all-devices as a last resort fallback eg.
when blkid cache is not available.
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
-static int uuid_search(struct btrfs_fs_devices *fs_devices, char *search)
-{
- char uuidbuf[37];
- struct list_head *cur;
- struct btrfs_device *device;
- int search_len = strlen(search);
-
- search_len = min(search_len, 37);
- uuid_unparse(fs_devices-fsid, uuidbuf);
- if (!strncmp(uuidbuf, search, search_len))
- return 1;
-
- list_for_each(cur, fs_devices-devices) {
- device = list_entry(cur, struct btrfs_device, dev_list);
- if ((device-label strcmp(device-label, search) == 0) ||
- strcmp(device-name, search) == 0)
- return 1;
- }
- return 0;
-}
This is removing functionality and I don't understand why. It's used for
$ btrfs fi show 9f135b48-cc15-424b-8730-a6432c67dc34
[prints only the given filesystem]
and with your patch does not work as such. Can it be implemented on top
of the code you're adding in this patch? If yes, please make a separate
patch for that.
@@ -232,8 +214,108 @@ static void print_one_uuid(struct btrfs_fs_devices
*fs_devices)
printf(\n);
}
+/* adds up all the used spaces as reported by the space info ioctl
+ */
+static u64 cal_used_bytes(struct btrfs_ioctl_space_args *si)
calc_used_bytes
+static int btrfs_scan_kernel(void *input, int type)
+{
+ int ret = 0, fd;
+ FILE *f;
+ struct mntent *mnt;
+ struct btrfs_ioctl_fs_info_args fi;
+ struct btrfs_ioctl_dev_info_args *di = NULL;
+ struct btrfs_ioctl_space_args *si;
the variable names are not very descriptive
+ char label[BTRFS_LABEL_SIZE];
+
+ f = setmntent(/proc/mounts, r);
should be /proc/self/mounts
+ if (f == NULL)
+ return -errno;
man says that setmntent does not set errno
+
+ while ((mnt = getmntent(f)) != NULL) {
+ if (strcmp(mnt-mnt_type, btrfs))
+ continue;
+ ret = get_fs_info(mnt-mnt_dir, fi, di);
+ if (ret)
+ return ret;
+
+ switch (type) {
Please add defines instead of the integers representing 'type'.
+ case 0:
+ break;
+ case 1:
+ if (uuid_compare(fi.fsid, (u8 *)input))
+ continue;
+ break;
+ case 2:
+ if (strcmp(input, mnt-mnt_dir))
+ continue;
+ break;
I haven't seen 1 and 2 used anywhere
+ default:
+ break;
+ }
+
+ fd = open(mnt-mnt_dir, O_RDONLY);
+ if (fd 0 !get_df(fd, si)) {
+ get_label_mounted(mnt-mnt_dir, label);
+ print_one_fs(fi, di, si, label, mnt-mnt_dir);
+ free(si);
+ }
+ if (fd 0)
+ close(fd);
+ free(di);
+ }
+ return ret;
+}
@@ -244,36 +326,42 @@ static int cmd_show(int argc, char **argv)
+ /* show only mounted btrfs disks */
+ if (argc 1 !strcmp(argv[1], --mounted))
+ where = BTRFS_SCAN_MOUNTED;
- all_uuids = btrfs_scanned_uuids();
- list_for_each(cur_uuid, all_uuids) {
- fs_devices = list_entry(cur_uuid, struct btrfs_fs_devices,
+ switch (where) {
+ case 0:
+ /* no option : show both mounted and unmounted
+ */
+ /* mounted */
+ ret = btrfs_scan_kernel(NULL, 0);
+ if (ret)
+ fprintf(stderr, ERROR: scan kernel failed, %d\n,
+ ret);
I see this warning and there are no mounted filesystems listed in the
output:
$ ./btrfs fi show
ERROR: scan kernel failed, -1
+
+ /* unmounted */
+ scan_for_btrfs_v2(!BTRFS_UPDATE_KERNEL);
+ all_uuids = btrfs_scanned_uuids();
+ list_for_each(cur_uuid, all_uuids) {
+ fs_devices = list_entry(cur_uuid,
+ struct btrfs_fs_devices,
list);
- if (search uuid_search(fs_devices, search) == 0)
- continue;
- print_one_uuid(fs_devices);
+ print_one_uuid(fs_devices);
+ }
+ break;
+ case BTRFS_SCAN_MOUNTED:
+ ret = btrfs_scan_kernel(NULL, 0);
+ if (ret)
+
