[libvirt] [PATCH v2] util, qemu: Fix virDoes*Exist usage

Martin Kletzander mkletzan at redhat.com
Mon Nov 19 21:22:18 UTC 2018


On Mon, Nov 19, 2018 at 05:27:01PM +0100, Andrea Bolognani wrote:
>On Mon, 2018-11-19 at 15:10 +0100, Martin Kletzander wrote:
>> Since the functions only return 0 or 1, they should return bool (missed the
>> change in the first commit).  That way it's clearer that the check for
>> non-existing group should be either "== 0" instead.
>
>I don't get this part of the commit message... With the original
>implementation, calling virDoesGroupExist() on a non-existing
>group would have returned 0, correct? And now it will return false
>instead...
>
>Oh, I think I see what you mean: the original code expected the
>integer return value to follow the usual libvirt conventions where
>0 means success and <0 means failure, but in this case the functions
>returned 1 on success and 0 on failure, so the logic in
>virQEMUDriverConfigPtr() was wrong. Gotcha.
>
>So, here's what I would do: I would split this into two patches, the
>first one which contains only the first hunk - it will work even if
>the return types are int - and fixes the bug, and the second one
>which contains the remaining hunks and makes usage of the functions
>more obvious and hopefully prevents more bugs from slipping in.
>
>Does that sound reasonable?
>

Brutal honesty?  No.  For a patch that I would consider trivial I think it's a
waste of time, but nevertheless I'll do it.

>--
>Andrea Bolognani / Red Hat / Virtualization
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181119/ae2f6455/attachment-0001.sig>


More information about the libvir-list mailing list