[igt-dev] [PATCH i-g-t 2/2] i915/kms_chamelium: Pull chameleond logs for debugging

[Summers, Stuart]

On Thu, 2019-02-21 at 17:37 +0200, Arkadiusz Hiler wrote:
> On Thu, Feb 14, 2019 at 04:40:43PM -0800, Stuart Summers via igt-dev
> wrote:
> > IGT currently logs some state information when passing commands
> > to the Chamelium. There is a daemon running on the Chamelium
> > itself,
> > chameleond, which logs some additional information locally. Add
> > a new set of libssh calls to extract those logs to be used in CI
> > debug.
> > 
> > Signed-off-by: Stuart Summers <stuart.summers at intel.com>
> > ---
> >  Dockerfile.debian   |   1 +
> >  Dockerfile.fedora   |   1 +
> >  lib/igt_chamelium.c | 150
> > +++++++++++++++++++++++++++++++++++++++++++-
> >  lib/meson.build     |   4 ++
> >  meson.build         |   4 +-
> >  5 files changed, 158 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Dockerfile.debian b/Dockerfile.debian
> > index b9c3be394..77928a9ef 100644dy have another way of talking to
> > chameli
> > --- a/Dockerfile.debian
> > +++ b/Dockerfile.debian
> > @@ -18,6 +18,7 @@ RUN apt-get install -y \
> >  			libudev-dev \
> >  			libgsl-dev \
> >  			libasound2-dev \
> > +			libssh-dev \
> >  			libxmlrpc-core-c3-dev \
> >  			libjson-c-dev \
> >  			libcurl4-openssl-dev \
> > diff --git a/Dockerfile.fedora b/Dockerfile.fedora
> > index 08a4bd224..b041bf252 100644
> > --- a/Dockerfile.fedora
> > +++ b/Dockerfile.fedora
> > @@ -16,6 +16,7 @@ RUN dnf install -y gcc \
> >  		json-c-devel \
> >  		libdrm-devel \
> >  		libudev-devel \
> > +		libssh-devel \
> >  		xmlrpc-c-devel \
> >  		elfutils-devel \
> >  		libunwind-devel \
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index 32b859eac..61b563e08 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -30,6 +30,7 @@
> >  #include <errno.h>
> >  #include <xmlrpc-c/base.h>
> >  #include <xmlrpc-c/client.h>
> > +#include <libssh/libssh.h>
> 
> The logs looks super useful to have, indeed!dy have another way of
> talking to chameli
> 
> Pulling ssh library and adding all the complexity, especially when we
> already have another way of talking to chamelium, seems a little too
> much though.
> 
> Have you looked into how complex it would be to add a XMLRPC call
> to fetch them instead?
> 

Yeah I thought about that...

Basically I'm thinking we have a few options:
1) Do nothing. Don't get the logs.
2) Implement C-based SSH. Obviously there is the code complexity.
3) Implement shell-based SSH. This seems contradictory to the rest of
IGT, but would of course remove the code complexity. Assuming this was
some sort of post script, it would also not give you quite the fine
grained granular logging I have in this patch (i.e. only take the logs
leading up to an assert).
4) Implement RPC based approach. I like this idea, since like you said
we already have the interface. It means we're adding more code here
(minimal) and in chameleond (also minimal). Which means the turn-around 
time is going to be greater.
5) Implement in both SSH here and in RPC in chameleond. This could be a
two staged approach where we first push something like what I have,
then either get rid of it once we get the RPC interface in chameleond.
or support both options if that makes sense.

Thoughts?

> > diff --git a/lib/meson.build b/lib/meson.build
> > index dd36f8180..40d1cb65e 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -97,6 +97,10 @@ if alsa.found()
> >  	lib_sources += 'igt_alsa.c'
> >  endif
> >  
> > +if libssh.found()
> > +	lib_deps += libssh
> > +endif
> > +
> >  if chamelium.found()
> >  	lib_deps += chamelium
> >  	lib_sources += 'igt_chamelium.c'
> > diff --git a/meson.build b/meson.build
> > index 356a54142..3b4e40ce5 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -166,6 +166,8 @@ cairo = dependency('cairo', version :
> > '>1.12.0', required : true)
> >  libudev = dependency('libudev', required : true)
> >  glib = dependency('glib-2.0', required : true)
> >  
> > +libssh = dependency('libssh', required : true)
> 
> "required : true"  makes this dependency mandatory for the whole
> project, making the build fail.
> 
> What you want to do here is to make it not-required. The check you
> have
> below, 'libssh.found()' will make sure that the dependency is there
> for
> just the Chamelium.

Thanks for the tip! If we get the infrastructure to support libssh on
the chamelium hosts, I'll post an update with this change.

> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3270 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190221/957c6220/attachment-0001.bin>