Re: [PATCH v5 17/17] vfs: expose delegation support to userland

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2025-11-11 at 11:48 +0100, Jan Kara wrote:
> On Wed 05-11-25 11:54:03, Jeff Layton wrote:
> > Now that support for recallable directory delegations is available,
> > expose this functionality to userland with new F_SETDELEG and F_GETDELEG
> > commands for fcntl().
> > 
> > Note that this also allows userland to request a FL_DELEG type lease on
> > files too. Userland applications that do will get signalled when there
> > are metadata changes in addition to just data changes (which is a
> > limitation of FL_LEASE leases).
> > 
> > These commands accept a new "struct delegation" argument that contains a
> > flags field for future expansion.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> For new apis CCing linux-api is a good practice ;)
> 
> ...
> 

Doh! I definitely will on the next posting.

> > diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> > index 3741ea1b73d8500061567b6590ccf5fb4c6770f0..8123fe70e03cfb1ba9ce1b5e20d61b62e462a7ea 100644
> > --- a/include/uapi/linux/fcntl.h
> > +++ b/include/uapi/linux/fcntl.h
> > @@ -79,6 +79,16 @@
> >   */
> >  #define RWF_WRITE_LIFE_NOT_SET	RWH_WRITE_LIFE_NOT_SET
> >  
> > +/* Set/Get delegations */
> > +#define F_GETDELEG		(F_LINUX_SPECIFIC_BASE + 15)
> > +#define F_SETDELEG		(F_LINUX_SPECIFIC_BASE + 16)
> > +
> > +/* Argument structure for F_GETDELEG and F_SETDELEG */
> > +struct delegation {
> > +	unsigned int	d_flags;	/* Must be 0 */
> > +	short		d_type;		/* F_RDLCK, F_WRLCK, F_UNLCK */
> > +};
> > +
> 
> I think it would make sense for d_type to be unsigned since it's more or
> less enum.
> 

FWIW, struct flock has the l_type as a signed short, so that's why I
copied it here. Making it unsigned is better though.

> Also struct delegation is going to have a hole in it at the end
> which is always a concern with uAPI structures (passing around
> uninitialized stuff). I think it would be good to put an explicit padding
> there and enforce it is zeroed out.
> 

Makes sense. I'll incorporate that too.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>