On Thu, Nov 19, 2015 at 10:52:35AM +0100, Erik Skultety wrote:
On 16/11/15 17:42, Martin Kletzander wrote:const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -71,5 +75,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CONNECT_CLOSE = 2 + ADMIN_PROC_CONNECT_CLOSE = 2, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3I was wondering why 'connect' is here, it does not necessarily relate to connection and makes the name long, we could start using 'daemon' instead as that is what we'll need to add anyway. Also 'lib' seems unnecessary here.Well, this is a matter of consistency with libvirt library. You also mentioned the general API naming convention that should be met according to the arguments the API takes in one of your earlier reviews [1]. The 'connect' is there because virAdmConnectGetLibVersion indeed takes virAdmConnectPtr as its 1st argument and the remote version name of the API imho should not differ that much.
We want to stay consistent with libvirt, but not with all the wrong choices that were made along the way. I wish I named the initial pointer the user gets with virAdmConnect() something like virAdmDaemonPtr, everything you call with that would start with virAdmDaemon and would make tad more sense. But I didn't know there will be a "daemon" back then. Having said that, we *can* change that, but I already confused myself as to what is the best naming convention we should go with. Wider discussion about that would be fine and I think we would achieve a conclusion in less than 5 minutes if there's more than the two of us and it's not done through mail. So for now keep it as it is, and we'll add the discussion about naming to the TODO list of things needed to be done before enabling the admin api in the daemon.
Erik [1] https://www.redhat.com/archives/libvir-list/2015-September/msg00079.html
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list