On 6/29/19 12:28 PM, Richard W.M. Jones wrote:
Obvious change given the previous patch, so ACK.
I do wonder if we should make the notify parameter mandatory (and therefore avoid the duplicate functions).
Maybe. I've already posted the patch for nbdkit to use the notify parameter. But for other language bindings, the version without a notify parameter may still be easier to use.
Another trick you might do if feeling really keen is to programmatically generate the notify variants of commands, something like this:
let make_notify_variant_of_call ({ args; longdesc } as call) = let args = List.rev args in let flags, args = List.hd args, List.tl args in assert (flags = Flags "flags"); let args = flags :: CallbackPersist ("notify", [ etc ]) :: args in
It's a little trickier - the nbd_aio_pread_structured and nbd_aio_block_status already have an opaque parameter, but the other calls need to add an opaque.
let args = List.rev args in let longdesc = longdesc ^ "\nThe C<notify> callback blah blah ..." in { call with args; longdesc }
let aio_pwrite_call = { default_call with args = [ BytesPersistIn ("buf", "count"); UInt64 "offset"; Flags "flags" ]; ] (* etc copy the definition of aio_pwrite *) }
let aio_pwrite_notify_call = make_notify_variant_of_call aio_pwrite_call
let handle_calls = [ ... "aio_pwrite", aio_pwrite_call; "aio_pwrite_notify", aio_pwrite_notify_call; ... ]
I may still play with the idea of better auto-generation of things, but it's orthogonal enough to not hold up this patch going in. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org