Re: [PATCH 10/13] block: introduce BLKREPORTZONESV2 ioctl

Christoph Hellwig
2025-11-03T10:24:05Z

On Mon, Nov 03, 2025 at 02:51:57PM +0900, Damien Le Moal wrote:
> On 11/1/25 01:52, Bart Van Assche wrote:
> >> + case BLKREPORTZONEV2:
> >> + if (rep.flags & ~BLK_ZONE_REPV2_INPUT_FLAGS)
> >> + return -EINVAL;
> >
> > -EINVAL probably should be changed into something that indicates "not
> > supported" rather than "invalid argument"?
>
> Not supported could be confused with the cached report zones not being
> supported. It is, but the user cannot specify input flags that are not defined.

Yes.

> This is to ensure that undefined flags are always 0 and that we can use these
> going forward when we need to define a new flag.
> So EINVAL seems appropriate to me.

Using EINVAL here is consistent with other APIs, but a bit of an
anti-feature. Not sure what another good more specific error code
would be, but given that we don't have other major EINVAL conditions
we might as well stick to it.

> >> + /* Input flags */
> >> + BLK_ZONE_REP_CACHED = (1 << 31),
> >> };
> >
> > Why 1 << 31 instead of 1 << 1?
>
> Why not ? That separates input and output flags instead of mixing them in
> tighter definition.

Agreed. Thinking about it, once you go up to bit 31 all of them
should be marked unsigned, though, i.e.,

.... = (1U << bitnr),