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
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs