Re: [PATCH v3 1/1] nvme : Add ioctl to query nvme attributes

[Joel Granados <j.granados@samsung.com>]

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

On Sat, Nov 19, 2022 at 12:04:43AM +0000, Chaitanya Kulkarni wrote:
> On 11/18/22 14:35, Keith Busch wrote:
> > On Thu, Nov 17, 2022 at 03:13:37PM +0100, Joel Granados wrote:
> >> +#define NVME_IOCTL_GET_ATTR	_IOWR('N', 0x50, struct nvme_get_attr)
> > 
> > Isn't the ioctl's number derived from the sizeof the parameter? IOW, if
> > you add new fields to the struct in the future, the ioctl number will
> > change, breaking older applications that were using it. I don't think we
> > want to keep adding new ioctl's to support legacy users, so probably
> > padding that struct with plenty of reserved space is the way to go.
> 
> This is exactly I've asked explicitly to make the size of the structure
> close to nvme command size (or even better to nvme identify controller
> data structure size) so it will be future proof and we can add new
> parameters.
> 
> Instead the rsvd field seems to be removed in this patch.

By including the size of the struct in the ioctl we do not need to
pad it. We can add all the struct members that we want in the future as
long as we do not remove any that already existed. In the same way that
it is implemented in https://github.com/torvalds/linux/blob/master/include/uapi/linux/vfio.h.

> 
> -ck
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]