[libvirt] [PATCH 0/2] Couple of virt-host-validate fixes

While preparing my talk for DevConf, where I want to present this awesome tool I've found couple of bugs in it. Michal Privoznik (2): virt-host-validate: Check those CGroups that we actually use virt-host-validate: Fix error level for user namespace check tools/virt-host-validate-lxc.c | 18 +++++++++--------- tools/virt-host-validate-qemu.c | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) -- 2.4.10

Since the introduction of virt-host-validate tool the set of cgroup controllers we use has changed so the tool is checking for some cgroups that we don't need (e.g. net_cls, although I doubt we have ever used that one) and is not checking for those we actually use (e.g. cpuset). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virt-host-validate-lxc.c | 16 ++++++++-------- tools/virt-host-validate-qemu.c | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/virt-host-validate-lxc.c b/tools/virt-host-validate-lxc.c index 48323da..e604f83 100644 --- a/tools/virt-host-validate-lxc.c +++ b/tools/virt-host-validate-lxc.c @@ -78,19 +78,19 @@ int virHostValidateLXC(void) "CGROUP_CPUACCT") < 0) ret = -1; + if (virHostValidateCGroupController("LXC", "cpuset", + VIR_HOST_VALIDATE_FAIL, + "CPUSETS") < 0) + ret = -1; + if (virHostValidateCGroupController("LXC", "devices", VIR_HOST_VALIDATE_FAIL, "CGROUP_DEVICE") < 0) ret = -1; - if (virHostValidateCGroupController("LXC", "net_cls", - VIR_HOST_VALIDATE_WARN, - "NET_CLS_CGROUP") < 0) - ret = -1; - - if (virHostValidateCGroupController("LXC", "freezer", - VIR_HOST_VALIDATE_WARN, - "CGROUP_FREEZER") < 0) + if (virHostValidateCGroupController("LXC", "blkio", + VIR_HOST_VALIDATE_FAIL, + "BLK_CGROUP") < 0) ret = -1; return ret; diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c index 044df65..a9f6c1e 100644 --- a/tools/virt-host-validate-qemu.c +++ b/tools/virt-host-validate-qemu.c @@ -74,16 +74,16 @@ int virHostValidateQEMU(void) "CGROUP_CPUACCT") < 0) ret = -1; + if (virHostValidateCGroupController("QEMU", "cpuset", + VIR_HOST_VALIDATE_WARN, + "CPUSETS") < 0) + ret = -1; + if (virHostValidateCGroupController("QEMU", "devices", VIR_HOST_VALIDATE_WARN, "CGROUP_DEVICES") < 0) ret = -1; - if (virHostValidateCGroupController("QEMU", "net_cls", - VIR_HOST_VALIDATE_WARN, - "NET_CLS_CGROUP") < 0) - ret = -1; - if (virHostValidateCGroupController("QEMU", "blkio", VIR_HOST_VALIDATE_WARN, "BLK_CGROUP") < 0) -- 2.4.10

On Mon, Jan 25, 2016 at 08:23:21AM +0100, Michal Privoznik wrote:
Since the introduction of virt-host-validate tool the set of cgroup controllers we use has changed so the tool is checking for some cgroups that we don't need (e.g. net_cls, although I doubt we have ever used that one) and is not checking for those we actually use (e.g. cpuset).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virt-host-validate-lxc.c | 16 ++++++++-------- tools/virt-host-validate-qemu.c | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-)
ACK

From the code it seems to me that we need user namespace iff configured in domain XML. Otherwise we don't use it at all. However our tool is more strict about that. Fix this discrepancy.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virt-host-validate-lxc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virt-host-validate-lxc.c b/tools/virt-host-validate-lxc.c index e604f83..89a6388 100644 --- a/tools/virt-host-validate-lxc.c +++ b/tools/virt-host-validate-lxc.c @@ -59,7 +59,7 @@ int virHostValidateLXC(void) ret = -1; if (virHostValidateNamespace("LXC", "user", - VIR_HOST_VALIDATE_FAIL, + VIR_HOST_VALIDATE_WARN, _("User namespace support is recommended")) < 0) ret = -1; -- 2.4.10

On Mon, Jan 25, 2016 at 08:23:22AM +0100, Michal Privoznik wrote:
From the code it seems to me that we need user namespace iff
s/>From/From/ s/iff/if/
configured in domain XML. Otherwise we don't use it at all. However our tool is more strict about that. Fix this discrepancy.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virt-host-validate-lxc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK

On Mon, 2016-01-25 at 16:40 +0100, Pavel Hrdina wrote:
On Mon, Jan 25, 2016 at 08:23:22AM +0100, Michal Privoznik wrote:
From the code it seems to me that we need user namespace iff s/>From/From/
This is fun. The leading ">" is an artifact of escaping, to prevent (poorly written?) MUAs from mistaking a line starting with "From" in the body for the From: header. You would think 'git format-patch', and most importantly 'git am', would be clever enough to take this into account, but alas they don't and just pass the commit message through.
s/iff/if/
Not a typo: "iff" is shortand for "if and only if", and by doing a quick search in the git log you'll find that there are quite a few previous instances. Mostly introduced by Michal :) Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (3)
-
Andrea Bolognani
-
Michal Privoznik
-
Pavel Hrdina