2011/5/10 Daniel P. Berrange <berra...@redhat.com>:
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
