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@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@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(-)
 create mode 100644 m4/virt-golang.m4
 create mode 100644 tools/host-validate/go.mod
 create mode 100644 tools/host-validate/go.sum
 create mode 100644 tools/host-validate/main.go
 create mode 100644 tools/host-validate/pkg/engine.go
 create mode 100644 tools/host-validate/pkg/facts.go
 rename tools/{virt-host-validate-bhyve.h => host-validate/pkg/facts_test.go} (52%)
 create mode 100644 tools/host-validate/rules/builtin.yaml
 create mode 100644 tools/host-validate/rules/cpu.yaml
 create mode 100644 tools/host-validate/rules/freebsd-kernel.yaml
 create mode 100644 tools/host-validate/rules/linux-acpi.yaml
 create mode 100644 tools/host-validate/rules/linux-cgroups.yaml
 create mode 100644 tools/host-validate/rules/linux-cpu-hardware-flaws.yaml
 create mode 100644 tools/host-validate/rules/linux-cpu.yaml
 create mode 100644 tools/host-validate/rules/linux-devices.yaml
 create mode 100644 tools/host-validate/rules/linux-iommu.yaml
 create mode 100644 tools/host-validate/rules/linux-namespaces.yaml
 create mode 100644 tools/host-validate/rules/linux-pci.yaml
 delete mode 100644 tools/virt-host-validate-bhyve.c
 delete mode 100644 tools/virt-host-validate-common.c
 delete mode 100644 tools/virt-host-validate-common.h
 delete mode 100644 tools/virt-host-validate-lxc.c
 delete mode 100644 tools/virt-host-validate-lxc.h
 delete mode 100644 tools/virt-host-validate-qemu.c
 delete mode 100644 tools/virt-host-validate-qemu.h
 delete mode 100644 tools/virt-host-validate.c

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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@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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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@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.

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Sep 30, 2019 at 15:48:29 +0100, Daniel Berrange wrote:
> 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:

[...]

> 
> 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.

In my opinion this is a misunderstanding.

Whether it's YAML or XML used for syntax is not that important. What is
important is that the new 'language' is the custom declarative language
which uses YAML or XML to express the constructions.

It's custom and barely documented. To get to the documentation you need
to read the source of the interpreter where you need to transform it
mentally to what it translates to.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Sep 30, 2019 at 05:21:51PM +0200, Peter Krempa wrote:
>On Mon, Sep 30, 2019 at 15:48:29 +0100, Daniel Berrange wrote:
>> 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:
>
>[...]
>
>>
>> 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.
>
>In my opinion this is a misunderstanding.
>
>Whether it's YAML or XML used for syntax is not that important. What is
>important is that the new 'language' is the custom declarative language
>which uses YAML or XML to express the constructions.
>
>It's custom and barely documented. To get to the documentation you need
>to read the source of the interpreter where you need to transform it
>mentally to what it translates to.

Yes, that is the more concerning one, thanks for formulating my thoughts better.
I really like YAML and I would definitely go for that if choosing a format for
similar purpose as yours.

It's like ansible.  YAML is a good fit there (debatably), but even if you know
python, YAML and jinja templating, you still need to learn ansible to actually
be able to write any YAML for it.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Daniel P. Berrangé <berrange@redhat.com> [2019-09-30, 03:48PM +0100]:
> 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. 

Any hard arguments on why that's preferential? I don't see a reasoning
as to not ship python code as many other project do just fine.

-- 
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 01, 2019 at 01:36:07PM +0200, Bjoern Walk wrote:
> Daniel P. Berrangé <berrange@redhat.com> [2019-09-30, 03:48PM +0100]:
> > 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. 
> 
> Any hard arguments on why that's preferential? I don't see a reasoning
> as to not ship python code as many other project do just fine.

The various core system tools which involve python have a history of
poor performance. For example, we previously used "firewall-cmd" for
talking to firewalld and it caused a massive performance hit, because
of all the time spent during startup processing imported modules. This
is seen again in OpenStack with its command line tools being so terribly
slow - it takes 1.5 seconds just to load the client & do nothing.

There's the classic difference of dynamic vs compiled languages where
with the former many basic syntax errors don't appear until you hit
them at runtime unless. Only mitigated if you invest a tonne of time
in unit testing every possible codepath.

Using compiled languages also enables us to share code between the
tools and library. This would enable us to actually embed the host
validation functionality into the virt driver, so its exposed as a
normal API in libvirt.

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 01, 2019 at 12:53:50PM +0100, Daniel P. Berrangé wrote:
>On Tue, Oct 01, 2019 at 01:36:07PM +0200, Bjoern Walk wrote:
>> Daniel P. Berrangé <berrange@redhat.com> [2019-09-30, 03:48PM +0100]:
>> > 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.
>>
>> Any hard arguments on why that's preferential? I don't see a reasoning
>> as to not ship python code as many other project do just fine.
>
>The various core system tools which involve python have a history of
>poor performance. For example, we previously used "firewall-cmd" for
>talking to firewalld and it caused a massive performance hit, because
>of all the time spent during startup processing imported modules. This
>is seen again in OpenStack with its command line tools being so terribly
>slow - it takes 1.5 seconds just to load the client & do nothing.
>
>There's the classic difference of dynamic vs compiled languages where
>with the former many basic syntax errors don't appear until you hit
>them at runtime unless. Only mitigated if you invest a tonne of time
>in unit testing every possible codepath.
>

Both are good points, but they are greyscale rather than black/white.  It
depends on the size and complexity of the script.  It also contradicts the use
of declarative language when you are offloading lot of the logic into the input
file.  I also think that both points are very much relevant to the build system
as well, just not that visible because not all parts of the build system are
used/modified by everyone.

>Using compiled languages also enables us to share code between the
>tools and library. This would enable us to actually embed the host
>validation functionality into the virt driver, so its exposed as a
>normal API in libvirt.
>
>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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Martin Kletzander <mkletzan@redhat.com> [2019-09-30, 10:47AM +0200]:
> On Fri, Sep 27, 2019 at 01:52:19PM +0100, Daniel P. Berrangé wrote:
> > 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.

I totally agree. Not only does this series fail to reduce the number of
languages of LOCs, it actually increases the complexity and
domain-specific knowledge to understand and contribute to libvirt.

> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


-- 
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, 2019-09-27 at 13:52 +0100, Daniel P. Berrangé wrote:
> 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.

I'll add my own 0.2 $currency to what others have said.

I think Go is a fine language to use for this kind of tool, so I'm
in favor of that; having a more granular view into the details of
the system also looks like a good idea.

What I'm not sold on is the advantage of a YAML-driven approach:
it seems to me that the same result could be achieved much more
conveniently using regular Go code instead.

Perhaps it would be useful if you explained in detail why you
decided to take this approach in the first place.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Sep 30, 2019 at 12:03:22PM +0200, Andrea Bolognani wrote:
> On Fri, 2019-09-27 at 13:52 +0100, Daniel P. Berrangé wrote:
> > 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.
> 
> I'll add my own 0.2 $currency to what others have said.
> 
> I think Go is a fine language to use for this kind of tool, so I'm
> in favor of that; having a more granular view into the details of
> the system also looks like a good idea.
> 
> What I'm not sold on is the advantage of a YAML-driven approach:
> it seems to me that the same result could be achieved much more
> conveniently using regular Go code instead.
> 
> Perhaps it would be useful if you explained in detail why you
> decided to take this approach in the first place.

I've essentially answered this in my response to Martin's comment
in https://www.redhat.com/archives/libvir-list/2019-September/msg01419.html
so to avoid splitting the discussion I won't repeat it here.


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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list