On Wed, Sep 29, 2010 at 12:48:15AM +0200, Goffredo Baroncelli wrote:
> Hi Josef,
>
> please, if you add or change a command pay attention to update the man page
> too (man/btrfs.8.in).
>
Hrm I missed that with git add, I'll fix that up.
> See my comments below
>
> On Tuesday, 28 September, 2010, Josef Bacik wrote:
> > This command just implements the BTRFS_IOC_DISK_INFO ioctl. It gets all the
> > devices for a filesystem, reads /proc/partitions for the right dev info and
> > spits out pretty names.
> >
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxx>
> > ---
> > btrfs.c | 4 ++
> > btrfs_cmds.c | 101
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > btrfs_cmds.h | 1 +
> > ioctl.h | 8 ++++
> > 4 files changed, 114 insertions(+), 0 deletions(-)
> >
> > diff --git a/btrfs.c b/btrfs.c
> > index ab5e57f..22b241b 100644
> > --- a/btrfs.c
> > +++ b/btrfs.c
> > @@ -91,6 +91,10 @@ static struct Command commands[] = {
> > "filesystem df", "<path>\n"
> > "Show space usage information for a mount point\n."
> > },
> > + { do_disk_info, 1,
> > + "filesystem disk-info", "<path>\n"
> > + "Show disks contained within this filesysten."
> > + },
> > { do_balance, 1,
> > "filesystem balance", "<path>\n"
> > "Balance the chunks across the device."
> > diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> > index 683aec0..b1a4d8d 100644
> > --- a/btrfs_cmds.c
> > +++ b/btrfs_cmds.c
> > @@ -835,6 +835,107 @@ int do_set_default_subvol(int nargs, char **argv)
> > return 0;
> > }
> >
> > +int do_disk_info(int nargs, char **argv)
> > +{
> > + struct btrfs_ioctl_disk_info_args *args;
> > + char *path = argv[1];
> > + int devs;
> > + int fd;
> > + int ret;
> > + int i;
> > +
> > + fd = open_file_or_dir(path);
> > + if (fd < 0) {
> > + fprintf(stderr, "Error: can't access '%s'\n", path);
> > + return 12;
> > + }
> > +
> > + args = malloc(sizeof(*args));
> > + if (!args)
> > + return -ENOMEM;
> > +
> > + args->num_devices = 0;
> > +
> > + ret = ioctl(fd, BTRFS_IOC_DISK_INFO, args);
> > + if (ret) {
> > + printf("failed ioctl\n");
> Please use
> fprintf(stderr,"ERROR: failed ioctl\n");
>
Ugh oops sorry, forgot to get rid of my debug printing, I'll kill that.
> > + free(args);
> > + return ret;
> > + }
> > +
> > + if (!args->num_devices) {
> > + printf("no devs\n");
> > + free(args);
> > + return 0;
> > + }
> > +
> > + devs = args->num_devices;
> > + args = realloc(args, sizeof(*args) + (devs * sizeof(dev_t)));
> > + if (!args) {
> > + printf("enomem\n");
> Please use
> fprintf(stderr,"ERROR: not enough memory\n");
Yeah another debug print.
> > + return -ENOMEM;
> > + }
> > + args->num_devices = devs;
> > +
> > + ret = ioctl(fd, BTRFS_IOC_DISK_INFO, args);
> > + if (ret) {
> > + printf("ioctl failed second time\n");
> Please use
> fprintf(stderr,"ERROR: ioctl failed (second time)\n");
>
> > + free(args);
> > + return -ENOMEM;
> > + }
> > + close(fd);
> > +
> > + fd = open_file_or_dir("/proc/partitions");
>
> Why not
>
> f = fopen("/proc/partitions","r")
>
> So you can avoid "f = fdopen(fd,"r")" and the "goto print"
>
> > + if (fd < 0) {
> > +print:
> > + printf("Devices in %s\n", path);
> > + for (i = 0; i < args->num_devices; i++)
> > + printf("\t%d:%d\n", major(args->devices[i]),
> > + minor(args->devices[i]));
> > + close(fd);
> ^^^^^^^^^
> why close fd ? ... ok, in case of fdopen(fd,"r") == 0
>
Yeah I didn't think this through well, I'll fix it up. I mostly just sent this
as an afterthought. When we figure out how the interface is going to look I'll
fix everything up and re-send. Thanks,
Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
