2011/5/10 Daniel P. Berrange <berrange@xxxxxxxxxx>: > On Mon, May 09, 2011 at 03:45:29PM -0600, Eric Blake wrote: >> On 05/07/2011 06:28 AM, Matthias Bolte wrote: >> > --- >> > Âdaemon/Makefile.am         |  20 ++++- >> > Âdaemon/qemu_dispatch.blacklist   |  Â3 + >> > Âdaemon/qemu_dispatch.whitelist   |  Â1 + >> > Âdaemon/remote_dispatch.blacklist  |  37 ++++++++ >> > Âdaemon/remote_dispatch.whitelist  | Â169 +++++++++++++++++++++++++++++++++++ >> > Âdaemon/remote_generator.pl     | Â171 +++++++++++++----------------------- >> > Âsrc/Makefile.am          Â|  24 ++++- >> > Âsrc/remote/qemu_client.blacklist  |  Â3 + >> > Âsrc/remote/qemu_client.whitelist  |  Â1 + >> > Âsrc/remote/remote_client.blacklist |  47 ++++++++++ >> > Âsrc/remote/remote_client.whitelist | Â159 +++++++++++++++++++++++++++++++++ >> >> Hmm. ÂGiven the difference in sizes between >> daemon/remote_dispatch.whitelist and src/remote/remote_client.whitelist, >> there are some functions where we are only doing half the job? ÂThat >> means every new API has to touch two, rather than one, file, and that's >> out of a choice of four files. >> >> Maybe a better thing to do would be having a single file, that lists >> every API, along with two states, as in: >> >> >> # name  daemon Âsrc/remote >> function yes   no >> >> In fact, rather than maintaining separate files, could we instead >> maintain this list directly in {remote,qemu}_protocol.x, via stylized >> comments? >> >> enum remote_procedure { >>   /* Each function must have a two-word comment. ÂThe first word is >>   Â* whether remote_generator.pl handles daemon, the second whether >>   Â* it handles src/remote. Â*/ >>   REMOTE_PROC_OPEN = 1, /* yes no */ >> ... >> >> That way, when we add a new API, we are _already_ editing the file that >> contains the white/blacklist, and have the precedence of the lines >> beforehand to remind us whether we need to write manual code or rely on >> the generator. >> >> Although I think that this patch does a good job as-is, I think it is >> worth a v2 that avoids the extra files (the fewer files you have to edit >> when adding a new API, the better). > > Having annotations in the .x is a nice idea. We could also annotate the > methods with 'readonly' and 'readwrite' keywords, and use that to auto > generate some readonly ACL checks in the dispatch code as an extra layer > of defence. Do you mean checks regarding the VIR_CONNECT_RO flag? > So instead of yes|no, how about just "skipgen|autogen" Yep, more explicit, I'll go with that. Matthias -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list