On Mon, Sep 30, 2019 at 10:47:39AM +0200, Martin Kletzander wrote: > 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@xxxxxxxxxx> > > 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@xxxxxxxxxx> > > > > 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. The language consolidation is not about reducing languages as an absolute. Where we were using multiple langauges, to tackle problems in the same problem space, we consolidated on one language for consistency. This opens the door to adding use of other languages in libvirt, without continually increasing the burden of developers. The mix of Perl, Python & Shell all for doing misc build scripts was the prime example as there was nothing that can be done in Perl or SHell that could not be done just as well in Python. There's no justification for using all three languages in this context. In this case, our existing C language for writing our production code is not well suited for the task we're applying it too. You could make an argument that the virt-host-validate could be written in Python, but I think it is preferred to keep all our deployable code written in compiled languages, leaving the scripting languages for our supporting build system. Counting the addition of another language solely against this tool is not a reflection on the long term direction of libvirt code. It is highly likely that we'll incorporate Go code for more use cases over time as it is a good systems programming language, able to replace C where Python could not. The choice of YAML is a valid point to use as that is definitely adding a new language where one already exists (XML) that is expressive enough to cope with the problem. As you saw the original v1 did indeed use XML for this reason. The decision to try YAML was an experiment to see if the simplicity and readability of YAML would outweigh the cost of having another data language. Personally I think it is a net win to use YAML instead of XML, but it would not be the end of the world to go back to the v1 approach using XML if people really want a less readable language just to not have to add another data language. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list