All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-01-27  8:52 ` [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS Anand Jain
@ 2014-01-27  8:47   ` Hugo Mills
  2014-01-27  9:08     ` Anand Jain
  2014-02-06 19:49     ` David Sterba
  0 siblings, 2 replies; 14+ messages in thread
From: Hugo Mills @ 2014-01-27  8:47 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 9173 bytes --]

On Mon, Jan 27, 2014 at 04:52:50PM +0800, Anand Jain wrote:
> The user land progs needs   a simple way to read
> the raw list of disks and its parameters as
> btrfs kernel understands it. This patch will
> introduce BTRFS_IOC_GET_DEVS which dumps
> every thing under fs_devices.
> 
> As of now btrfs-devlist uses this ioctl.
> 
> In the long run this ioctl would help to optimize
> some part of btrfs-progs, mainly the current
> btrfs filesystem show

   Just thinking out loud here, really, but can we export this
information in /sys instead, rather than adding yet more ioctls?

   Hugo.

> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/super.c           |   56 +++++++++++++++++++++++++
>  fs/btrfs/volumes.c         |   99 ++++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/volumes.h         |    2 +
>  include/uapi/linux/btrfs.h |   45 ++++++++++++++++++++
>  4 files changed, 202 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 878874b..2fc0e2b 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1693,6 +1693,59 @@ static struct file_system_type btrfs_fs_type = {
>  };
>  MODULE_ALIAS_FS("btrfs");
>  
> +static int btrfs_ioc_get_devlist(void __user *arg)
> +{
> +	int ret = 0;
> +	u64 sz_devlist_arg;
> +	u64 sz_devlist;
> +	u64 sz_out;
> +
> +	struct btrfs_ioctl_devlist_args *devlist_arg;
> +	struct btrfs_ioctl_devlist_args *tmp_devlist_arg;
> +	struct btrfs_ioctl_devlist *devlist;
> +
> +	u64 cnt = 0, ucnt;
> +
> +	sz_devlist_arg = sizeof(*devlist_arg);
> +	sz_devlist = sizeof(*devlist);
> +
> +	if (copy_from_user(&ucnt,
> +		(struct btrfs_ioctl_devlist_args __user *)(arg +
> +		offsetof(struct btrfs_ioctl_devlist_args, count)),
> +			sizeof(ucnt)))
> +		return -EFAULT;
> +
> +	cnt = btrfs_get_devlist_cnt();
> +
> +	if (cnt > ucnt) {
> +		if (copy_to_user(arg +
> +		offsetof(struct btrfs_ioctl_devlist_args, count),
> +			&cnt, sizeof(cnt)))
> +			return -EFAULT;
> +		return 1;
> +	}
> +
> +	sz_out = sz_devlist_arg + sz_devlist * cnt;
> +
> +	tmp_devlist_arg = devlist_arg = memdup_user(arg, sz_out);
> +	if (IS_ERR(devlist_arg))
> +		return PTR_ERR(devlist_arg);
> +
> +	devlist = (struct btrfs_ioctl_devlist *) (++tmp_devlist_arg);
> +	cnt = btrfs_get_devlist(devlist, cnt);
> +	devlist_arg->count = cnt;
> +
> +	if (copy_to_user(arg, devlist_arg, sz_out)) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +	ret = 0;
> +out:
> +	kfree(devlist_arg);
> +	return ret;
> +
> +}
> +
>  static int btrfs_ioc_get_fslist(void __user *arg)
>  {
>  	int ret = 0;
> @@ -1777,6 +1830,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>  	case BTRFS_IOC_GET_FSLIST:
>  		ret = btrfs_ioc_get_fslist(argp);
>  		break;
> +	case BTRFS_IOC_GET_DEVS:
> +		ret = btrfs_ioc_get_devlist(argp);
> +		break;
>  	}
>  
>  	return ret;
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2ca91fc..f00ba27 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6322,3 +6322,102 @@ u64 btrfs_get_fslist(struct btrfs_ioctl_fslist *fslist, u64 ucnt)
>  
>  	return cnt;
>  }
> +
> +int btrfs_get_devlist_cnt(void)
> +{
> +	int cnt = 0;
> +	struct btrfs_device *device;
> +	struct btrfs_fs_devices *fs_devices;
> +
> +	mutex_lock(&uuid_mutex);
> +	list_for_each_entry(fs_devices, &fs_uuids, list)
> +		list_for_each_entry(device, &fs_devices->devices, dev_list)
> +			cnt++;
> +	mutex_unlock(&uuid_mutex);
> +
> +	return cnt;
> +}
> +
> +u64 btrfs_get_devlist(struct btrfs_ioctl_devlist *dev, u64 alloc_cnt)
> +{
> +	u64 cnt = 0;
> +
> +	struct btrfs_device *device;
> +	struct btrfs_fs_devices *fs_devices;
> +
> +	mutex_lock(&uuid_mutex);
> +	/* Todo: there must be better way of doing this */
> +	list_for_each_entry(fs_devices, &fs_uuids, list) {
> +
> +		mutex_lock(&fs_devices->device_list_mutex);
> +
> +		list_for_each_entry(device, &fs_devices->devices, dev_list) {
> +
> +			if (!(cnt < alloc_cnt))
> +				break;
> +
> +			memcpy(dev->fsid, fs_devices->fsid, BTRFS_FSID_SIZE);
> +			dev->fs_latest_devid = fs_devices->latest_devid;
> +			dev->fs_latest_trans = fs_devices->latest_trans;
> +			dev->fs_num_devices = fs_devices->num_devices;
> +			dev->fs_open_devices = fs_devices->open_devices;
> +			dev->fs_rw_devices = fs_devices->rw_devices;
> +			dev->fs_missing_devices = fs_devices->missing_devices;
> +			dev->fs_total_rw_bytes = fs_devices->total_rw_bytes;
> +			dev->fs_num_can_discard = fs_devices->num_can_discard;
> +			dev->fs_total_devices = fs_devices->total_devices;
> +
> +			if (fs_devices->opened)
> +				dev->flags = BTRFS_FS_MOUNTED;
> +			if (fs_devices->seeding)
> +				dev->flags |= BTRFS_FS_SEEDING;
> +			if (fs_devices->rotating)
> +				dev->flags |= BTRFS_FS_ROTATING;
> +
> +			if (device->writeable)
> +				dev->flags |= BTRFS_DEV_WRITEABLE;
> +			if (device->in_fs_metadata)
> +				dev->flags |= BTRFS_DEV_IN_FS_MD;
> +			if (device->missing)
> +				dev->flags |= BTRFS_DEV_MISSING;
> +			if (device->can_discard)
> +				dev->flags |= BTRFS_DEV_CAN_DISCARD;
> +			if (device->is_tgtdev_for_dev_replace)
> +				dev->flags |= BTRFS_DEV_SUBSTITUTED;
> +			if (device->running_pending)
> +				dev->flags |= BTRFS_DEV_RUN_PENDING;
> +			if (device->nobarriers)
> +				dev->flags |= BTRFS_DEV_NOBARRIERS;
> +			if (device->dev_stats_valid)
> +				dev->flags |= BTRFS_DEV_STATS_VALID;
> +			if (device->dev_stats_dirty)
> +				dev->flags |= BTRFS_DEV_STATS_DIRTY;
> +
> +			dev->gen = device->generation;
> +			dev->devid = device->devid;
> +			dev->total_bytes = device->total_bytes;
> +			dev->disk_total_bytes = device->disk_total_bytes;
> +			dev->bytes_used = device->bytes_used;
> +			dev->type = device->type;
> +			dev->io_align = device->io_align;
> +			dev->io_width = device->io_width;
> +			dev->sector_size = device->sector_size;
> +			memcpy(dev->uuid, device->uuid, BTRFS_UUID_SIZE);
> +			if (device->name) {
> +				rcu_read_lock();
> +				memcpy(dev->name, rcu_str_deref(device->name),
> +					BTRFS_PATH_NAME_MAX);
> +				rcu_read_unlock();
> +			} else {
> +				memset(dev->name, 0, BTRFS_PATH_NAME_MAX);
> +			}
> +			dev++;
> +			cnt++;
> +		}
> +
> +		mutex_unlock(&fs_devices->device_list_mutex);
> +	}
> +	mutex_unlock(&uuid_mutex);
> +
> +	return cnt;
> +}
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index ebb0d0c..82a459c 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -392,4 +392,6 @@ static inline void btrfs_dev_stat_reset(struct btrfs_device *dev,
>  }
>  int btrfs_get_fslist_cnt(void);
>  u64 btrfs_get_fslist(struct btrfs_ioctl_fslist *fslist, u64 ucnt);
> +int btrfs_get_devlist_cnt(void);
> +u64 btrfs_get_devlist(struct btrfs_ioctl_devlist *devlist, u64 ucnt);
>  #endif
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 7d7f776..2983019 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -520,6 +520,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
>  
>  /* fs flags */
>  #define BTRFS_FS_MOUNTED	(1LLU << 0)
> +#define BTRFS_FS_SEEDING	(1LLU << 1)
> +#define BTRFS_FS_ROTATING	(1LLU << 2)
>  
>  struct btrfs_ioctl_fslist {
>  	__u64 self_sz;			/* in/out */
> @@ -535,6 +537,47 @@ struct btrfs_ioctl_fslist_args {
>  	__u64 count;		/* out */
>  };
>  
> +#define BTRFS_DEV_WRITEABLE	(1LLU << 8)
> +#define BTRFS_DEV_IN_FS_MD	(1LLU << 9)
> +#define BTRFS_DEV_MISSING	(1LLU << 10)
> +#define BTRFS_DEV_CAN_DISCARD	(1LLU << 11)
> +#define BTRFS_DEV_SUBSTITUTED	(1LLU << 12)
> +#define BTRFS_DEV_RUN_PENDING   (1LLU << 13)
> +#define BTRFS_DEV_NOBARRIERS	(1LLU << 14)
> +#define BTRFS_DEV_STATS_VALID	(1LLU << 15)
> +#define BTRFS_DEV_STATS_DIRTY	(1LLU << 16)
> +
> +struct btrfs_ioctl_devlist {
> +	__u64 sz_self;
> +	__u64 fs_latest_devid;
> +	__u64 fs_latest_trans;
> +	__u64 fs_num_devices;
> +	__u64 fs_open_devices;
> +	__u64 fs_rw_devices;
> +	__u64 fs_missing_devices;
> +	__u64 fs_total_rw_bytes;
> +	__u64 fs_num_can_discard;
> +	__u64 fs_total_devices;
> +	__u64 gen;
> +	__u64 flags;
> +	__u64 devid;
> +	__u64 total_bytes;
> +	__u64 disk_total_bytes;
> +	__u64 bytes_used;
> +	__u64 type;
> +	__u32 io_align;
> +	__u32 io_width;
> +	__u32 sector_size;
> +	__u8 fsid[BTRFS_FSID_SIZE];
> +	__u8 uuid[BTRFS_UUID_SIZE];
> +	__u8 name[BTRFS_PATH_NAME_MAX];
> +}__attribute__ ((__packed__));
> +
> +struct btrfs_ioctl_devlist_args {
> +	__u64 self_sz;		/* in/out */
> +	__u64 count;		/* in/out */
> +};
> +
>  #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
>  				   struct btrfs_ioctl_vol_args)
>  #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
> @@ -638,5 +681,7 @@ struct btrfs_ioctl_fslist_args {
>  				   struct btrfs_ioctl_feature_flags[2])
>  #define BTRFS_IOC_GET_SUPPORTED_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 57, \
>  				   struct btrfs_ioctl_feature_flags[3])
> +#define BTRFS_IOC_GET_DEVS _IOWR(BTRFS_IOCTL_MAGIC, 58, \
> +				     struct btrfs_ioctl_devlist_args)
>  
>  #endif /* _UAPI_LINUX_BTRFS_H */

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
    --- All mushrooms are edible,  but some are only edible once. ---    

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] dump device list as seen by the kernel
@ 2014-01-27  8:52 Anand Jain
  2014-01-27  8:52 ` [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS Anand Jain
  2014-01-27  8:56 ` [PATCH] btrfs-progs: introduce btrfs-devlist Anand Jain
  0 siblings, 2 replies; 14+ messages in thread
From: Anand Jain @ 2014-01-27  8:52 UTC (permalink / raw)
  To: linux-btrfs

For debugging we need a way to understand what kernel
thinks about the devices under its control mainly 
when device disappear and reappear.

The current btrfs-progs sub commands wouldn't help because
the output from the kernel is greatly fine tuned before
printing on the terminal and in some cases provides 
wrong a view (more on it later) as well.

So here I wrote a code to dump fs_devices for the kernel,
the implementation here uses ioctl rather than
memory dumps, as we need this ioctl to fix btrfs subcommands
as well.


Anand Jain (1):
  btrfs: introduce BTRFS_IOC_GET_DEVS

 fs/btrfs/super.c           |   56 +++++++++++++++++++++++++
 fs/btrfs/volumes.c         |   99 ++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h         |    2 +
 include/uapi/linux/btrfs.h |   45 ++++++++++++++++++++
 4 files changed, 202 insertions(+), 0 deletions(-)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-01-27  8:52 [PATCH] dump device list as seen by the kernel Anand Jain
@ 2014-01-27  8:52 ` Anand Jain
  2014-01-27  8:47   ` Hugo Mills
  2014-01-27  8:56 ` [PATCH] btrfs-progs: introduce btrfs-devlist Anand Jain
  1 sibling, 1 reply; 14+ messages in thread
From: Anand Jain @ 2014-01-27  8:52 UTC (permalink / raw)
  To: linux-btrfs

The user land progs needs   a simple way to read
the raw list of disks and its parameters as
btrfs kernel understands it. This patch will
introduce BTRFS_IOC_GET_DEVS which dumps
every thing under fs_devices.

As of now btrfs-devlist uses this ioctl.

In the long run this ioctl would help to optimize
some part of btrfs-progs, mainly the current
btrfs filesystem show

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           |   56 +++++++++++++++++++++++++
 fs/btrfs/volumes.c         |   99 ++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h         |    2 +
 include/uapi/linux/btrfs.h |   45 ++++++++++++++++++++
 4 files changed, 202 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 878874b..2fc0e2b 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1693,6 +1693,59 @@ static struct file_system_type btrfs_fs_type = {
 };
 MODULE_ALIAS_FS("btrfs");
 
+static int btrfs_ioc_get_devlist(void __user *arg)
+{
+	int ret = 0;
+	u64 sz_devlist_arg;
+	u64 sz_devlist;
+	u64 sz_out;
+
+	struct btrfs_ioctl_devlist_args *devlist_arg;
+	struct btrfs_ioctl_devlist_args *tmp_devlist_arg;
+	struct btrfs_ioctl_devlist *devlist;
+
+	u64 cnt = 0, ucnt;
+
+	sz_devlist_arg = sizeof(*devlist_arg);
+	sz_devlist = sizeof(*devlist);
+
+	if (copy_from_user(&ucnt,
+		(struct btrfs_ioctl_devlist_args __user *)(arg +
+		offsetof(struct btrfs_ioctl_devlist_args, count)),
+			sizeof(ucnt)))
+		return -EFAULT;
+
+	cnt = btrfs_get_devlist_cnt();
+
+	if (cnt > ucnt) {
+		if (copy_to_user(arg +
+		offsetof(struct btrfs_ioctl_devlist_args, count),
+			&cnt, sizeof(cnt)))
+			return -EFAULT;
+		return 1;
+	}
+
+	sz_out = sz_devlist_arg + sz_devlist * cnt;
+
+	tmp_devlist_arg = devlist_arg = memdup_user(arg, sz_out);
+	if (IS_ERR(devlist_arg))
+		return PTR_ERR(devlist_arg);
+
+	devlist = (struct btrfs_ioctl_devlist *) (++tmp_devlist_arg);
+	cnt = btrfs_get_devlist(devlist, cnt);
+	devlist_arg->count = cnt;
+
+	if (copy_to_user(arg, devlist_arg, sz_out)) {
+		ret = -EFAULT;
+		goto out;
+	}
+	ret = 0;
+out:
+	kfree(devlist_arg);
+	return ret;
+
+}
+
 static int btrfs_ioc_get_fslist(void __user *arg)
 {
 	int ret = 0;
@@ -1777,6 +1830,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 	case BTRFS_IOC_GET_FSLIST:
 		ret = btrfs_ioc_get_fslist(argp);
 		break;
+	case BTRFS_IOC_GET_DEVS:
+		ret = btrfs_ioc_get_devlist(argp);
+		break;
 	}
 
 	return ret;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2ca91fc..f00ba27 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6322,3 +6322,102 @@ u64 btrfs_get_fslist(struct btrfs_ioctl_fslist *fslist, u64 ucnt)
 
 	return cnt;
 }
+
+int btrfs_get_devlist_cnt(void)
+{
+	int cnt = 0;
+	struct btrfs_device *device;
+	struct btrfs_fs_devices *fs_devices;
+
+	mutex_lock(&uuid_mutex);
+	list_for_each_entry(fs_devices, &fs_uuids, list)
+		list_for_each_entry(device, &fs_devices->devices, dev_list)
+			cnt++;
+	mutex_unlock(&uuid_mutex);
+
+	return cnt;
+}
+
+u64 btrfs_get_devlist(struct btrfs_ioctl_devlist *dev, u64 alloc_cnt)
+{
+	u64 cnt = 0;
+
+	struct btrfs_device *device;
+	struct btrfs_fs_devices *fs_devices;
+
+	mutex_lock(&uuid_mutex);
+	/* Todo: there must be better way of doing this */
+	list_for_each_entry(fs_devices, &fs_uuids, list) {
+
+		mutex_lock(&fs_devices->device_list_mutex);
+
+		list_for_each_entry(device, &fs_devices->devices, dev_list) {
+
+			if (!(cnt < alloc_cnt))
+				break;
+
+			memcpy(dev->fsid, fs_devices->fsid, BTRFS_FSID_SIZE);
+			dev->fs_latest_devid = fs_devices->latest_devid;
+			dev->fs_latest_trans = fs_devices->latest_trans;
+			dev->fs_num_devices = fs_devices->num_devices;
+			dev->fs_open_devices = fs_devices->open_devices;
+			dev->fs_rw_devices = fs_devices->rw_devices;
+			dev->fs_missing_devices = fs_devices->missing_devices;
+			dev->fs_total_rw_bytes = fs_devices->total_rw_bytes;
+			dev->fs_num_can_discard = fs_devices->num_can_discard;
+			dev->fs_total_devices = fs_devices->total_devices;
+
+			if (fs_devices->opened)
+				dev->flags = BTRFS_FS_MOUNTED;
+			if (fs_devices->seeding)
+				dev->flags |= BTRFS_FS_SEEDING;
+			if (fs_devices->rotating)
+				dev->flags |= BTRFS_FS_ROTATING;
+
+			if (device->writeable)
+				dev->flags |= BTRFS_DEV_WRITEABLE;
+			if (device->in_fs_metadata)
+				dev->flags |= BTRFS_DEV_IN_FS_MD;
+			if (device->missing)
+				dev->flags |= BTRFS_DEV_MISSING;
+			if (device->can_discard)
+				dev->flags |= BTRFS_DEV_CAN_DISCARD;
+			if (device->is_tgtdev_for_dev_replace)
+				dev->flags |= BTRFS_DEV_SUBSTITUTED;
+			if (device->running_pending)
+				dev->flags |= BTRFS_DEV_RUN_PENDING;
+			if (device->nobarriers)
+				dev->flags |= BTRFS_DEV_NOBARRIERS;
+			if (device->dev_stats_valid)
+				dev->flags |= BTRFS_DEV_STATS_VALID;
+			if (device->dev_stats_dirty)
+				dev->flags |= BTRFS_DEV_STATS_DIRTY;
+
+			dev->gen = device->generation;
+			dev->devid = device->devid;
+			dev->total_bytes = device->total_bytes;
+			dev->disk_total_bytes = device->disk_total_bytes;
+			dev->bytes_used = device->bytes_used;
+			dev->type = device->type;
+			dev->io_align = device->io_align;
+			dev->io_width = device->io_width;
+			dev->sector_size = device->sector_size;
+			memcpy(dev->uuid, device->uuid, BTRFS_UUID_SIZE);
+			if (device->name) {
+				rcu_read_lock();
+				memcpy(dev->name, rcu_str_deref(device->name),
+					BTRFS_PATH_NAME_MAX);
+				rcu_read_unlock();
+			} else {
+				memset(dev->name, 0, BTRFS_PATH_NAME_MAX);
+			}
+			dev++;
+			cnt++;
+		}
+
+		mutex_unlock(&fs_devices->device_list_mutex);
+	}
+	mutex_unlock(&uuid_mutex);
+
+	return cnt;
+}
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index ebb0d0c..82a459c 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -392,4 +392,6 @@ static inline void btrfs_dev_stat_reset(struct btrfs_device *dev,
 }
 int btrfs_get_fslist_cnt(void);
 u64 btrfs_get_fslist(struct btrfs_ioctl_fslist *fslist, u64 ucnt);
+int btrfs_get_devlist_cnt(void);
+u64 btrfs_get_devlist(struct btrfs_ioctl_devlist *devlist, u64 ucnt);
 #endif
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 7d7f776..2983019 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -520,6 +520,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 
 /* fs flags */
 #define BTRFS_FS_MOUNTED	(1LLU << 0)
+#define BTRFS_FS_SEEDING	(1LLU << 1)
+#define BTRFS_FS_ROTATING	(1LLU << 2)
 
 struct btrfs_ioctl_fslist {
 	__u64 self_sz;			/* in/out */
@@ -535,6 +537,47 @@ struct btrfs_ioctl_fslist_args {
 	__u64 count;		/* out */
 };
 
+#define BTRFS_DEV_WRITEABLE	(1LLU << 8)
+#define BTRFS_DEV_IN_FS_MD	(1LLU << 9)
+#define BTRFS_DEV_MISSING	(1LLU << 10)
+#define BTRFS_DEV_CAN_DISCARD	(1LLU << 11)
+#define BTRFS_DEV_SUBSTITUTED	(1LLU << 12)
+#define BTRFS_DEV_RUN_PENDING   (1LLU << 13)
+#define BTRFS_DEV_NOBARRIERS	(1LLU << 14)
+#define BTRFS_DEV_STATS_VALID	(1LLU << 15)
+#define BTRFS_DEV_STATS_DIRTY	(1LLU << 16)
+
+struct btrfs_ioctl_devlist {
+	__u64 sz_self;
+	__u64 fs_latest_devid;
+	__u64 fs_latest_trans;
+	__u64 fs_num_devices;
+	__u64 fs_open_devices;
+	__u64 fs_rw_devices;
+	__u64 fs_missing_devices;
+	__u64 fs_total_rw_bytes;
+	__u64 fs_num_can_discard;
+	__u64 fs_total_devices;
+	__u64 gen;
+	__u64 flags;
+	__u64 devid;
+	__u64 total_bytes;
+	__u64 disk_total_bytes;
+	__u64 bytes_used;
+	__u64 type;
+	__u32 io_align;
+	__u32 io_width;
+	__u32 sector_size;
+	__u8 fsid[BTRFS_FSID_SIZE];
+	__u8 uuid[BTRFS_UUID_SIZE];
+	__u8 name[BTRFS_PATH_NAME_MAX];
+}__attribute__ ((__packed__));
+
+struct btrfs_ioctl_devlist_args {
+	__u64 self_sz;		/* in/out */
+	__u64 count;		/* in/out */
+};
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -638,5 +681,7 @@ struct btrfs_ioctl_fslist_args {
 				   struct btrfs_ioctl_feature_flags[2])
 #define BTRFS_IOC_GET_SUPPORTED_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 57, \
 				   struct btrfs_ioctl_feature_flags[3])
+#define BTRFS_IOC_GET_DEVS _IOWR(BTRFS_IOCTL_MAGIC, 58, \
+				     struct btrfs_ioctl_devlist_args)
 
 #endif /* _UAPI_LINUX_BTRFS_H */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH] btrfs-progs: introduce btrfs-devlist
  2014-01-27  8:52 [PATCH] dump device list as seen by the kernel Anand Jain
  2014-01-27  8:52 ` [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS Anand Jain
@ 2014-01-27  8:56 ` Anand Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Anand Jain @ 2014-01-27  8:56 UTC (permalink / raw)
  To: linux-btrfs

This is a small (debug) program to dump the device list
in the raw format from the btrfs kernel.
here I use ioctl which was introduced in the below kernel
patch

btrfs: introduce BTRFS_IOC_GET_DEVS

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 .gitignore      |    1 +
 Makefile        |    4 +-
 btrfs-devlist.c |  230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ioctl.h         |   45 +++++++++++
 4 files changed, 278 insertions(+), 2 deletions(-)
 create mode 100644 btrfs-devlist.c

diff --git a/.gitignore b/.gitignore
index ab8b81c..0928374 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,3 +34,4 @@ libbtrfs.a
 libbtrfs.so
 libbtrfs.so.0
 libbtrfs.so.0.1
+btrfs-devlist
diff --git a/Makefile b/Makefile
index f99ca7c..2a52c17 100644
--- a/Makefile
+++ b/Makefile
@@ -47,7 +47,7 @@ MAKEOPTS = --no-print-directory Q=$(Q)
 
 progs = mkfs.btrfs btrfs-debug-tree btrfsck \
 	btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \
-	btrfs-find-root btrfstune btrfs-show-super
+	btrfs-find-root btrfstune btrfs-show-super btrfs-devlist
 
 # external libs required by various binaries; for btrfs-foo,
 # specify btrfs_foo_libs = <list of libs>; see $($(subst...)) rules below
@@ -226,7 +226,7 @@ clean: $(CLEANDIRS)
 	@echo "Cleaning"
 	$(Q)rm -f $(progs) cscope.out *.o *.o.d btrfs-convert btrfs-image btrfs-select-super \
 	      btrfs-zero-log btrfstune dir-test ioctl-test quick-test send-test btrfsck \
-	      btrfs.static mkfs.btrfs.static btrfs-calc-size \
+	      btrfs.static mkfs.btrfs.static btrfs-calc-size btrfs-devlist\
 	      version.h $(check_defs) \
 	      $(libs) $(lib_links)
 
diff --git a/btrfs-devlist.c b/btrfs-devlist.c
new file mode 100644
index 0000000..7d9ef73
--- /dev/null
+++ b/btrfs-devlist.c
@@ -0,0 +1,230 @@
+/*
+ * Copyright (C) 2014 Oracle.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <unistd.h>
+#include <getopt.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <uuid/uuid.h>
+
+#include "ctree.h"
+#include "ioctl.h"
+#include "commands.h"
+#include "utils.h"
+
+void print_header(void)
+{
+	printf(
+		"fsid "\
+		"uuid "\
+		"name \n"\
+		"\tfs_latest_devid "\
+		"fs_latest_trans "\
+		"fs_num_devices "\
+		"fs_open_devices "\
+		"fs_rw_devices "\
+		"fs_missing_devices "\
+		"fs_total_rw_bytes "\
+		"fs_num_can_discard "\
+		"fs_total_devices \n"\
+		"\tgen "\
+		"devid "\
+		"total_bytes "\
+		"disk_total_bytes "\
+		"bytes_used "\
+		"type "\
+		"io_align "\
+		"io_width "\
+		"sector_size \n"\
+		"\tflags\n");
+}
+
+void print_dev(struct btrfs_ioctl_devlist *dev)
+{
+	char uuid[BTRFS_UUID_UNPARSED_SIZE];
+	char fsid[BTRFS_UUID_UNPARSED_SIZE];
+
+	uuid_unparse(dev->uuid, uuid);
+	uuid_unparse(dev->fsid, fsid);
+
+	printf("\n%s %s %s\n"\
+		"\t%llu %llu %llu %llu %llu %llu %llu %llu %llu\n"\
+		"\t%llu %llu %llu %llu %llu %llu %u %u %u\n"\
+		"\t%s|%s|%s\n"\
+		"\t%s|%s|%s|%s|%s|%s|%s|%s|%s\n",
+		fsid,
+		uuid,
+		dev->name,
+		dev->fs_latest_devid,
+		dev->fs_latest_trans,
+		dev->fs_num_devices,
+		dev->fs_open_devices,
+		dev->fs_rw_devices,
+		dev->fs_missing_devices,
+		dev->fs_total_rw_bytes,
+		dev->fs_num_can_discard,
+		dev->fs_total_devices,
+		dev->gen,
+		dev->devid,
+		dev->total_bytes,
+		dev->disk_total_bytes,
+		dev->bytes_used,
+		dev->type,
+		dev->io_align,
+		dev->io_width,
+		dev->sector_size,
+		dev->flags & BTRFS_FS_MOUNTED ? "fs_Mounted":"not_fs_Mounted",
+		dev->flags & BTRFS_FS_SEEDING ? "fs_Seeding":"not_fs_Seeding",
+		dev->flags & BTRFS_FS_ROTATING ? "fs_Rotating":"not_fs_Rotating",
+		dev->flags & BTRFS_DEV_WRITEABLE ? "Writable":"not_Writable",
+		dev->flags & BTRFS_DEV_IN_FS_MD ? "MD":"not_MD",
+		dev->flags & BTRFS_DEV_MISSING ? "Missing":"not_Missing",
+		dev->flags & BTRFS_DEV_CAN_DISCARD ? "Discard":"not_Discard",
+		dev->flags & BTRFS_DEV_SUBSTITUTED ? "Substituted":"not_Substituted",
+		dev->flags & BTRFS_DEV_RUN_PENDING ? "Run_pending":"not_Run_pending",
+		dev->flags & BTRFS_DEV_NOBARRIERS ? "Nobarriers":"not_Nobarriers",
+		dev->flags & BTRFS_DEV_STATS_VALID ? "Stat_valid":"not_Stat_valid",
+		dev->flags & BTRFS_DEV_STATS_DIRTY ? "Stat_dirty":"not_Stat_dirty");
+}
+
+int get_devlist(struct btrfs_ioctl_devlist **out_devlist, u64 *out_count)
+{
+	int ret, fd, e;
+	struct btrfs_ioctl_devlist_args *devargs;
+	struct btrfs_ioctl_devlist_args *devargs_saved = NULL;
+	struct btrfs_ioctl_devlist *devlist;
+	u64 sz;
+	int count;
+
+	fd = open("/dev/btrfs-control", O_RDWR);
+	e = errno;
+	if (fd < 0) {
+		perror("failed to open /dev/btrfs-control");
+		return -e;
+	}
+
+	/* space to hold 512 fsids, doesn't matter if small
+	 * it would fail and return count so then we try again
+	 */
+	count = 512;
+again:
+	sz = sizeof(*devargs) + sizeof(*devlist) * count;
+
+	devargs_saved = devargs = malloc(sz);
+	if (!devargs) {
+		close(fd);
+		return -ENOMEM;
+	}
+
+	memset(devargs, 0, sz);
+	devargs->count = count;
+
+	ret = ioctl(fd, BTRFS_IOC_GET_DEVS, devargs);
+	e = errno;
+	if (ret == 1) {
+		/* out of size so reallocate */
+		count = devargs->count;
+		free(devargs);
+		goto again;
+	} else if (ret < 0) {
+		printf("ERROR: scan_fsid ioctl failed - %s\n",
+			strerror(e));
+		ret = -e;
+		goto out;
+	}
+
+	/* ioctl returns devs count in count parameter*/
+
+	*out_count = devargs->count;
+	if (*out_count == 0) {
+		*out_devlist = NULL;
+		ret = 0;
+		goto out;
+	}
+
+	devlist = (struct btrfs_ioctl_devlist *) (++devargs);
+
+	sz = sizeof(*devlist) * *out_count;
+	*out_devlist = malloc(sz);
+	if (*out_devlist == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	memcpy(*out_devlist, devlist, sz);
+	ret = 0;
+
+out:
+	free(devargs_saved);
+	close(fd);
+	return ret;
+}
+
+static void local_usage(void)
+{
+	printf("btrfs_devlist [options]\n"\
+	"	Dumps the device list from the btrfs kernel "\
+	"without altering at the user level\n"\
+	"	options:\n"\
+	"		-h|--help prints this usage\n");
+	exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+	int ret;
+	u64 cnt;
+	struct btrfs_ioctl_devlist *devlist;
+	struct btrfs_ioctl_devlist *tmp_devlist;
+
+	while(1) {
+		int long_index;
+		static struct option long_options[] = {
+			{"help", no_argument, NULL, 'h'},
+			{ NULL, no_argument, NULL, 0 }
+		};
+
+		int c = getopt_long(argc, argv, "h", long_options,
+			&long_index);
+
+		if(c < 0)
+			break;
+
+		switch(c) {
+		case 'h':
+		default:
+			local_usage();
+		}
+	}
+
+	ret = get_devlist(&devlist, &cnt);
+	if (ret) {
+		fprintf(stderr, "get devlist failed %d\n", ret);
+		return 1;
+	}
+
+	tmp_devlist = devlist;
+
+	print_header();
+	while (cnt--) {
+		print_dev(devlist);
+		devlist++;
+	}
+	kfree(tmp_devlist);
+	return ret;
+}
diff --git a/ioctl.h b/ioctl.h
index f84300b..7343a33 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -508,6 +508,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 
 /* fs flags */
 #define BTRFS_FS_MOUNTED	(1LLU << 0)
+#define BTRFS_FS_SEEDING	(1LLU << 1)
+#define BTRFS_FS_ROTATING	(1LLU << 2)
 
 struct btrfs_ioctl_fslist {
 	__u64 self_sz;			/* in/out */
@@ -523,6 +525,47 @@ struct btrfs_ioctl_fslist_args {
 	__u64 count;		/* out */
 };
 
+#define BTRFS_DEV_WRITEABLE	(1LLU << 8)
+#define BTRFS_DEV_IN_FS_MD	(1LLU << 9)
+#define BTRFS_DEV_MISSING	(1LLU << 10)
+#define BTRFS_DEV_CAN_DISCARD	(1LLU << 11)
+#define BTRFS_DEV_SUBSTITUTED	(1LLU << 12)
+#define BTRFS_DEV_RUN_PENDING   (1LLU << 13)
+#define BTRFS_DEV_NOBARRIERS	(1LLU << 14)
+#define BTRFS_DEV_STATS_VALID	(1LLU << 15)
+#define BTRFS_DEV_STATS_DIRTY	(1LLU << 16)
+
+struct btrfs_ioctl_devlist {
+	__u64 sz_self;
+	__u64 fs_latest_devid;
+	__u64 fs_latest_trans;
+	__u64 fs_num_devices;
+	__u64 fs_open_devices;
+	__u64 fs_rw_devices;
+	__u64 fs_missing_devices;
+	__u64 fs_total_rw_bytes;
+	__u64 fs_num_can_discard;
+	__u64 fs_total_devices;
+	__u64 gen;
+	__u64 flags;
+	__u64 devid;
+	__u64 total_bytes;
+	__u64 disk_total_bytes;
+	__u64 bytes_used;
+	__u64 type;
+	__u32 io_align;
+	__u32 io_width;
+	__u32 sector_size;
+	__u8 fsid[BTRFS_FSID_SIZE];
+	__u8 uuid[BTRFS_UUID_SIZE];
+	__u8 name[BTRFS_PATH_NAME_MAX];
+}__attribute__ ((__packed__));
+
+struct btrfs_ioctl_devlist_args {
+	__u64 self_sz;		/* in/out */
+	__u64 count;		/* in/out */
+};
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -631,6 +674,8 @@ struct btrfs_ioctl_clone_range_args {
                                   struct btrfs_ioctl_feature_flags[2])
 #define BTRFS_IOC_GET_SUPPORTED_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 57, \
                                   struct btrfs_ioctl_feature_flags[3])
+#define BTRFS_IOC_GET_DEVS _IOWR(BTRFS_IOCTL_MAGIC, 58, \
+				     struct btrfs_ioctl_devlist_args)
 #ifdef __cplusplus
 }
 #endif
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-01-27  8:47   ` Hugo Mills
@ 2014-01-27  9:08     ` Anand Jain
  2014-02-06 19:49     ` David Sterba
  1 sibling, 0 replies; 14+ messages in thread
From: Anand Jain @ 2014-01-27  9:08 UTC (permalink / raw)
  To: Hugo Mills, linux-btrfs



On 01/27/2014 04:47 PM, Hugo Mills wrote:
> On Mon, Jan 27, 2014 at 04:52:50PM +0800, Anand Jain wrote:
>> The user land progs needs   a simple way to read
>> the raw list of disks and its parameters as
>> btrfs kernel understands it. This patch will
>> introduce BTRFS_IOC_GET_DEVS which dumps
>> every thing under fs_devices.
>>
>> As of now btrfs-devlist uses this ioctl.
>>
>> In the long run this ioctl would help to optimize
>> some part of btrfs-progs, mainly the current
>> btrfs filesystem show
>
>     Just thinking out loud here, really, but can we export this
> information in /sys instead, rather than adding yet more ioctls?
>

  I was more inclined towards (live) memory dump. But I need to fix
  btrfs-progs as well so ioctl.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-01-27  8:47   ` Hugo Mills
  2014-01-27  9:08     ` Anand Jain
@ 2014-02-06 19:49     ` David Sterba
  2014-02-06 20:09       ` Goffredo Baroncelli
  1 sibling, 1 reply; 14+ messages in thread
From: David Sterba @ 2014-02-06 19:49 UTC (permalink / raw)
  To: Hugo Mills, Anand Jain, linux-btrfs

On Mon, Jan 27, 2014 at 08:47:09AM +0000, Hugo Mills wrote:
> On Mon, Jan 27, 2014 at 04:52:50PM +0800, Anand Jain wrote:
> > The user land progs needs   a simple way to read
> > the raw list of disks and its parameters as
> > btrfs kernel understands it. This patch will
> > introduce BTRFS_IOC_GET_DEVS which dumps
> > every thing under fs_devices.
> > 
> > As of now btrfs-devlist uses this ioctl.
> > 
> > In the long run this ioctl would help to optimize
> > some part of btrfs-progs, mainly the current
> > btrfs filesystem show
> 
>    Just thinking out loud here, really, but can we export this
> information in /sys instead, rather than adding yet more ioctls?

I tend to agree that this belongs to sysfs, it's more flexible in case
we'll add more per-device stats, ie. one file that holds all the stats
about the device. This is also easier to use from scripts that gather
system information.

With 3.14 the sysfs interface is available, but the devices under
 /sys/fs/btrfs/<fs-uuid>/devices/...
are symlinks to the sysfs devices, so this btrfs-specific device
information has to be located in a separate directory.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-06 19:49     ` David Sterba
@ 2014-02-06 20:09       ` Goffredo Baroncelli
  2014-02-06 22:05         ` David Sterba
  0 siblings, 1 reply; 14+ messages in thread
From: Goffredo Baroncelli @ 2014-02-06 20:09 UTC (permalink / raw)
  To: dsterba, Hugo Mills, Anand Jain, linux-btrfs

On 02/06/2014 08:49 PM, David Sterba wrote:
> On Mon, Jan 27, 2014 at 08:47:09AM +0000, Hugo Mills wrote:
>> On Mon, Jan 27, 2014 at 04:52:50PM +0800, Anand Jain wrote:
>>> The user land progs needs   a simple way to read
>>> the raw list of disks and its parameters as
>>> btrfs kernel understands it. This patch will
>>> introduce BTRFS_IOC_GET_DEVS which dumps
>>> every thing under fs_devices.
>>>
>>> As of now btrfs-devlist uses this ioctl.
>>>
>>> In the long run this ioctl would help to optimize
>>> some part of btrfs-progs, mainly the current
>>> btrfs filesystem show
>>
>>    Just thinking out loud here, really, but can we export this
>> information in /sys instead, rather than adding yet more ioctls?
> 
> I tend to agree that this belongs to sysfs, it's more flexible in case
> we'll add more per-device stats, ie. one file that holds all the stats
> about the device. This is also easier to use from scripts that gather
> system information.
> 
> With 3.14 the sysfs interface is available, but the devices under
>  /sys/fs/btrfs/<fs-uuid>/devices/...
> are symlinks to the sysfs devices, so this btrfs-specific device
> information has to be located in a separate directory.

yes please; it would scale better than the ioctl()s; anyway I suggest 1
file per property, and not "...one file that holds all the stats
 about the device"

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


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-06 20:09       ` Goffredo Baroncelli
@ 2014-02-06 22:05         ` David Sterba
  2014-02-07 10:08           ` Anand Jain
  0 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2014-02-06 22:05 UTC (permalink / raw)
  To: Goffredo Baroncelli; +Cc: Hugo Mills, Anand Jain, linux-btrfs

On Thu, Feb 06, 2014 at 09:09:57PM +0100, Goffredo Baroncelli wrote:
> > With 3.14 the sysfs interface is available, but the devices under
> >  /sys/fs/btrfs/<fs-uuid>/devices/...
> > are symlinks to the sysfs devices, so this btrfs-specific device
> > information has to be located in a separate directory.
> 
> yes please; it would scale better than the ioctl()s; anyway I suggest 1
> file per property, and not "...one file that holds all the stats
>  about the device"

Well, we can do both, I don't have a preference and both make sense.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-06 22:05         ` David Sterba
@ 2014-02-07 10:08           ` Anand Jain
  2014-02-07 10:20             ` Hugo Mills
  0 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2014-02-07 10:08 UTC (permalink / raw)
  To: dsterba, Goffredo Baroncelli, Hugo Mills, linux-btrfs



  Thanks for the comments.

  mainly here sysfs way defeats the purpose - debug as
  mentioned. Sysfs would/should show only mounted disks,
  the ioctl way doesn't have such a limitation (of course
  as I commented memory dump way would have been best choice
  but I got stuck with that approach if anybody wants to give
  a try with that approach you are most welcome).

  IMO no harm to have both sysfs way and ioctl way let user
  or developer use which were is suitable in their context.

Thanks, Anand


On 02/07/2014 06:05 AM, David Sterba wrote:
> On Thu, Feb 06, 2014 at 09:09:57PM +0100, Goffredo Baroncelli wrote:
>>> With 3.14 the sysfs interface is available, but the devices under
>>>   /sys/fs/btrfs/<fs-uuid>/devices/...
>>> are symlinks to the sysfs devices, so this btrfs-specific device
>>> information has to be located in a separate directory.
>>
>> yes please; it would scale better than the ioctl()s; anyway I suggest 1
>> file per property, and not "...one file that holds all the stats
>>   about the device"
>
> Well, we can do both, I don't have a preference and both make sense.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-07 10:08           ` Anand Jain
@ 2014-02-07 10:20             ` Hugo Mills
  2014-02-12 16:01               ` David Sterba
  0 siblings, 1 reply; 14+ messages in thread
From: Hugo Mills @ 2014-02-07 10:20 UTC (permalink / raw)
  To: Anand Jain; +Cc: dsterba, Goffredo Baroncelli, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 2673 bytes --]

On Fri, Feb 07, 2014 at 06:08:56PM +0800, Anand Jain wrote:
> 
> 
>  Thanks for the comments.
> 
>  mainly here sysfs way defeats the purpose - debug as
>  mentioned. Sysfs would/should show only mounted disks,

   So let's find a way of showing the "known-about" data structure
separately from the "mounted" data structure. Just thinking aloud
here, how about something like this:

For unmounted, but known filesystems:
/sys/fs/btrfs/devmap/<UUID>/<symlink to device>   (for each device known)
/sys/fs/btrfs/devmap/<UUID>/label
/sys/fs/btrfs/devmap/<UUID>/complete      (0 if missing devices, 1 otherwise)
/sys/fs/btrfs/devmap/<UUID>/<other properties>

And then for mounted filesystems, we can populate the <other
properties> with more details as appropriate (and remove them when
unmounted again). We can also add a symlink to the <UUID> directory
for the filesystem to e.g.:

/sys/fs/btrfs/mounted/<symlink to UUID>

   If truly the *only* reason to do this is debug, then simply dump
the info in debugfs and have done with it. However, there are good
reasons to want to have this information in an ordinary running
system, too. So stick with sysfs, IMO.

>  the ioctl way doesn't have such a limitation (of course
>  as I commented memory dump way would have been best choice
>  but I got stuck with that approach if anybody wants to give
>  a try with that approach you are most welcome).
> 
>  IMO no harm to have both sysfs way and ioctl way let user
>  or developer use which were is suitable in their context.

   Extra maintenance overhead; divergence of the APIs. "You can find
out the label of the filesystem using sysfs, but not the ioctl. You
can find out the size of the filesystem using the ioctl, but not using
sysfs." That way lies quite a bit of pain for the user of the
interfaces.

   Hugo.

> Thanks, Anand
> 
> 
> On 02/07/2014 06:05 AM, David Sterba wrote:
> >On Thu, Feb 06, 2014 at 09:09:57PM +0100, Goffredo Baroncelli wrote:
> >>>With 3.14 the sysfs interface is available, but the devices under
> >>>  /sys/fs/btrfs/<fs-uuid>/devices/...
> >>>are symlinks to the sysfs devices, so this btrfs-specific device
> >>>information has to be located in a separate directory.
> >>
> >>yes please; it would scale better than the ioctl()s; anyway I suggest 1
> >>file per property, and not "...one file that holds all the stats
> >>  about the device"
> >
> >Well, we can do both, I don't have a preference and both make sense.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
       --- Great oxymorons of the world, no. 4: Future Perfect ---       

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-07 10:20             ` Hugo Mills
@ 2014-02-12 16:01               ` David Sterba
  2014-02-12 16:15                 ` Hugo Mills
  0 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2014-02-12 16:01 UTC (permalink / raw)
  To: Hugo Mills, Anand Jain, Goffredo Baroncelli, linux-btrfs

On Fri, Feb 07, 2014 at 10:20:10AM +0000, Hugo Mills wrote:
> On Fri, Feb 07, 2014 at 06:08:56PM +0800, Anand Jain wrote:
> >  mainly here sysfs way defeats the purpose - debug as
> >  mentioned. Sysfs would/should show only mounted disks,
> 
>    So let's find a way of showing the "known-about" data structure
> separately from the "mounted" data structure. Just thinking aloud
> here, how about something like this:
> 
> For unmounted, but known filesystems:
> /sys/fs/btrfs/devmap/<UUID>/<symlink to device>   (for each device known)
> /sys/fs/btrfs/devmap/<UUID>/label
> /sys/fs/btrfs/devmap/<UUID>/complete      (0 if missing devices, 1 otherwise)
> /sys/fs/btrfs/devmap/<UUID>/<other properties>

I like that, the device map represents the global state maintained by the
module, does not interfere with the current sysfs structure.

One minor comment, the symlink to device should have probably a fixed
name so it can be easily located.

> And then for mounted filesystems, we can populate the <other
> properties> with more details as appropriate (and remove them when
> unmounted again). We can also add a symlink to the <UUID> directory
> for the filesystem to e.g.:
> 
> /sys/fs/btrfs/mounted/<symlink to UUID>
> 
>    If truly the *only* reason to do this is debug, then simply dump
> the info in debugfs and have done with it. However, there are good
> reasons to want to have this information in an ordinary running
> system, too. So stick with sysfs, IMO.

Agreed.

> >  the ioctl way doesn't have such a limitation (of course
> >  as I commented memory dump way would have been best choice
> >  but I got stuck with that approach if anybody wants to give
> >  a try with that approach you are most welcome).
> > 
> >  IMO no harm to have both sysfs way and ioctl way let user
> >  or developer use which were is suitable in their context.
> 
>    Extra maintenance overhead; divergence of the APIs. "You can find
> out the label of the filesystem using sysfs, but not the ioctl. You
> can find out the size of the filesystem using the ioctl, but not using
> sysfs." That way lies quite a bit of pain for the user of the
> interfaces.

I won't mind having several ways how to get the information, sysfs,
ioctl or the properties. Each way has it's own pros and cons or is
suitable for some type of user (C program, shell scripts).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-12 16:01               ` David Sterba
@ 2014-02-12 16:15                 ` Hugo Mills
  2014-02-25 17:45                   ` David Sterba
  0 siblings, 1 reply; 14+ messages in thread
From: Hugo Mills @ 2014-02-12 16:15 UTC (permalink / raw)
  To: dsterba, Anand Jain, Goffredo Baroncelli, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 3437 bytes --]

On Wed, Feb 12, 2014 at 05:01:23PM +0100, David Sterba wrote:
> On Fri, Feb 07, 2014 at 10:20:10AM +0000, Hugo Mills wrote:
> > On Fri, Feb 07, 2014 at 06:08:56PM +0800, Anand Jain wrote:
> > >  mainly here sysfs way defeats the purpose - debug as
> > >  mentioned. Sysfs would/should show only mounted disks,
> > 
> >    So let's find a way of showing the "known-about" data structure
> > separately from the "mounted" data structure. Just thinking aloud
> > here, how about something like this:
> > 
> > For unmounted, but known filesystems:
> > /sys/fs/btrfs/devmap/<UUID>/<symlink to device>   (for each device known)
> > /sys/fs/btrfs/devmap/<UUID>/label
> > /sys/fs/btrfs/devmap/<UUID>/complete      (0 if missing devices, 1 otherwise)
> > /sys/fs/btrfs/devmap/<UUID>/<other properties>
> 
> I like that, the device map represents the global state maintained by the
> module, does not interfere with the current sysfs structure.
> 
> One minor comment, the symlink to device should have probably a fixed
> name so it can be easily located.

   Two ways spring to mind: Number them according to the internal FS's
device numbers, so you'll have something like:

$ ls -l /sys/fs/btrfs/devmap/3993e50e-a926-48a4-867f-36b53d924c35
total 0
lrwxrwxrwx 1 root root    0 Feb  1 11:22 0 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:0:0/0:0:0:0/block/sda
lrwxrwxrwx 1 root root    0 Feb  1 11:22 1 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:1:0/0:1:0:0/block/sdb
lrwxrwxrwx 1 root root    0 Feb  1 11:22 2 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:2:0/0:2:0:0/block/sdc
lrwxrwxrwx 1 root root    0 Feb  1 11:22 3 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:3:0/0:3:0:0/block/sdd
-r--r--r-- 1 root root 4096 Feb  1 11:22 complete
-r--r--r-- 1 root root 4096 Feb  1 11:22 label

   Then you can find them easily by enumerating the filenames, and
anything which is composed entirely of digits [0-9] is a device.
Alternatively, put them into a subdirectory, and anything at all
within that subdir is a device symlink.

[snip]
> > >  the ioctl way doesn't have such a limitation (of course
> > >  as I commented memory dump way would have been best choice
> > >  but I got stuck with that approach if anybody wants to give
> > >  a try with that approach you are most welcome).
> > > 
> > >  IMO no harm to have both sysfs way and ioctl way let user
> > >  or developer use which were is suitable in their context.
> > 
> >    Extra maintenance overhead; divergence of the APIs. "You can find
> > out the label of the filesystem using sysfs, but not the ioctl. You
> > can find out the size of the filesystem using the ioctl, but not using
> > sysfs." That way lies quite a bit of pain for the user of the
> > interfaces.
> 
> I won't mind having several ways how to get the information, sysfs,
> ioctl or the properties. Each way has it's own pros and cons or is
> suitable for some type of user (C program, shell scripts).

   Fair enough. I do worry a little about the API divergence, though.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
  --- "What's so bad about being drunk?" "You ask a glass of water" ---  
                                                                         

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-12 16:15                 ` Hugo Mills
@ 2014-02-25 17:45                   ` David Sterba
  2014-02-26  2:34                     ` Anand Jain
  0 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2014-02-25 17:45 UTC (permalink / raw)
  To: Hugo Mills, dsterba, Anand Jain, Goffredo Baroncelli, linux-btrfs

On Wed, Feb 12, 2014 at 04:15:55PM +0000, Hugo Mills wrote:
> On Wed, Feb 12, 2014 at 05:01:23PM +0100, David Sterba wrote:
> > On Fri, Feb 07, 2014 at 10:20:10AM +0000, Hugo Mills wrote:
> > > For unmounted, but known filesystems:
> > > /sys/fs/btrfs/devmap/<UUID>/<symlink to device>   (for each device known)
> > > /sys/fs/btrfs/devmap/<UUID>/label
> > > /sys/fs/btrfs/devmap/<UUID>/complete      (0 if missing devices, 1 otherwise)
> > > /sys/fs/btrfs/devmap/<UUID>/<other properties>
> > 
> > I like that, the device map represents the global state maintained by the
> > module, does not interfere with the current sysfs structure.
> > 
> > One minor comment, the symlink to device should have probably a fixed
> > name so it can be easily located.
> 
>    Two ways spring to mind: Number them according to the internal FS's
> device numbers, so you'll have something like:
> 
> $ ls -l /sys/fs/btrfs/devmap/3993e50e-a926-48a4-867f-36b53d924c35
> total 0
> lrwxrwxrwx 1 root root    0 Feb  1 11:22 0 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:0:0/0:0:0:0/block/sda
> lrwxrwxrwx 1 root root    0 Feb  1 11:22 1 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:1:0/0:1:0:0/block/sdb
> lrwxrwxrwx 1 root root    0 Feb  1 11:22 2 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:2:0/0:2:0:0/block/sdc
> lrwxrwxrwx 1 root root    0 Feb  1 11:22 3 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:3:0/0:3:0:0/block/sdd
> -r--r--r-- 1 root root 4096 Feb  1 11:22 complete
> -r--r--r-- 1 root root 4096 Feb  1 11:22 label
> 
>    Then you can find them easily by enumerating the filenames, and
> anything which is composed entirely of digits [0-9] is a device.
> Alternatively, put them into a subdirectory, and anything at all
> within that subdir is a device symlink.

A separate subdirectory looks more user friendly, a simple subdir/*
would get them all, regardless of the names. This is much simpler than
writing a "[0-9] [0-9][0-9] ..." pattern to match digits-only filenames.

> > > >  IMO no harm to have both sysfs way and ioctl way let user
> > > >  or developer use which were is suitable in their context.
> > > 
> > >    Extra maintenance overhead; divergence of the APIs. "You can find
> > > out the label of the filesystem using sysfs, but not the ioctl. You
> > > can find out the size of the filesystem using the ioctl, but not using
> > > sysfs." That way lies quite a bit of pain for the user of the
> > > interfaces.
> > 
> > I won't mind having several ways how to get the information, sysfs,
> > ioctl or the properties. Each way has it's own pros and cons or is
> > suitable for some type of user (C program, shell scripts).
> 
>    Fair enough. I do worry a little about the API divergence, though.

That's a valid point, given the number of data retrieved by the ioctl,
(I'm looking at v2 of Anand's patch https://patchwork.kernel.org/patch/3708901/)
this would need to make it extensible and backward compatible.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS
  2014-02-25 17:45                   ` David Sterba
@ 2014-02-26  2:34                     ` Anand Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2014-02-26  2:34 UTC (permalink / raw)
  To: dsterba, Hugo Mills, Goffredo Baroncelli, linux-btrfs



On 26/02/2014 01:45, David Sterba wrote:
> On Wed, Feb 12, 2014 at 04:15:55PM +0000, Hugo Mills wrote:
>> On Wed, Feb 12, 2014 at 05:01:23PM +0100, David Sterba wrote:
>>> On Fri, Feb 07, 2014 at 10:20:10AM +0000, Hugo Mills wrote:
>>>> For unmounted, but known filesystems:
>>>> /sys/fs/btrfs/devmap/<UUID>/<symlink to device>   (for each device known)
>>>> /sys/fs/btrfs/devmap/<UUID>/label
>>>> /sys/fs/btrfs/devmap/<UUID>/complete      (0 if missing devices, 1 otherwise)
>>>> /sys/fs/btrfs/devmap/<UUID>/<other properties>
>>>
>>> I like that, the device map represents the global state maintained by the
>>> module, does not interfere with the current sysfs structure.
>>>
>>> One minor comment, the symlink to device should have probably a fixed
>>> name so it can be easily located.
>>
>>     Two ways spring to mind: Number them according to the internal FS's
>> device numbers, so you'll have something like:
>>
>> $ ls -l /sys/fs/btrfs/devmap/3993e50e-a926-48a4-867f-36b53d924c35
>> total 0
>> lrwxrwxrwx 1 root root    0 Feb  1 11:22 0 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:0:0/0:0:0:0/block/sda
>> lrwxrwxrwx 1 root root    0 Feb  1 11:22 1 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:1:0/0:1:0:0/block/sdb
>> lrwxrwxrwx 1 root root    0 Feb  1 11:22 2 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:2:0/0:2:0:0/block/sdc
>> lrwxrwxrwx 1 root root    0 Feb  1 11:22 3 -> ../../../../devices/pci0000:00/0000:00:04.0/0000:02:00.0/ata1/host0/target0:3:0/0:3:0:0/block/sdd
>> -r--r--r-- 1 root root 4096 Feb  1 11:22 complete
>> -r--r--r-- 1 root root 4096 Feb  1 11:22 label
>>
>>     Then you can find them easily by enumerating the filenames, and
>> anything which is composed entirely of digits [0-9] is a device.
>> Alternatively, put them into a subdirectory, and anything at all
>> within that subdir is a device symlink.
>
> A separate subdirectory looks more user friendly, a simple subdir/*
> would get them all, regardless of the names. This is much simpler than
> writing a "[0-9] [0-9][0-9] ..." pattern to match digits-only filenames.


  Its better to have a btrfs sysfs layout posted (and update)
  at the wiki so that (I)/anybody-keen can attempt it (at a later
  point). Just would like to highlight that sysfs is end user
  visible, having tons-of-info /debug information would clutter.
  It doesn't have to match all the contents of BTRFS_IOC_GET_DEVS
  (For example BTRFS_IOC_GET_DEVS tells if bdev pointer is set,
  I doubt if this has to go into sysfs, knowing this info is
  just good for debugging).


>>>>>   IMO no harm to have both sysfs way and ioctl way let user
>>>>>   or developer use which were is suitable in their context.
>>>>
>>>>     Extra maintenance overhead; divergence of the APIs. "You can find
>>>> out the label of the filesystem using sysfs, but not the ioctl. You
>>>> can find out the size of the filesystem using the ioctl, but not using
>>>> sysfs." That way lies quite a bit of pain for the user of the
>>>> interfaces.
>>>
>>> I won't mind having several ways how to get the information, sysfs,
>>> ioctl or the properties. Each way has it's own pros and cons or is
>>> suitable for some type of user (C program, shell scripts).
>>
>>     Fair enough. I do worry a little about the API divergence, though.
>
> That's a valid point, given the number of data retrieved by the ioctl,
> (I'm looking at v2 of Anand's patch https://patchwork.kernel.org/patch/3708901/)
> this would need to make it extensible and backward compatible.

   BTRFS_IOC_GET_DEVS wasn't in the btrfs-next so backward compatible
   part should be ok to defer as of now .? In any case would make
   a frame-work for backward compatible.

Thanks, Anand

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-02-26  2:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-27  8:52 [PATCH] dump device list as seen by the kernel Anand Jain
2014-01-27  8:52 ` [PATCH] btrfs: introduce BTRFS_IOC_GET_DEVS Anand Jain
2014-01-27  8:47   ` Hugo Mills
2014-01-27  9:08     ` Anand Jain
2014-02-06 19:49     ` David Sterba
2014-02-06 20:09       ` Goffredo Baroncelli
2014-02-06 22:05         ` David Sterba
2014-02-07 10:08           ` Anand Jain
2014-02-07 10:20             ` Hugo Mills
2014-02-12 16:01               ` David Sterba
2014-02-12 16:15                 ` Hugo Mills
2014-02-25 17:45                   ` David Sterba
2014-02-26  2:34                     ` Anand Jain
2014-01-27  8:56 ` [PATCH] btrfs-progs: introduce btrfs-devlist Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.