Re: [PATCH v3 10/11] admin: Introduce virAdmConnectGetLibVersion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 = 3
> 
> I 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.

Erik

[1] https://www.redhat.com/archives/libvir-list/2015-September/msg00079.html

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]