[libvirt] [PATCH 02/21] admin: Check for flags properly

Martin Kletzander mkletzan at redhat.com
Thu Mar 10 10:27:42 UTC 2016


On Thu, Mar 10, 2016 at 09:53:59AM +0100, Erik Skultety wrote:
>On 10/03/16 05:53, Martin Kletzander wrote:
>> Function virAdmConnectListServers() forgot to check for flags at all,
>> virAdmConnectOpen() on the other hand checked them but did no dispatch
>> the error.  virCheckFlags() should be used only when there should be no
>> other thing done after erroring out and since they are used on different
>> places then just public API, they cannot dispatch errors.  So let' suse
>> virCheckFlagsGoto instead.
>>
>
>s/let' s/let's /
>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  src/libvirt-admin.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
>> index 54ae5ad3135e..44b4d4090e59 100644
>> --- a/src/libvirt-admin.c
>> +++ b/src/libvirt-admin.c
>> @@ -204,7 +204,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
>>
>>      VIR_DEBUG("flags=%x", flags);
>>      virResetLastError();
>> -    virCheckFlags(VIR_CONNECT_NO_ALIASES, NULL);
>> +    virCheckFlagsGoto(VIR_CONNECT_NO_ALIASES, error);
>>
>>      if (!(conn = virAdmConnectNew()))
>>          goto error;
>> @@ -603,7 +603,7 @@ int virAdmServerFree(virAdmServerPtr srv)
>>   * @conn: daemon connection reference
>>   * @servers: Pointer to a list to store an array containing objects or NULL
>>   *           if the list is not required (number of servers only)
>> - * @flags: bitwise-OR of virAdmConnectListServersFlags
>> + * @flags: unused, must be 0
>>   *
>
>you could as well make use of the description we use across libvirt:
>"extra flags; not used yet, so callers should always pass 0"
>

This was taken from other API so I kinda thought it'll be consistent.  I
will change libvirt-admin to use what other use (and you mentioned).
Thanks for catching that.

I changed both things in my local tree for now.

>>   * Collect list of all servers provided by daemon the client is connected to.
>>   *
>> @@ -624,6 +624,7 @@ virAdmConnectListServers(virAdmConnectPtr conn,
>>      VIR_DEBUG("conn=%p, servers=%p, flags=%x", conn, servers, flags);
>>
>>      virResetLastError();
>> +    virCheckFlagsGoto(0, error);
>>
>>      if (servers)
>>          *servers = NULL;
>>
>
>ACK
>
>Erik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160310/5553cbb8/attachment-0001.sig>


More information about the libvir-list mailing list