[Libguestfs] [libnbd PATCH] api: Add way to avoid structured replies

Eric Blake eblake at redhat.com
Thu Sep 5 11:29:34 UTC 2019


On 9/5/19 2:40 AM, Richard W.M. Jones wrote:

> I have one comment unrelated to the patch:
> 
>> +  "set_request_structured_replies", {
>> +    default_call with
>> +    args = [Bool "request"]; ret = RErr;
>> +    permitted_states = [ Created ];
>> +    first_version = (1, 2);
> 
> I just know that we're going to end up adding new APIs and forgetting
> to set the first_version field.  There are various things we could do
> to prevent this:
> 
> (1) In ‘default_call’ set first_version = (0, 0).  Update all existing
> calls with first_version = (1, 0).  Then add a check that
> first_version <> (0, 0).  There's still a danger of copy and paste
> from an existing API, but we should be able to catch that in review.

Or maybe couple that with a declaration that maps version (1, 0) has X
signatures and version (1, 2) has Y releases, then if we detect a count
mismatch, it must be a new addition incorrectly versioned.

> 
> (2) Store first_version in a separate table.  Add checks to ensure the
> new table exhaustively covers all APIs.  It should be obvious when
> submitting a new API that the first_version table must be updated and
> what to add here:
> 
>   let first_version = [
>     "set_debug", (1, 0);
>     ...
>     "set_request_structured_replies", (1, 2);
>     "get_request_structured_replies", (1, 2);
>   ]

This approach also seems fine (it's a bit closer to maintaining the
.syms file by hand, but with the compiler guaranteeing that we touch
.syms for everything we add).  I lean slightly towards option 2.

-- 
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: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190905/16a17fdb/attachment.sig>


More information about the Libguestfs mailing list