Re: [libvirt] [PATCH v2] util, qemu: Fix virDoes*Exist usage
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?
--
Andrea Bolognani / Red Hat / Virtualization