[igt-dev] [PATCH i-g-t 0/2] Enhance debug capability in kms_chamelium

[igt-dev] [PATCH i-g-t 0/2] Enhance debug capability in kms_chamelium

[Stuart Summers via igt-dev]

The first patch in the series cleans up the missed requirement for VGA
tests in CI due to lack of support in later hardware.

The second patch adds a new libssh interface to extract chameleond
daemon logs from the Chamelion board. Over time, it might be interesting
to add new remote logging capability into the Python daemon itself or to
move this ssh based log capture into the existing xmlrpc interface. For
the short term, I am hoping to extract some additional logs from CI with
this patch.

Stuart Summers (2):
  i915/kms_chamelium: Require gen < 9 for VGA testing
  i915/kms_chamelium: Pull chameleond logs for debugging

 Dockerfile.debian     |   1 +
 Dockerfile.fedora     |   1 +
 lib/igt_chamelium.c   | 150 +++++++++++++++++++++++++++++++++++++++++-
 lib/meson.build       |   4 ++
 meson.build           |   4 +-
 tests/kms_chamelium.c |   2 +
 6 files changed, 160 insertions(+), 2 deletions(-)

-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 1/2] i915/kms_chamelium: Require gen < 9 for VGA testing

[Stuart Summers via igt-dev]

From: Stuart Summers <jssummer@jssummer-desk1.amr.corp.intel.com>

VGA not supported on gen 9 and above. Add an explicit requirement
to skip VGA tests to a Chamelium on later gens.

Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 tests/kms_chamelium.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 64f87d3ae..ab89c5d47 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -1015,6 +1015,8 @@ igt_main
 
 	igt_subtest_group {
 		igt_fixture {
+			int gen = intel_gen(intel_get_drm_devid(data.drm_fd));
+			igt_require_f(gen < 9, "VGA not supported\n");
 			require_connector_present(
 			    &data, DRM_MODE_CONNECTOR_VGA);
 		}
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Stuart Summers via igt-dev]

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@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 100644
--- 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>
 #include <pthread.h>
 #include <glib.h>
 #include <pixman.h>
@@ -120,6 +121,8 @@ struct chamelium {
 	struct chamelium_edid *edids;
 	struct chamelium_port *ports;
 	int port_count;
+
+	ssh_session session;
 };
 
 static struct chamelium *cleanup_instance;
@@ -256,6 +259,56 @@ static void *chamelium_fsm_mon(void *data)
 	return NULL;
 }
 
+static void
+chamelium_send_ssh_command(struct chamelium *chamelium, const char *cmd,
+			   bool print_result)
+{
+	int rc;
+	ssh_channel channel;
+
+	channel = ssh_channel_new(chamelium->session);
+	if (channel == NULL)
+		return;
+
+	rc = ssh_channel_open_session(channel);
+	if (rc != SSH_OK)
+		goto free;
+
+	rc = ssh_channel_request_exec(channel, cmd);
+	if (rc != SSH_OK)
+		goto free;
+
+	if (print_result) {
+		int nbytes;
+		char buffer[256];
+
+		nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
+		while (nbytes > 0) {
+			igt_debug("%s", buffer);
+			nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
+		}
+		igt_debug("\n");
+		ssh_channel_send_eof(channel);
+	}
+
+	ssh_channel_close(channel);
+
+free:
+	ssh_channel_free(channel);
+}
+
+static void
+chamelium_clear_chameleond(struct chamelium *chamelium)
+{
+	chamelium_send_ssh_command(chamelium, "> /var/log/chameleond", false);
+}
+
+static void
+chamelium_dump_chameleond(struct chamelium *chamelium)
+{
+	chamelium_send_ssh_command(chamelium, "cat /var/log/chameleond", true);
+}
+
 static xmlrpc_value *chamelium_rpc(struct chamelium *chamelium,
 				   struct chamelium_port *fsm_port,
 				   const char *method_name,
@@ -298,9 +351,11 @@ static xmlrpc_value *chamelium_rpc(struct chamelium *chamelium,
 		igt_cleanup_hotplug(monitor_args.mon);
 	}
 
+	chamelium_dump_chameleond(chamelium);
 	igt_assert_f(!chamelium->env.fault_occurred,
 		     "Chamelium RPC call failed: %s\n",
 		     chamelium->env.fault_string);
+	chamelium_clear_chameleond(chamelium);
 
 	return res;
 }
@@ -1510,7 +1565,7 @@ out:
 	return ret;
 }
 
-static bool chamelium_read_config(struct chamelium *chamelium, int drm_fd)
+static bool chamelium_get_url(struct chamelium *chamelium)
 {
 	GError *error = NULL;
 
@@ -1527,6 +1582,11 @@ static bool chamelium_read_config(struct chamelium *chamelium, int drm_fd)
 		return false;
 	}
 
+	return true;
+}
+
+static bool chamelium_read_config(struct chamelium *chamelium, int drm_fd)
+{
 	return chamelium_read_port_mappings(chamelium, drm_fd);
 }
 
@@ -1551,6 +1611,84 @@ static void chamelium_exit_handler(int sig)
 		chamelium_deinit(cleanup_instance);
 }
 
+/*
+ * Extract the IP address from the URL in the form:
+ *   http://<ip addr>:<port>
+ *
+ * Expect caller to free the resulting string.
+ */
+static char *
+get_ip_addr_from_url(struct chamelium *chamelium)
+{
+	char *tmp;
+	char *ip_addr;
+	char *port;
+
+	tmp = strrchr(chamelium->url, '/');
+	if (!tmp)
+		return NULL;
+	tmp++;
+
+	port = strrchr(chamelium->url, ':');
+	if (!port)
+		return NULL;
+
+	ip_addr = malloc(strlen(tmp));
+	if (!ip_addr)
+		return NULL;
+
+	strcpy(ip_addr, tmp);
+	ip_addr[port - tmp] = '\0';
+
+	return ip_addr;
+}
+
+static int
+chamelium_ssh_init(struct chamelium *chamelium)
+{
+	int rc;
+	char *ip_addr = get_ip_addr_from_url(chamelium);
+	if (!ip_addr)
+		return false;
+
+	chamelium->session = ssh_new();
+	if (chamelium->session == NULL) {
+		free(ip_addr);
+		return false;
+	}
+
+	ssh_options_set(chamelium->session, SSH_OPTIONS_HOST, ip_addr);
+	ssh_options_set(chamelium->session, SSH_OPTIONS_USER, "root");
+
+	rc = ssh_connect(chamelium->session);
+	if (rc != SSH_OK)
+		goto free;
+
+	/* Currently assuming no password is set */
+	rc = ssh_userauth_none(chamelium->session, NULL);
+	if (rc != SSH_AUTH_SUCCESS)
+		goto disconnect;
+
+	free(ip_addr);
+
+	return true;
+
+disconnect:
+	ssh_disconnect(chamelium->session);
+free:
+	ssh_free(chamelium->session);
+	free(ip_addr);
+
+	return false;
+}
+
+static void
+chamelium_ssh_deinit(struct chamelium *chamelium)
+{
+	ssh_disconnect(chamelium->session);
+	ssh_free(chamelium->session);
+}
+
 /**
  * chamelium_init:
  * @chamelium: The Chamelium instance to use
@@ -1592,6 +1730,14 @@ struct chamelium *chamelium_init(int drm_fd)
 		goto error;
 	}
 
+	if (!chamelium_get_url(chamelium))
+		goto error;
+
+	if (!chamelium_ssh_init(chamelium))
+		goto error;
+
+	chamelium_clear_chameleond(chamelium);
+
 	if (!chamelium_read_config(chamelium, drm_fd))
 		goto error;
 
@@ -1636,6 +1782,8 @@ void chamelium_deinit(struct chamelium *chamelium)
 		free(pos);
 	}
 
+	chamelium_ssh_deinit(chamelium);
+
 	xmlrpc_client_destroy(chamelium->client);
 	xmlrpc_env_clean(&chamelium->env);
 
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)
+
 gsl = null_dep
 alsa = null_dep
 if _build_audio or _build_chamelium
@@ -205,7 +207,7 @@ endif
 
 chamelium = null_dep
 chameliuminfo = 'No'
-if _build_chamelium and gsl.found() and xmlrpc.found() and xmlrpc_util.found() and xmlrpc_client.found()
+if _build_chamelium and gsl.found() and libssh.found() and xmlrpc.found() and xmlrpc_util.found() and xmlrpc_client.found()
 	chamelium = declare_dependency(dependencies : [ xmlrpc,
 							xmlrpc_util, xmlrpc_client])
 	config.set('HAVE_CHAMELIUM', 1)
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for Enhance debug capability in kms_chamelium

[Patchwork]

== Series Details ==

Series: Enhance debug capability in kms_chamelium
URL   : https://patchwork.freedesktop.org/series/56699/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
395eaffd7e1390c9d6043c2980dc14ce3e08b154 kms_content_protection: Macros replace the constants

Compiler for C supports argument -Wmissing-format-attribute: YES
Compiler for C supports argument -Wmissing-noreturn: YES
Compiler for C supports argument -Wmissing-prototypes: YES
Compiler for C supports argument -Wnested-externs: YES
Compiler for C supports argument -Wold-style-definition: YES
Compiler for C supports argument -Wpointer-arith: YES
Compiler for C supports argument -Wredundant-decls: YES
Compiler for C supports argument -Wshadow: YES
Compiler for C supports argument -Wstrict-prototypes: YES
Compiler for C supports argument -Wuninitialized: YES
Compiler for C supports argument -Wunused: YES
Compiler for C supports argument -Wno-clobbered: YES
Compiler for C supports argument -Wno-maybe-uninitialized: YES
Compiler for C supports argument -Wno-missing-field-initializers: YES
Compiler for C supports argument -Wno-pointer-arith: YES
Compiler for C supports argument -Wno-sign-compare: YES
Compiler for C supports argument -Wno-type-limits: YES
Compiler for C supports argument -Wno-unused-parameter: YES
Compiler for C supports argument -Wno-unused-result: YES
Compiler for C supports argument -Werror=address: YES
Compiler for C supports argument -Werror=array-bounds: YES
Compiler for C supports argument -Werror=implicit: YES
Compiler for C supports argument -Werror=init-self: YES
Compiler for C supports argument -Werror=int-to-pointer-cast: YES
Compiler for C supports argument -Werror=main: YES
Compiler for C supports argument -Werror=missing-braces: YES
Compiler for C supports argument -Werror=nonnull: YES
Compiler for C supports argument -Werror=pointer-to-int-cast: YES
Compiler for C supports argument -Werror=return-type: YES
Compiler for C supports argument -Werror=sequence-point: YES
Compiler for C supports argument -Werror=trigraphs: YES
Compiler for C supports argument -Werror=write-strings: YES
Found pkg-config: /usr/bin/pkg-config (0.29.1)
Native dependency libdrm found: YES 2.4.97
Native dependency libdrm_intel found: YES 2.4.97
Native dependency libdrm_nouveau found: YES 2.4.91
Native dependency libdrm_amdgpu found: YES 2.4.97
Native dependency pciaccess found: YES 0.14
Native dependency libkmod found: YES 24
Native dependency libprocps found: YES 3.3.12
Native dependency libunwind found: YES 1.21
Native dependency libdw found: YES 0.170
Native dependency pixman-1 found: YES 0.34.0
Native dependency cairo found: YES 1.15.10
Native dependency libudev found: YES 237
Native dependency glib-2.0 found: YES 2.56.3

meson.build:169:0: ERROR: Native dependency 'libssh' not found

A full log can be found at /home/cidrm/igt-gpu-tools/build/meson-logs/meson-log.txt

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/2] i915/kms_chamelium: Require gen < 9 for VGA testing

[Petri Latvala]

On Thu, Feb 14, 2019 at 04:40:42PM -0800, Stuart Summers via igt-dev wrote:
> From: Stuart Summers <jssummer@jssummer-desk1.amr.corp.intel.com>
> 
> VGA not supported on gen 9 and above. Add an explicit requirement
> to skip VGA tests to a Chamelium on later gens.
> 
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---
>  tests/kms_chamelium.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 64f87d3ae..ab89c5d47 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -1015,6 +1015,8 @@ igt_main
>  
>  	igt_subtest_group {
>  		igt_fixture {
> +			int gen = intel_gen(intel_get_drm_devid(data.drm_fd));
> +			igt_require_f(gen < 9, "VGA not supported\n");
>  			require_connector_present(
>  			    &data, DRM_MODE_CONNECTOR_VGA);

The obvious first comment is that you're not necessarily running on an
intel device. But let's put that aside, there's a more important
question.

What problem does this solve? The line directly after checks for a VGA
connector being present. VGA via DP not good enough? If there's
problems with that, surely there are better ways to get around it than
ignoring it.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/2] i915/kms_chamelium: Require gen < 9 for VGA testing

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 2059 bytes --]

On Mon, 2019-02-18 at 12:57 +0200, Petri Latvala wrote:
> On Thu, Feb 14, 2019 at 04:40:42PM -0800, Stuart Summers via igt-dev
> wrote:
> > From: Stuart Summers <jssummer@jssummer-desk1.amr.corp.intel.com>
> > 
> > VGA not supported on gen 9 and above. Add an explicit requirement
> > to skip VGA tests to a Chamelium on later gens.
> > 
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >  tests/kms_chamelium.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 64f87d3ae..ab89c5d47 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -1015,6 +1015,8 @@ igt_main
> >  
> >  	igt_subtest_group {
> >  		igt_fixture {
> > +			int gen =
> > intel_gen(intel_get_drm_devid(data.drm_fd));
> > +			igt_require_f(gen < 9, "VGA not supported\n");
> >  			require_connector_present(
> >  			    &data, DRM_MODE_CONNECTOR_VGA);
> 
> The obvious first comment is that you're not necessarily running on
> an
> intel device. But let's put that aside, there's a more important
> question.

Good point. From your below feedback, I'm thinking of just dropping
this. If it seems interesting to keep this around (if I hear more
feedback here), I'll clean this up.

> 
> What problem does this solve? The line directly after checks for a
> VGA

I personally found this useful to differenciate an invalid system
configuration causing CI to complain (i.e. someone forgot to plug in
the cable) vs an intentional limitation (i.e. the platform doesn't
support this). I agree it isn't strictly necessary since the subsequent
check will prevent any actual test content from being exercised.

Also, maybe a precident of checking all platform capabilities for each
of these isn't really the right direction. I'll drop this. Thanks for
the feedback!

> connector being present. VGA via DP not good enough? If there's
> problems with that, surely there are better ways to get around it
> than
> ignoring it.
> 
> 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Arkadiusz Hiler]

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@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 100644
> --- 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!

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?

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

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 4515 bytes --]

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

> 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev