package/sysdig: New package

PATCH Superseded Angelo Compagnucci 3 participants 5 messages Mar 26, 2015 – Mar 27, 2015

Share & tools

Angelo Compagnucci

author

2015-03-26 07:48 UTC #0 lore raw

PATCH v9 1/1 package/sysdig: New package

Sysdig is open source, system-level exploration:
capture system state and activity from a running Linux instance, then save, filter and analyze.

Signed-off-by Angelo Compagnucci <[email protected]>

---
Changes v8 -> v9:
- Added SYSDIG_MAKE_ENV to correctly compile kernel driver (thanks Ryan Barnett)
- Added an explicit SYSDIG_POST_INSTALL_TARGET_HOOKS for installing kernel driver as stated in the official documentation [1]
- Fixed jsoncpp dependency
- The patch should be in a good final shape now

[1] https://github.com/draios/sysdig/wiki/How%20to%20Install%20Sysdig%20from%20the%20Source%20Code

Changes v7 -> v8:
- sysdig now alphabetically ordered in Config.in

Changes v6 -> v7:
- Fixing a nasty mistake in Config.in

Changes v5 -> v6:
- Patching kernel module makefile to be compatible with buildroot.
- Patching cmakefile to remove unneed installation of DKMS infrastructure.
- Removing of unneeded post installation script.
- Added -DUSE_BUNDLED_JSONCPP = NO
- Package is now at the bare minimum.

Changes v4 -> v5:
- Adjusted to 80 columns for sysdig.mk header

Changes v3 -> v4:
- Changed "depends on" to "select" and fixed selected packages dependencies.
- moved "comment" section to the bottom

Changes v2 -> v3:
- Changed "depends on" and "select" to simplify package

Changes v1 -> v2:
- Changed "depends on" with "select" for dependencies (suggested by Baruch)
- Added comment "sysdig needs a Linux kernel to be built" (suggested by Baruch)
- Upgreded to recently released 0.1.99

 package/Config.in                                  |  1 +
 .../0001-makefile-driver-compile-options.patch     | 23 ++++++++++++++++
 .../sysdig/0002-remove-dkms-module-updater.patch   | 31 ++++++++++++++++++++++
 package/sysdig/Config.in                           | 21 +++++++++++++++
 package/sysdig/sysdig.mk                           | 24 +++++++++++++++++
 5 files changed, 100 insertions(+)
 create mode 100644 package/sysdig/0001-makefile-driver-compile-options.patch
 create mode 100644 package/sysdig/0002-remove-dkms-module-updater.patch
 create mode 100644 package/sysdig/Config.in
 create mode 100644 package/sysdig/sysdig.mk

package/Config.in +1 -0

index e4ee95d..aaf12ec 100644
@@ -94,6 +94,7 @@ endif
 	source "package/spidev_test/Config.in"
 	source "package/strace/Config.in"
 	source "package/stress/Config.in"
+	source "package/sysdig/Config.in"
 	source "package/sysprof/Config.in"
 	source "package/tinymembench/Config.in"
 	source "package/trace-cmd/Config.in"

package/sysdig/0001-makefile-driver-compile-options.patch new file +23 -0

new file mode 100644
index 0000000..74c74d8
@@ -0,0 +1,23 @@
+Updated Makefile compile options
+
+This patch updates linux kernel module (driver) of sysdig to be
+compatible with buildroot compile flags.
+
+Signed-off-by: Angelo Compagnucci <[email protected]>
+
+--- a/driver/Makefile.in
++++ b/driver/Makefile.in
+@@ -6,10 +6,10 @@ KERNELDIR 		?= /lib/modules/$(shell uname -r)/build
+
+ TOP := $(shell pwd)
+ all:
+-	$(MAKE) -C $(KERNELDIR) M=$(TOP) modules
++	$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) M=$(TOP) modules
+
+ clean:
+-	$(MAKE) -C $(KERNELDIR) M=$(TOP) clean
++	$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) M=$(TOP) clean
+
+ install: all
+-	$(MAKE) -C $(KERNELDIR) M=$(TOP) modules_install
++	$(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) M=$(TOP) modules_install

package/sysdig/0002-remove-dkms-module-updater.patch new file +31 -0

new file mode 100644
index 0000000..8c0e739
@@ -0,0 +1,31 @@
+Rmove DKMS module updater
+
+This patch disables the in target installation of DKMS module updater
+mechanism unneeded in buildroot.
+
+Signed-off-by: Angelo Compagnucci <[email protected]>
+
+--- a/driver/CMakeLists.txt
++++ b/driver/CMakeLists.txt
+@@ -39,21 +39,3 @@ add_custom_target(install_driver
+ 	WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+ 	VERBATIM)
+ 
+-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/Makefile.dkms
+-	RENAME Makefile
+-	DESTINATION "src/sysdig-${SYSDIG_VERSION}")
+-
+-install(FILES
+-	${CMAKE_CURRENT_BINARY_DIR}/dkms.conf
+-	dynamic_params_table.c
+-	event_table.c
+-	flags_table.c
+-	main.c
+-	ppm.h
+-	ppm_events.c
+-	ppm_events.h
+-	ppm_events_public.h
+-	ppm_fillers.c
+-	ppm_ringbuffer.h
+-	syscall_table.c
+-	DESTINATION "src/sysdig-${SYSDIG_VERSION}")

package/sysdig/Config.in new file +21 -0

new file mode 100644
index 0000000..caf7ef8
@@ -0,0 +1,21 @@
+config BR2_PACKAGE_SYSDIG
+	bool "sysdig"
+	select BR2_PACKAGE_ZLIB
+	select BR2_PACKAGE_LUAJIT
+	select BR2_PACKAGE_JSONCPP
+	depends on BR2_LINUX_KERNEL
+	depends on BR2_INSTALL_LIBSTDCPP # libjson
+	depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS
+	help
+	  Sysdig is open source, system-level exploration:
+	  capture system state and activity from a running Linux instance,
+	  then save, filter and analyze.
+	  Think of it as strace + tcpdump + lsof + awesome sauce.
+	  With a little Lua cherry on top.
+
+	  http://sysdig.org
+
+comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"
+	depends on !BR2_LINUX_KERNEL
+	depends on !BR2_INSTALL_LIBSTDCPP
+	depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

package/sysdig/sysdig.mk new file +24 -0

new file mode 100644
index 0000000..e05124b
@@ -0,0 +1,24 @@
+################################################################################
+#
+# sysdig
+#
+################################################################################
+
+SYSDIG_VERSION = 0.1.99
+SYSDIG_SITE = $(call github,draios,sysdig,$(SYSDIG_VERSION))
+SYSDIG_LICENSE = GPLv2
+SYSDIG_LICENSE_FILES = COPYING
+SYSDIG_CONF_OPTS = -DUSE_BUNDLED_LUAJIT=OFF -DUSE_BUNDLED_ZLIB=OFF \
+	-DUSE_BUNDLED_JSONCPP=OFF
+SYSDIG_DEPENDENCIES = zlib luajit jsoncpp linux
+SYSDIG_SUPPORTS_IN_SOURCE_BUILD = NO
+
+SYSDIG_MAKE_ENV = LINUX_DIR='$(LINUX_DIR)' LINUX_MAKE_FLAGS='$(LINUX_MAKE_FLAGS)'
+
+define SYSDIG_INSTALL_DRIVER
+	cd $(@D)/buildroot-build; $(MAKE) $(SYSDIG_MAKE_ENV) install_driver
+endef
+
+SYSDIG_POST_INSTALL_TARGET_HOOKS += SYSDIG_INSTALL_DRIVER
+
+$(eval $(cmake-package))

Signature · 1 line

1.9.1

Ryan Barnett

2015-03-26 18:38 UTC #1 lore raw

PATCH v9 1/1 package/sysdig: New package

Hi Angelo,

Quoted 5 lines · Angelo Compagnucci

Sysdig is open source, system-level exploration:
capture system state and activity from a running Linux instance, then save, filter and analyze.
Signed-off-by Angelo Compagnucci <[email protected]>

It would be good to carry forward previous reviews when you submit a new version of a patch. In your v8 of this patch Yegor Yefremov reviewed your patch so after your Signed-off-by line you should put this:

Reviewed-by Yegor Yefremov <[email protected]>

I have a found a few other things when testing v9 and I have listed them below.

[...]

package/sysdig/0002-remove-dkms-module-updater.patch line 1

Quoted 7 lines

package/sysdig/0002-remove-dkms-module-updater.patch new file +1 -0
new file mode 100644
index 0000000..8c0e739
@@ -0,0 +1,31 @@
+Rmove DKMS module updater

Minor typo: 'Rmove' should be 'Remove'.

Quoted 47 lines

package/sysdig/0002-remove-dkms-module-updater.patch new file +30 -0

+
+This patch disables the in target installation of DKMS module updater
+mechanism unneeded in buildroot.
+
+Signed-off-by: Angelo Compagnucci <[email protected]>
+
+--- a/driver/CMakeLists.txt
++++ b/driver/CMakeLists.txt
+@@ -39,21 +39,3 @@ add_custom_target(install_driver
+       WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+       VERBATIM)
+
+-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/Makefile.dkms
+-      RENAME Makefile
+-      DESTINATION "src/sysdig-${SYSDIG_VERSION}")
+-
+-install(FILES
+-      ${CMAKE_CURRENT_BINARY_DIR}/dkms.conf
+-      dynamic_params_table.c
+-      event_table.c
+-      flags_table.c
+-      main.c
+-      ppm.h
+-      ppm_events.c
+-      ppm_events.h
+-      ppm_events_public.h
+-      ppm_fillers.c
+-      ppm_ringbuffer.h
+-      syscall_table.c
+-      DESTINATION "src/sysdig-${SYSDIG_VERSION}")

package/sysdig/Config.in new file +11 -0

new file mode 100644
index 0000000..caf7ef8
@@ -0,0 +1,21 @@
+config BR2_PACKAGE_SYSDIG
+       bool "sysdig"
+       select BR2_PACKAGE_ZLIB
+       select BR2_PACKAGE_LUAJIT
+       select BR2_PACKAGE_JSONCPP
+       depends on BR2_LINUX_KERNEL
+       depends on BR2_INSTALL_LIBSTDCPP # libjson
+       depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS
+       help
+         Sysdig is open source, system-level exploration:
+         capture system state and activity from a running Linux instance,

package/sysdig/Config.in line 21

Quoted 10 lines

package/sysdig/Config.in new file +10 -0

+         then save, filter and analyze.
+         Think of it as strace + tcpdump + lsof + awesome sauce.
+         With a little Lua cherry on top.
+
+         http://sysdig.org
+
+comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"
+       depends on !BR2_LINUX_KERNEL
+       depends on !BR2_INSTALL_LIBSTDCPP
+       depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

When you have a toolchain that doesn't have one of the required dependencies, this comment doesn't show up. This is due to have 3 separate depends. Instead the depends should look like this:

comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"

        depends on !BR2_LINUX_KERNEL || !BR2_INSTALL_LIBSTDCPP \
                  || !BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

(Note indentation above should be tabs instead of spaces)

Quoted 20 lines

package/sysdig/sysdig.mk new file +14 -0

new file mode 100644
index 0000000..e05124b
@@ -0,0 +1,24 @@
+################################################################################
+#
+# sysdig
+#
+################################################################################
+
+SYSDIG_VERSION = 0.1.99
+SYSDIG_SITE = $(call github,draios,sysdig,$(SYSDIG_VERSION))
+SYSDIG_LICENSE = GPLv2
+SYSDIG_LICENSE_FILES = COPYING
+SYSDIG_CONF_OPTS = -DUSE_BUNDLED_LUAJIT=OFF -DUSE_BUNDLED_ZLIB=OFF \
+       -DUSE_BUNDLED_JSONCPP=OFF
+SYSDIG_DEPENDENCIES = zlib luajit jsoncpp linux
+SYSDIG_SUPPORTS_IN_SOURCE_BUILD = NO

package/sysdig/sysdig.mk line 24

Quoted 10 lines

package/sysdig/sysdig.mk new file +10 -0

+
+SYSDIG_MAKE_ENV = LINUX_DIR='$(LINUX_DIR)' LINUX_MAKE_FLAGS='$(LINUX_MAKE_FLAGS)'
+
+define SYSDIG_INSTALL_DRIVER
+       cd $(@D)/buildroot-build; $(MAKE) $(SYSDIG_MAKE_ENV) install_driver
+endef
+
+SYSDIG_POST_INSTALL_TARGET_HOOKS += SYSDIG_INSTALL_DRIVER
+
+$(eval $(cmake-package))

With these final fixups I think we are about done with some issues. I will look for you v10

Thanks,
-Ryan

Signature · 5 lines

Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com

Angelo Compagnucci

author

2015-03-26 20:37 UTC #2 lore raw

PATCH v9 1/1 package/sysdig: New package

Dear Ryan Barnett,

2015-03-26 19:38 GMT+01:00 Ryan Barnett <[email protected]>:

Quoted 8 lines

Hi Angelo,
Quoted 4 lines · Angelo Compagnucci
Sysdig is open source, system-level exploration:
capture system state and activity from a running Linux instance, then save, filter and analyze.

Quoted 8 lines

Quoted 1 line
Signed-off-by Angelo Compagnucci <[email protected]>
It would be good to carry forward previous reviews when you submit a new version of a patch. In your v8 of this patch Yegor Yefremov reviewed your patch so after your Signed-off-by line you should put this:
Reviewed-by Yegor Yefremov <[email protected]>

Are you sure? IMO, If the patch changes it should be reviewed again.

Quoted 5 lines

I have a found a few other things when testing v9 and I have listed them below.
[...]

Quoted 9 lines

package/sysdig/0002-remove-dkms-module-updater.patch line 1
Quoted 7 lines
package/sysdig/0002-remove-dkms-module-updater.patch new file +1 -0
new file mode 100644
index 0000000..8c0e739
@@ -0,0 +1,31 @@
+Rmove DKMS module updater
Minor typo: 'Rmove' should be 'Remove'.

good catch!

Quoted 63 lines

Quoted 48 lines

package/sysdig/0002-remove-dkms-module-updater.patch new file +30 -0

+
+This patch disables the in target installation of DKMS module updater
+mechanism unneeded in buildroot.
+
+Signed-off-by: Angelo Compagnucci <[email protected]>
+
+--- a/driver/CMakeLists.txt
++++ b/driver/CMakeLists.txt
+@@ -39,21 +39,3 @@ add_custom_target(install_driver
+       WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+       VERBATIM)
+
+-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/Makefile.dkms
+-      RENAME Makefile
+-      DESTINATION "src/sysdig-${SYSDIG_VERSION}")
+-
+-install(FILES
+-      ${CMAKE_CURRENT_BINARY_DIR}/dkms.conf
+-      dynamic_params_table.c
+-      event_table.c
+-      flags_table.c
+-      main.c
+-      ppm.h
+-      ppm_events.c
+-      ppm_events.h
+-      ppm_events_public.h
+-      ppm_fillers.c
+-      ppm_ringbuffer.h
+-      syscall_table.c
+-      DESTINATION "src/sysdig-${SYSDIG_VERSION}")

package/sysdig/Config.in new file +11 -0

new file mode 100644
index 0000000..caf7ef8
@@ -0,0 +1,21 @@
+config BR2_PACKAGE_SYSDIG
+       bool "sysdig"
+       select BR2_PACKAGE_ZLIB
+       select BR2_PACKAGE_LUAJIT
+       select BR2_PACKAGE_JSONCPP
+       depends on BR2_LINUX_KERNEL
+       depends on BR2_INSTALL_LIBSTDCPP # libjson
+       depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS
+       help
+         Sysdig is open source, system-level exploration:
+         capture system state and activity from a running Linux instance,

package/sysdig/Config.in line 21

Quoted 10 lines

package/sysdig/Config.in new file +10 -0

+         then save, filter and analyze.
+         Think of it as strace + tcpdump + lsof + awesome sauce.
+         With a little Lua cherry on top.
+
+         http://sysdig.org
+
+comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"
+       depends on !BR2_LINUX_KERNEL
+       depends on !BR2_INSTALL_LIBSTDCPP
+       depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

When you have a toolchain that doesn't have one of the required dependencies, this comment doesn't show up. This is due to have 3 separate depends. Instead the depends should look like this:

Quoted 3 lines

comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"
        depends on !BR2_LINUX_KERNEL || !BR2_INSTALL_LIBSTDCPP \
                  || !BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

Good catch, but the message should be viewed only on supported arhcs, so it should be:

comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"
depends on !BR2_LINUX_KERNEL || !BR2_INSTALL_LIBSTDCPP depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

Quoted 45 lines

(Note indentation above should be tabs instead of spaces)
Quoted 20 lines
package/sysdig/sysdig.mk new file +14 -0
new file mode 100644
index 0000000..e05124b
@@ -0,0 +1,24 @@
+################################################################################
+#
+# sysdig
+#
+################################################################################
+
+SYSDIG_VERSION = 0.1.99
+SYSDIG_SITE = $(call github,draios,sysdig,$(SYSDIG_VERSION))
+SYSDIG_LICENSE = GPLv2
+SYSDIG_LICENSE_FILES = COPYING
+SYSDIG_CONF_OPTS = -DUSE_BUNDLED_LUAJIT=OFF -DUSE_BUNDLED_ZLIB=OFF \
+       -DUSE_BUNDLED_JSONCPP=OFF
+SYSDIG_DEPENDENCIES = zlib luajit jsoncpp linux
+SYSDIG_SUPPORTS_IN_SOURCE_BUILD = NO
package/sysdig/sysdig.mk line 24
Quoted 10 lines
package/sysdig/sysdig.mk new file +10 -0
+
+SYSDIG_MAKE_ENV = LINUX_DIR='$(LINUX_DIR)' LINUX_MAKE_FLAGS='$(LINUX_MAKE_FLAGS)'
+
+define SYSDIG_INSTALL_DRIVER
+       cd $(@D)/buildroot-build; $(MAKE) $(SYSDIG_MAKE_ENV) install_driver
+endef
+
+SYSDIG_POST_INSTALL_TARGET_HOOKS += SYSDIG_INSTALL_DRIVER
+
+$(eval $(cmake-package))
With these final fixups I think we are about done with some issues. I will look for you v10
Thanks,
-Ryan
--
Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA ryan.barnett at rockwellcollins.com www.rockwellcollins.com

Signature · 1 line

Profile: http://it.linkedin.com/in/compagnucciangelo

Ryan Barnett

2015-03-27 00:46 UTC #3 lore raw

PATCH v9 1/1 package/sysdig: New package

Quoted 3 lines · Angelo Compagnucci

Dear Ryan Barnett,
Are you sure? IMO, If the patch changes it should be reviewed again.

You do bring up a good point. However, I was basing this on what Yann did with his hash improvement series:

http://patchwork.ozlabs.org/patch/453147/

Quoted 6 lines

Good catch, but the message should be viewed only on supported arhcs, so it should be:
comment "sysdig needs a toolchain w/ C++ and a Linux kernel to be built"
depends on !BR2_LINUX_KERNEL || !BR2_INSTALL_LIBSTDCPP depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS

Yes I agree with this.

Thanks,
-Ryan

Signature · 5 lines

Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com

Arnout Vandecappelle

2015-03-27 21:57 UTC #4 lore raw

PATCH v9 1/1 package/sysdig: New package

In reply to Angelo Compagnucci

Quoted 13 lines · Angelo Compagnucci

Dear Ryan Barnett,
2015-03-26 19:38 GMT+01:00 Ryan Barnett <[email protected]>:
Quoted 10 lines
Quoted 10 lines
Hi Angelo,
Quoted 5 lines · Angelo Compagnucci
Quoted 5 lines
Sysdig is open source, system-level exploration:
capture system state and activity from a running Linux instance, then save, filter and analyze.
Signed-off-by Angelo Compagnucci <[email protected]>

Quoted 7 lines

Quoted 6 lines
Quoted 6 lines
It would be good to carry forward previous reviews when you submit a new version of a patch. In your v8 of this patch Yegor Yefremov reviewed your patch so after your Signed-off-by line you should put this:
Reviewed-by Yegor Yefremov <[email protected]>
Are you sure? IMO, If the patch changes it should be reviewed again.

It basically depends on how much you still changed after that reviewed-by was
given. In this case, it indeed probably changed too much to keep the tag.

For Yegor's convenience, however, it could be good to add something like:

Previous-version-reviewed-by: ...

(At least in my case, I tend to look more carefully at the patches I reviewed before, and I tend to forget which ones I reviewed in the past.)

Regards,
Arnout

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F