Re: [PATCH v2] btrfs: introduce BTRFS_IOC_GET_DEVS

2014-02-25 Thread Anand Jain



On 26/02/2014 01:51, David Sterba wrote:

On Mon, Feb 24, 2014 at 07:48:45PM +0800, Anand Jain wrote:

The user land progs needs   a simple way to see
the raw list of disks and its parameters as seen
by the btrfs kernel.
As of now btrfs-devlist uses this ioctl.


As discussed before together with Hugo, the ioctl could exist parallel
to sysfs export of the device data. From that point it's easier to
maintain and update a set of sysfs files than an ioctl. The way you
suggest it right now lacks future-proof extensibility and backward
compatibility handling.

The userspace code is said to be a small and debug program, the sysfs
would IMHO satsify it needs as well, so I suggest not to continue on the
ioctl approach.



 sysfs interface has its advantage, but please lets not clutter
 it with the debug info as it just don't belong there. And one
 should be care full enough not to introduce regressions [1]
 when writing sysfs interfaces (sysfs has to be "managed"
 VS debug-friendly memory-dump/ioctl-dump).

 Next, btrfs-kernel device management isn't just ready
 fully[2] so that sysfs interface can be developed as
 of now. It better wait.

[1]
   https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg31432.html
   http://comments.gmane.org/gmane.comp.file-systems.btrfs/31915

[2] btrfs-kernel has no idea when disk disappears.
To get this done we appear to need more enhancements
related to the usage of kobjects with in kernel.

Thanks, Anand
--
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


Re: [PATCH v2] btrfs: introduce BTRFS_IOC_GET_DEVS

2014-02-25 Thread David Sterba
On Mon, Feb 24, 2014 at 07:48:45PM +0800, Anand Jain wrote:
> The user land progs needs   a simple way to see
> the raw list of disks and its parameters as seen
> by the btrfs kernel.
> As of now btrfs-devlist uses this ioctl.

As discussed before together with Hugo, the ioctl could exist parallel
to sysfs export of the device data. From that point it's easier to
maintain and update a set of sysfs files than an ioctl. The way you
suggest it right now lacks future-proof extensibility and backward
compatibility handling.

The userspace code is said to be a small and debug program, the sysfs
would IMHO satsify it needs as well, so I suggest not to continue on the
ioctl approach.
--
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


[PATCH v2] btrfs: introduce BTRFS_IOC_GET_DEVS

2014-02-24 Thread Anand Jain
From: Anand Jain 

The user land progs needs   a simple way to see
the raw list of disks and its parameters as seen
by the btrfs kernel.
As of now btrfs-devlist uses this ioctl.

Signed-off-by: Anand Jain 
---
v2: add more parameter to get from the kernel

 fs/btrfs/super.c   |  56 ++
 fs/btrfs/volumes.c | 140 +
 fs/btrfs/volumes.h |   2 +
 include/uapi/linux/btrfs.h |  49 
 4 files changed, 247 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index afb719e..f3c0247 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1717,6 +1717,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;
@@ -1801,6 +1854,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 253fd9f..3c44800 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6338,3 +6338,143 @@ 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;
+   struct btrfs_fs_devices *cur_fs_devices;
+
+   mutex_lock(&uuid_mutex);
+   list_for_each_entry(cur_fs_devices, &fs_uuids, list) {
+
+   fs_devices = cur_fs_devices;
+again_dev_cnt:
+   list_for_each_entry(device, &fs_devices->devices, dev_list)
+   cnt++;
+
+   fs_devices = fs_devices->seed;
+   if (fs_devices)
+   goto again_dev_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;
+   struct btrfs_fs_devices *cur_fs_devices;
+   struct btrfs_fs_devices *sprout_fs_devices;
+
+   mutex_lock(&uuid_mutex);
+   /* Todo: there must be better way of doing this */
+   list_for_each_entry(cur_fs_devices, &fs_uuids, list) {
+
+   mutex_lock(&cur_fs_devices->device_list_mutex);
+
+   fs_devices = cur_fs_devices;
+   sprout_fs_devices = NULL;
+
+again_dev:
+   list_for_each_entry(device, &fs_devices->devices, dev_list) {
+
+   if (!(cnt < alloc_cnt))
+   break;
+
+   memcpy(dev->fsid, fs_devices->fsid, BTRFS_FSID_SIZE);
+
+   if (fs_devices->seed)
+   memcpy(dev->seed_fsid, fs_devices->seed->fsid,
+   BTRFS_FSID_SIZE);
+   else
+   memset(dev->seed_fsid, 0, BTRFS_FSID_SIZE);
+
+   if (sprout_fs_devices)
+   memcpy(dev->sprout_fsid, 
sprout_fs_devices->fsid,
+   BTRFS_FSID_SIZE);
+   else
+   memset(dev->sp