[Libguestfs] [libnbd PATCH v2 4/5] api: Add STRICT_FLAGS to set_strict_mode

[Richard W.M. Jones]

On Thu, Sep 17, 2020 at 08:38:02AM -0500, Eric Blake wrote:
> On 9/17/20 8:32 AM, Richard W.M. Jones wrote:
> >On Fri, Sep 11, 2020 at 04:49:55PM -0500, Eric Blake wrote:
> >>The next strict knob: allow the user to pass unknown flags across the
> >>wire (this is different than passing a known flag at the wrong time).
> >>
> >>It is interesting to note that NBD only permits 16 bits of flags, but
> >>we have a signature that takes uint32_t; if we wanted, we could pack
> >>libnbd-specific flags in the upper bits that the NBD protocol would
> >>never see.
> >
> >This is fine ACK.
> >
> >About the "guard" change: I probably would have put that change in a
> >separate commit.  Also you could consider having a default_flags
> >parameter to avoid having to set guard = None everywhere, although you
> >would still have to make a one-off change to every *_flags.  It would
> >look like this:
> >
> >   let default_flags = { flag_prefix = ""; guard = None; flags = [] }
> >   ...
> >   let handshake_flags = {
> >     default_flags with
> >       flag_prefix = "HANDSHAKE_FLAG";
> >       flags = [ ... ]
> >   }
> >
> >(We already do this with "default_call").  Neither of these is a big deal
> >though.
> 
> Splitting the patch makes sense.  Also, I _did_ consider the
> 'default_flags with' solution before your reply, but only after I
> had written the email.  In the short term, it is no difference in
> the amount of churn (adding one line to every flags definition); but
> in the long term, it is nicer if we ever add more items.  So from
> that perspective, I'll go with that change.

Also to be aware of, OCaml has an annoying warning:

$ rlwrap ocaml
# type t = { foo : int; bar : int };;
type t = { foo : int; bar : int; }
# let default_t = { foo = 1; bar = 2 } ;;
val default_t : t = {foo = 1; bar = 2}
# { default_t with foo = 3 } ;;
- : t = {foo = 3; bar = 2}
# { default_t with foo = 3; bar = 4 } ;;
Warning 23: all the fields are explicitly listed in this record:
the 'with' clause is useless.
- : t = {foo = 3; bar = 4}

It's possible to disable the warning if you can assume a sufficiently
new OCaml, but in libguestfs I added a dummy field to struct instead:

https://github.com/libguestfs/libguestfs/blob/fce82fe55a2b64a1a7e494858aa272d608e5e54e/generator/structs.ml#L31

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org