Re: [PATCH 5/5] sed-opal: Add command to read locking range parameters.

Luca Boccassi
2023-04-05T10:53:34Z

On Wed, 5 Apr 2023 at 10:39, Ondrej Kozina <okozina@redhat.com> wrote:
>
> On 05. 04. 23 10:27, Christian Brauner wrote:
> > On Wed, Mar 22, 2023 at 04:16:04PM +0100, Ondrej Kozina wrote:
> >> diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
> >> index d7a1524023db..3905c8ffedbf 100644
> >> --- a/include/uapi/linux/sed-opal.h
> >> +++ b/include/uapi/linux/sed-opal.h
> >> @@ -78,6 +78,16 @@ struct opal_user_lr_setup {
> >> struct opal_session_info session;
> >> };
> >>
> >> +struct opal_lr_status {
> >> + struct opal_session_info session;
> >> + __u64 range_start;
> >> + __u64 range_length;
> >> + __u32 RLE; /* Read Lock enabled */
> >> + __u32 WLE; /* Write Lock Enabled */
> >
> > Why is that in capital letters if I may ask? That seems strange uapi for
> > Linux. And why not just "read_lock_enabled" and "write_lock_enabled"
> > given that we also have "range_start" and "range_length". Let's not
> > CREAT one of those weird uapis if we don't have to.
>
> See 'opal_user_lr_setup' struct above. Since the new command is supposed
> to return those parameters I did not want to add confusion by naming it
> differently.
>
> >
> >> + __u32 l_state;
> >
> > "locking_state"?
>
> Same as above, see 'opal_lock_unlock' struct. It's even spicier
> considering it's impossible to set WRITE_ONLY state (lock only read I/O)
> with sed-opal iface.

Yeah we really want to keep the parameters names the same across the
various ioctl, otherwise it's going to get very confusing very
quickly.