Re: [PATCH 17/19] ceph: ioctls

[Sage Weil <sage@newdream.net>]

On Thu, 23 Jul 2009, Andi Kleen wrote:
> Sage Weil <sage@newdream.net> writes:
> 
> > A few Ceph ioctls for getting and setting file layout (striping)
> > parameters.
> 
> It would be good if you posted manpages for these ioctls
> so that the interface can be reviewed. After all
> that's intended to be used by applications, isn't it?

Is there some existing manpage I might use as a reference?  e.g., an XFS 
ioctl manpage or something similar?

I'm also happy to replace these ioctls with a virtual xattr interface 
along the lines of what Andreas proposed.  That would make it much easier 
to maintain compatibility if the support layout parameters changes 
going forward.

> There don't seem to be compat ioctl handlers?

Oops,

diff --git a/src/kernel/file.c b/src/kernel/file.c
index fbf02c3..fbb5f94 100644
--- a/src/kernel/file.c
+++ b/src/kernel/file.c
@@ -810,5 +810,8 @@ const struct file_operations ceph_file_fops = {
        .splice_read = generic_file_splice_read,
        .splice_write = generic_file_splice_write,
        .unlocked_ioctl = ceph_ioctl,
+#ifdef CONFIG_COMPAT
+       .compat_ioctl   = ceph_ioctl,
+#endif
 };

The structs are all packed.

> > diff --git a/fs/ceph/ioctl.h b/fs/ceph/ioctl.h
> > new file mode 100644
> > index 0000000..537c27b
> > --- /dev/null
> > +++ b/fs/ceph/ioctl.h
> > @@ -0,0 +1,12 @@
> > +#ifndef FS_CEPH_IOCTL_H
> > +#define FS_CEPH_IOCTL_H
> > +
> > +#include <linux/ioctl.h>
> > +#include "types.h"
> > +
> > +#define CEPH_IOCTL_MAGIC 0x97
> > +
> > +#define CEPH_IOC_GET_LAYOUT _IOR(CEPH_IOCTL_MAGIC, 1, struct ceph_file_layout)
> > +#define CEPH_IOC_SET_LAYOUT _IOW(CEPH_IOCTL_MAGIC, 2, struct ceph_file_layout)
> 
> How should the application use that if the include file is in fs/ceph?
> Should be in include/linux I guess
> 
> Also this file should define all the types needed for the interface,
> especialy struct ceph_file_layout, but no kernel internal types.

The type is defined in ceph_fs.h, which is shared/synced with the userland 
code.  It's not specific to the ioctl interface.

My understanding (IIRC after reading comments from a recent fs merge) was 
that these sorts of headers normally shouldn't go in include/linux, since 
any userland (admin) progs will have their own version anyway while being 
built, and may not be synced with the installed kernel.  I also see 
autofs, btrfs, ext4, ocfs2, xfs ioctls defined in fs/*, not include/linux?

Thanks-
sage