[Libguestfs] [PATCH libnbd 2/9] generator: Generalize OFlags.
Eric Blake
eblake at redhat.com
Sat Aug 10 17:22:44 UTC 2019
On 8/10/19 8:02 AM, Richard W.M. Jones wrote:
> In a future commit we want to add (non-optional) Flags arg. As a step
> to doing this, generalize OFlags so it's not tied to just
> NBD_CMD_FLAG_*.
>
> This does not change the C API.
>
> It does introduce a small change to the OCaml API -- putting related
> flags into a submodule, basically renaming them. Note we don't
> provide guarantees for non-C APIs.
And even if we decide to provide guarantees for non-C, we're not at 1.0
yet ;)
> @@ -1387,7 +1403,7 @@ Returns the size in bytes of the NBD export."
> "pread", {
> default_call with
> args = [ BytesOut ("buf", "count"); UInt64 "offset" ];
> - optargs = [ OFlags "flags" ];
> + optargs = [ OFlags ("flags", cmd_flags) ];
Do we want to use this as a chance to document which flags a given API
supports? For example, pread supports DF but not FUA, REQ_ONE, or
MAY_TRIM. Then again, there's still a dynamic element - the API
supports the DF flag for compilation, but the server must also support
it (nbd_can_df) before you can use it. So any further restrictions we
decide to encode in the generator rather (or in addition) to
restrictions in lib/rw.c can be a later patch.
> +++ b/ocaml/tests/test_405_pread_structured.ml
> @@ -54,11 +54,13 @@ let () =
> NBD.pread_structured nbd buf 0_L (f 42);
> assert (buf = expected);
>
> - NBD.pread_structured nbd buf 0_L (f 42) ~flags:[NBD.cmd_flag_df];
> + let flags = let open NBD.CMD_FLAG in [DF] in
> +
> + NBD.pread_structured nbd buf 0_L (f 42) ~flags;
blank line here...
> +++ b/ocaml/tests/test_410_pwrite.ml
> @@ -33,7 +33,8 @@ let () =
> let nbd = NBD.create () in
> NBD.connect_command nbd ["nbdkit"; "-s"; "--exit-with-parent"; "-v";
> "file"; datafile];
> - NBD.pwrite nbd buf1 0_L ~flags:[NBD.cmd_flag_fua];
> + let flags = let open NBD.CMD_FLAG in [FUA] in
> + NBD.pwrite nbd buf1 0_L ~flags;
...but not here. It doesn't affect the code, but the consistency may be
worth it.
This and patch 1 look fine to me.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 484 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190810/e9520f86/attachment.sig>
More information about the Libguestfs
mailing list