[libvirt] [PATCH v2 0/6] rewrite virt-host-validate to be data driven, using Go & YAML
Martin Kletzander
mkletzan at redhat.com
Mon Sep 30 08:47:39 UTC 2019
On Fri, Sep 27, 2019 at 01:52:19PM +0100, Daniel P. Berrangé wrote:
>This is a followup to a previous PoC patch I submitted a
>month ago:
>
> https://www.redhat.com/archives/libvir-list/2019-September/msg00036.html
>
>The commit messages in the individual patches given quite a
>bit of detail, so I'll keep this cover letter brief.
>
>In my previous posting I was unhappy with the implications for
>the RPM packaging, and was considering having this as a separate
>source repo & RPM. On further investigation such an approach
>would not in fact solve the RPM packaging problem, because we
>would still not be using a pure go build toolchain, as we have
>data files that need installing in the right place.
>
>This forced me to actually address the RPM packaging problems
>that Fedora had with Go when used from a build tool like make
>or meson.
>
>After alot of debugging I finally got a viable solution merged
>into the Fedora go-rpm-macros package:
>
> https://pagure.io/go-rpm-macros/c/67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2
>
> commit 67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2
> Author: Daniel P. Berrangé <berrange at redhat.com>
> Date: Wed Sep 18 16:49:58 2019 +0100
>
> macros: define a %gobuildflags macro
>
> Using the %gobuild macro is fine for a project where the go
> code is the only thing being built, and can be built directly
> by invoking the Go toolchain from RPM.
>
> In more complex cases though, the Go code is just a small part
> of the project and the Go toolchain is invoked by a build
> system such as make (possibly automake), or meson. In such a
> case we need to be able to tell this build system what flags
> to pass to the compiler.
>
> The %gobuildflags macros services this purpose allowing a
> RPM spec todo
>
> GOBUILDFLAGS="%gobuildflags" %configure
>
> or
>
> %make GOBUILDFLAGS="%gobuildflags"
>
> Ideally the %gobuild macro would in turn reference the
> %gobuildflags macro, but that does not appear possible
> given the semantics around quote expansion and escaping
> across RPM and shell.
>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>
>As a result in this series, we're now fully integrated into the
>RPM build, on Fedora at least. I've not checked what approach
>RHEL takes for Go, whether it requires separate RPM for each
>3rd party dep, or prefers bundling. Either way though, we can
>deal with the problem now.
>
>The other obvious change is that this is now a patch series,
>to make it easier to review the code in managable chunks.
>
>The really big difference though is that I replaced the use
>of XML data files with YAML data files. This was done with
>the aim of making the data more human friendly. XML is really
>optimized for machines, not humans, so writing the data files
>was not pretty. YAML is optimized for human readability, and
>is actually even easier to consume in Go than the XML was,
>so its a double win.
>
>Finally, we also add new checks at the end for the various
>CPU hardware side channel mitigations, and report whether
>SMT/HT is unsafe or not (any Intel host is basically unsafe
>before Icelake).
>
>Daniel P. Berrangé (6):
> build: introduce logic for using golang in libvirt
> tools: introduce a data driven impl of virt-host-validate
> tools: define YAML rules for virt-host-validate checks
> tools: switch to build the new virt-host-validate impl
> tools: delete the old virt-host-validate impl
> tools: make virt-host-validate check CPU vulnerabilities
>
> configure.ac | 1 +
> libvirt.spec.in | 35 +-
> m4/virt-golang.m4 | 46 ++
> m4/virt-host-validate.m4 | 8 +-
> po/POTFILES | 5 -
> tools/Makefile.am | 76 +--
> tools/host-validate/go.mod | 10 +
> tools/host-validate/go.sum | 9 +
> tools/host-validate/main.go | 98 +++
> tools/host-validate/pkg/engine.go | 481 ++++++++++++++
> tools/host-validate/pkg/facts.go | 585 ++++++++++++++++++
> .../pkg/facts_test.go} | 36 +-
> tools/host-validate/rules/builtin.yaml | 20 +
> tools/host-validate/rules/cpu.yaml | 50 ++
> tools/host-validate/rules/freebsd-kernel.yaml | 77 +++
> tools/host-validate/rules/linux-acpi.yaml | 39 ++
> tools/host-validate/rules/linux-cgroups.yaml | 470 ++++++++++++++
> .../rules/linux-cpu-hardware-flaws.yaml | 165 +++++
> tools/host-validate/rules/linux-cpu.yaml | 134 ++++
> tools/host-validate/rules/linux-devices.yaml | 71 +++
> tools/host-validate/rules/linux-iommu.yaml | 113 ++++
> .../host-validate/rules/linux-namespaces.yaml | 119 ++++
> tools/host-validate/rules/linux-pci.yaml | 10 +
> tools/virt-host-validate-bhyve.c | 77 ---
> tools/virt-host-validate-common.c | 419 -------------
> tools/virt-host-validate-common.h | 85 ---
> tools/virt-host-validate-lxc.c | 87 ---
> tools/virt-host-validate-lxc.h | 24 -
> tools/virt-host-validate-qemu.c | 116 ----
> tools/virt-host-validate-qemu.h | 24 -
> tools/virt-host-validate.c | 152 -----
> tools/virt-host-validate.pod | 12 +-
> 32 files changed, 2609 insertions(+), 1045 deletions(-)
So this ^^ plus:
2 languages added, 0 languages removed
makes me feel like this goes against what you were trying to do in another
series. I understand that adding new fact checks is "easier" and does not
require recompilation, but I don't see any use for that benefit. I went through
the cover letters for both series just to find a reason for it and I didn't.
Sorry, but I don't really like this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190930/6db04430/attachment-0001.sig>
More information about the libvir-list
mailing list