[PATCH 0/2] deb to snap: use the kernel version from the

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] deb to snap: use the kernel version from the

Paolo Pisati-5
It was pointed to us that using the version from the linux-image-$BRANCH (e.g.
linux-image-generic) package is not a good idea, since that version doesn't map
1:1 to the real kernel that comes from the linux-image-$VER-$ABI-$BRANCH (e.g.
linux-image-4.4.0-98-generic) package - in other words, the same linux meta
package could actually end up using two completely different kernels since it
doesn't keep track of the upload number, and as such we won't be able to tell
univocally which kernel image ended up in the produced kernel snap: to fix this,
i added a mechanism that, given the meta package, it recurse down to find the
real linux kernel package, extract its version and compares it against the
expected version (while previously we used the "apt-get install
$(KERNEL)=$(VERSION)" to force the installation of exactly that linux meta
version, something we can't do anymore now as explained above).

Patch 0001 is a prerequisite for patch 2, and improve over our apt-pinning
method to let us use only kernel packages from the proposed pocket, and remove
the "apt-get install $(KERNEL)=$(VERSION)" mechanism - now we'll just install
the latest version of the required package available.
Patch 0002 is where the real version extraction and checking happens, and it
does so by making the checkversion shell snippet a prerequisite of the install
rule.

I tested the above by rebuilding all our kernel snaps on LP:

pc-kernel - i386 and amd64
raspi2-kernel - armhf
snapdragon-kernel - arm64
gke, gcp and aws - amd64

and testing all the possible error paths or conditions.

Since these patches don't change anything in the content on the generated kernel
snap (actually they add a stricter version checking and a better pinning), the
risk of regression is very low.

This series of patches apply to our 'deb to snap' packaging scripts located
here:
   
git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux-snap/+git/xenial

and should be applied to the master Makefile, while all the topic
branches should get the version field (version: in snapcraft.yaml) changed to
point to the version from the linux-image-$VER-$ABI-$BRANCH corresponding
package.

In other words, and at the time of this writing, these are the version that
should be used for all the topic branches:

pc-kernel: 4.4.0-98.121 (linux-image-4.4.0-98-generic / linux-signed-image-4.4.0-98-generic)
raspi2-kernel: 4.4.0-1076.84 (linux-image-4.4.0-1076-raspi2)
snapdragon-kernel: 4.4.0-1078.83 (linux-image-4.4.0-1078-snapdragon)
aws-kernel: 4.4.0-1039.48 (linux-image-4.4.0-1039-aws)
gcp-kernel: 4.10.0-1008.8 (linux-image-4.10.0-1008-gcp)
gke-kernel: 4.4.0-1033.33 (linux-image-4.4.0-1033-gke)

I could have sent a separate version patch for every branch, but since it takes
some time to get stuff reviewed and applied, and you might want to use the kernel
available in -proposed instead of the one in -security/updates, i'll leave the
version extraction to you.

Paolo Pisati (2):
  deb to snap: better apt-pinning, use $RELEASE everywhere and
    explicitly exclude linux-firmware from -proposed
  deb to snap: recurse down to find the real kernel image package, and
    chek its version

 Makefile | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] deb to snap: better apt-pinning, use $RELEASE everywhere and explicitly exclude linux-firmware from -proposed

Paolo Pisati-5
Signed-off-by: Paolo Pisati <[hidden email]>
---
 Makefile | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index b9973eb..dad3156 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,7 @@ endif
 
 # rewriting variables passed from the outside environment doesn't work in LP,
 # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
-KERNELDEB := $(KERNEL)=$(SNAPCRAFT_PROJECT_VERSION)
+KERNELDEB := $(KERNEL)
 
 # linux-pc-image is a meta package used to indicate either
 # linux-signed-image-generic or linux-image-generic, depending on the building
@@ -26,21 +26,21 @@ endif
 endif
 
 define APTPREF
-Package: *
-Pin: release a=$(RELEASE);
-Pin-Priority: 700
+Package: linux-firmware
+Pin: release a=$(RELEASE)-proposed
+Pin-Priority: 400
 
-Package: *
-Pin: release a=$(RELEASE)-updates
-Pin-Priority: 700
+Package: linux-*
+Pin: release a=$(RELEASE)-proposed
+Pin-Priority: 750
 
 Package: *
-Pin: release a=$(RELEASE)-security
-Pin-Priority: 700
+Pin: release a=$(RELEASE)-proposed
+Pin-Priority: 400
 
 Package: *
-Pin: release a=$(RELEASE)-proposed
-Pin-Priority: 650
+Pin: release a=$(RELEASE)*
+Pin-Priority: 700
 endef
 export APTPREF
 
@@ -49,7 +49,7 @@ install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinu
 all:
  debootstrap --variant=minbase $(RELEASE) chroot
  cp /etc/apt/sources.list chroot/etc/apt/sources.list
- echo "deb http://ppa.launchpad.net/snappy-dev/image/ubuntu xenial main" >> chroot/etc/apt/sources.list
+ echo "deb http://ppa.launchpad.net/snappy-dev/image/ubuntu $(RELEASE) main" >> chroot/etc/apt/sources.list
  if [ "$(PROPOSED)" = "true" ]; then \
   echo "deb http://$(MIRROR) $(RELEASE)-proposed main restricted" >> chroot/etc/apt/sources.list; \
   echo "deb http://$(MIRROR) $(RELEASE)-proposed universe" >> chroot/etc/apt/sources.list; \
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] deb to snap: recurse down to find the real kernel image package, and chek its version

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
Signed-off-by: Paolo Pisati <[hidden email]>
---
 Makefile | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dad3156..2bb8a89 100644
--- a/Makefile
+++ b/Makefile
@@ -11,6 +11,7 @@ endif
 # rewriting variables passed from the outside environment doesn't work in LP,
 # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
 KERNELDEB := $(KERNEL)
+KERNELPRE := linux-image-4
 
 # linux-pc-image is a meta package used to indicate either
 # linux-signed-image-generic or linux-image-generic, depending on the building
@@ -18,6 +19,7 @@ KERNELDEB := $(KERNEL)
 ifneq (,$(findstring linux-pc-image,$(KERNELDEB)))
 ifneq (,$(findstring amd64,$(DPKG_ARCH)))
 KERNELDEB := $(subst linux-pc-image,linux-signed-image-generic,$(KERNELDEB))
+KERNELPRE := linux-signed-image-4
 else ifneq (,$(findstring i386,$(DPKG_ARCH)))
 KERNELDEB := $(subst linux-pc-image,linux-image-generic,$(KERNELDEB))
 else
@@ -44,6 +46,7 @@ Pin-Priority: 700
 endef
 export APTPREF
 
+versioncheck: KIMGDEB = $(shell chroot chroot apt-cache depends $(KERNELDEB) | awk '/$(KERNELPRE)/ {print $$2}')
 install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinuz-//;s/.efi.signed$$//')
 
 all:
@@ -68,7 +71,7 @@ all:
  umount chroot/sys
  umount chroot/proc
 
-install:
+install: versioncheck
  mkdir -p $(DESTDIR)/lib $(DESTDIR)/meta $(DESTDIR)/firmware $(DESTDIR)/modules
  if [ -f chroot/boot/vmlinu?-*.signed ]; then \
   mv chroot/boot/vmlinu?-*.signed $(DESTDIR)/kernel.img; \
@@ -115,3 +118,16 @@ install:
  cd $(DESTDIR); ln -s kernel.img vmlinuz-$(KVERS)
  cd $(DESTDIR); ln -s kernel.img vmlinuz
  cd $(DESTDIR); ln -s initrd.img initrd.img-$(KVERS)
+
+versioncheck:
+ { \
+ set -e; \
+ echo $(KIMGDEB); \
+ KIMGVER="$$(dpkg --root=chroot -l | awk '/$(KIMGDEB)/ {print $$3}')"; \
+ echo $$KIMGVER; \
+ [ ! $$KIMGVER ] && echo "Unable to extract KIMGVER, exit" && exit 1; \
+ if [ $$KIMGVER != $(SNAPCRAFT_PROJECT_VERSION) ]; then \
+  echo "Version mismatch:\nInstalled: $$KIMGVER Requested: $(SNAPCRAFT_PROJECT_VERSION)"; \
+  exit 1; \
+ fi; \
+ }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/2] deb to snap: use the kernel version from the

Kleber Souza
In reply to this post by Paolo Pisati-5
The changes look good to me.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

On 10/31/17 19:02, Paolo Pisati wrote:

> It was pointed to us that using the version from the linux-image-$BRANCH (e.g.
> linux-image-generic) package is not a good idea, since that version doesn't map
> 1:1 to the real kernel that comes from the linux-image-$VER-$ABI-$BRANCH (e.g.
> linux-image-4.4.0-98-generic) package - in other words, the same linux meta
> package could actually end up using two completely different kernels since it
> doesn't keep track of the upload number, and as such we won't be able to tell
> univocally which kernel image ended up in the produced kernel snap: to fix this,
> i added a mechanism that, given the meta package, it recurse down to find the
> real linux kernel package, extract its version and compares it against the
> expected version (while previously we used the "apt-get install
> $(KERNEL)=$(VERSION)" to force the installation of exactly that linux meta
> version, something we can't do anymore now as explained above).
>
> Patch 0001 is a prerequisite for patch 2, and improve over our apt-pinning
> method to let us use only kernel packages from the proposed pocket, and remove
> the "apt-get install $(KERNEL)=$(VERSION)" mechanism - now we'll just install
> the latest version of the required package available.
> Patch 0002 is where the real version extraction and checking happens, and it
> does so by making the checkversion shell snippet a prerequisite of the install
> rule.
>
> I tested the above by rebuilding all our kernel snaps on LP:
>
> pc-kernel - i386 and amd64
> raspi2-kernel - armhf
> snapdragon-kernel - arm64
> gke, gcp and aws - amd64
>
> and testing all the possible error paths or conditions.
>
> Since these patches don't change anything in the content on the generated kernel
> snap (actually they add a stricter version checking and a better pinning), the
> risk of regression is very low.
>
> This series of patches apply to our 'deb to snap' packaging scripts located
> here:
>    
> git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux-snap/+git/xenial
>
> and should be applied to the master Makefile, while all the topic
> branches should get the version field (version: in snapcraft.yaml) changed to
> point to the version from the linux-image-$VER-$ABI-$BRANCH corresponding
> package.
>
> In other words, and at the time of this writing, these are the version that
> should be used for all the topic branches:
>
> pc-kernel: 4.4.0-98.121 (linux-image-4.4.0-98-generic / linux-signed-image-4.4.0-98-generic)
> raspi2-kernel: 4.4.0-1076.84 (linux-image-4.4.0-1076-raspi2)
> snapdragon-kernel: 4.4.0-1078.83 (linux-image-4.4.0-1078-snapdragon)
> aws-kernel: 4.4.0-1039.48 (linux-image-4.4.0-1039-aws)
> gcp-kernel: 4.10.0-1008.8 (linux-image-4.10.0-1008-gcp)
> gke-kernel: 4.4.0-1033.33 (linux-image-4.4.0-1033-gke)
>
> I could have sent a separate version patch for every branch, but since it takes
> some time to get stuff reviewed and applied, and you might want to use the kernel
> available in -proposed instead of the one in -security/updates, i'll leave the
> version extraction to you.
>
> Paolo Pisati (2):
>   deb to snap: better apt-pinning, use $RELEASE everywhere and
>     explicitly exclude linux-firmware from -proposed
>   deb to snap: recurse down to find the real kernel image package, and
>     chek its version
>
>  Makefile | 42 +++++++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 13 deletions(-)
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] deb to snap: recurse down to find the real kernel image package, and chek its version

Andy Whitcroft-3
In reply to this post by Paolo Pisati-5
On Tue, Oct 31, 2017 at 07:02:33PM +0100, Paolo Pisati wrote:

> Signed-off-by: Paolo Pisati <[hidden email]>
> ---
>  Makefile | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index dad3156..2bb8a89 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -11,6 +11,7 @@ endif
>  # rewriting variables passed from the outside environment doesn't work in LP,
>  # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
>  KERNELDEB := $(KERNEL)
> +KERNELPRE := linux-image-4

This 4 here likely should be something taken from the version we are
looking for?  Perhaps the whole ABI?

>  # linux-pc-image is a meta package used to indicate either
>  # linux-signed-image-generic or linux-image-generic, depending on the building
> @@ -18,6 +19,7 @@ KERNELDEB := $(KERNEL)
>  ifneq (,$(findstring linux-pc-image,$(KERNELDEB)))
>  ifneq (,$(findstring amd64,$(DPKG_ARCH)))
>  KERNELDEB := $(subst linux-pc-image,linux-signed-image-generic,$(KERNELDEB))
> +KERNELPRE := linux-signed-image-4

and here.

>  else ifneq (,$(findstring i386,$(DPKG_ARCH)))
>  KERNELDEB := $(subst linux-pc-image,linux-image-generic,$(KERNELDEB))
>  else
> @@ -44,6 +46,7 @@ Pin-Priority: 700
>  endef
>  export APTPREF
>  
> +versioncheck: KIMGDEB = $(shell chroot chroot apt-cache depends $(KERNELDEB) | awk '/$(KERNELPRE)/ {print $$2}')
>  install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinuz-//;s/.efi.signed$$//')
>  
>  all:
> @@ -68,7 +71,7 @@ all:
>   umount chroot/sys
>   umount chroot/proc
>  
> -install:
> +install: versioncheck
>   mkdir -p $(DESTDIR)/lib $(DESTDIR)/meta $(DESTDIR)/firmware $(DESTDIR)/modules
>   if [ -f chroot/boot/vmlinu?-*.signed ]; then \
>    mv chroot/boot/vmlinu?-*.signed $(DESTDIR)/kernel.img; \
> @@ -115,3 +118,16 @@ install:
>   cd $(DESTDIR); ln -s kernel.img vmlinuz-$(KVERS)
>   cd $(DESTDIR); ln -s kernel.img vmlinuz
>   cd $(DESTDIR); ln -s initrd.img initrd.img-$(KVERS)
> +
> +versioncheck:
> + { \
> + set -e; \
> + echo $(KIMGDEB); \
> + KIMGVER="$$(dpkg --root=chroot -l | awk '/$(KIMGDEB)/ {print $$3}')"; \
> + echo $$KIMGVER; \
> + [ ! $$KIMGVER ] && echo "Unable to extract KIMGVER, exit" && exit 1; \
> + if [ $$KIMGVER != $(SNAPCRAFT_PROJECT_VERSION) ]; then \
> +  echo "Version mismatch:\nInstalled: $$KIMGVER Requested: $(SNAPCRAFT_PROJECT_VERSION)"; \
> +  exit 1; \
> + fi; \
> + }

Otherwise it looks fine.

-apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] deb to snap: recurse down to find the real kernel image package, and chek its version

Paolo Pisati-5
Signed-off-by: Paolo Pisati <[hidden email]>
---
 Makefile | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dad3156..409bbc3 100644
--- a/Makefile
+++ b/Makefile
@@ -8,9 +8,12 @@ ifeq "$(strip $(KERNEL))" ""
 $(error KERNEL package name is missing, abort)
 endif
 
+ABI := $(shell echo $(SNAPCRAFT_PROJECT_VERSION) | cut -f1-3 -d".")
+
 # rewriting variables passed from the outside environment doesn't work in LP,
 # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
 KERNELDEB := $(KERNEL)
+KERNELPRE := linux-image-$(ABI)
 
 # linux-pc-image is a meta package used to indicate either
 # linux-signed-image-generic or linux-image-generic, depending on the building
@@ -18,6 +21,7 @@ KERNELDEB := $(KERNEL)
 ifneq (,$(findstring linux-pc-image,$(KERNELDEB)))
 ifneq (,$(findstring amd64,$(DPKG_ARCH)))
 KERNELDEB := $(subst linux-pc-image,linux-signed-image-generic,$(KERNELDEB))
+KERNELPRE := linux-signed-image-$(ABI)
 else ifneq (,$(findstring i386,$(DPKG_ARCH)))
 KERNELDEB := $(subst linux-pc-image,linux-image-generic,$(KERNELDEB))
 else
@@ -44,6 +48,7 @@ Pin-Priority: 700
 endef
 export APTPREF
 
+versioncheck: KIMGDEB = $(shell chroot chroot apt-cache depends $(KERNELDEB) | awk '/$(KERNELPRE)/ {print $$2}')
 install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinuz-//;s/.efi.signed$$//')
 
 all:
@@ -68,7 +73,7 @@ all:
  umount chroot/sys
  umount chroot/proc
 
-install:
+install: versioncheck
  mkdir -p $(DESTDIR)/lib $(DESTDIR)/meta $(DESTDIR)/firmware $(DESTDIR)/modules
  if [ -f chroot/boot/vmlinu?-*.signed ]; then \
   mv chroot/boot/vmlinu?-*.signed $(DESTDIR)/kernel.img; \
@@ -115,3 +120,16 @@ install:
  cd $(DESTDIR); ln -s kernel.img vmlinuz-$(KVERS)
  cd $(DESTDIR); ln -s kernel.img vmlinuz
  cd $(DESTDIR); ln -s initrd.img initrd.img-$(KVERS)
+
+versioncheck:
+ { \
+ set -e; \
+ echo $(KIMGDEB); \
+ KIMGVER="$$(dpkg --root=chroot -l | awk '/$(KIMGDEB)/ {print $$3}')"; \
+ echo $$KIMGVER; \
+ [ ! $$KIMGVER ] && echo "Unable to extract KIMGVER, exit" && exit 1; \
+ if [ $$KIMGVER != $(SNAPCRAFT_PROJECT_VERSION) ]; then \
+  echo "Version mismatch:\nInstalled: $$KIMGVER Requested: $(SNAPCRAFT_PROJECT_VERSION)"; \
+  exit 1; \
+ fi; \
+ }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Acked] [PATCH v2] deb to snap: recurse down to find the real kernel image package, and chek its version

Andy Whitcroft-3
On Wed, Nov 15, 2017 at 05:46:14PM +0100, Paolo Pisati wrote:

> Signed-off-by: Paolo Pisati <[hidden email]>
> ---
>  Makefile | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index dad3156..409bbc3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -8,9 +8,12 @@ ifeq "$(strip $(KERNEL))" ""
>  $(error KERNEL package name is missing, abort)
>  endif
>  
> +ABI := $(shell echo $(SNAPCRAFT_PROJECT_VERSION) | cut -f1-3 -d".")
> +
>  # rewriting variables passed from the outside environment doesn't work in LP,
>  # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
>  KERNELDEB := $(KERNEL)
> +KERNELPRE := linux-image-$(ABI)
>  
>  # linux-pc-image is a meta package used to indicate either
>  # linux-signed-image-generic or linux-image-generic, depending on the building
> @@ -18,6 +21,7 @@ KERNELDEB := $(KERNEL)
>  ifneq (,$(findstring linux-pc-image,$(KERNELDEB)))
>  ifneq (,$(findstring amd64,$(DPKG_ARCH)))
>  KERNELDEB := $(subst linux-pc-image,linux-signed-image-generic,$(KERNELDEB))
> +KERNELPRE := linux-signed-image-$(ABI)
>  else ifneq (,$(findstring i386,$(DPKG_ARCH)))
>  KERNELDEB := $(subst linux-pc-image,linux-image-generic,$(KERNELDEB))
>  else
> @@ -44,6 +48,7 @@ Pin-Priority: 700
>  endef
>  export APTPREF
>  
> +versioncheck: KIMGDEB = $(shell chroot chroot apt-cache depends $(KERNELDEB) | awk '/$(KERNELPRE)/ {print $$2}')
>  install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinuz-//;s/.efi.signed$$//')
>  
>  all:
> @@ -68,7 +73,7 @@ all:
>   umount chroot/sys
>   umount chroot/proc
>  
> -install:
> +install: versioncheck
>   mkdir -p $(DESTDIR)/lib $(DESTDIR)/meta $(DESTDIR)/firmware $(DESTDIR)/modules
>   if [ -f chroot/boot/vmlinu?-*.signed ]; then \
>    mv chroot/boot/vmlinu?-*.signed $(DESTDIR)/kernel.img; \
> @@ -115,3 +120,16 @@ install:
>   cd $(DESTDIR); ln -s kernel.img vmlinuz-$(KVERS)
>   cd $(DESTDIR); ln -s kernel.img vmlinuz
>   cd $(DESTDIR); ln -s initrd.img initrd.img-$(KVERS)
> +
> +versioncheck:
> + { \
> + set -e; \
> + echo $(KIMGDEB); \
> + KIMGVER="$$(dpkg --root=chroot -l | awk '/$(KIMGDEB)/ {print $$3}')"; \
> + echo $$KIMGVER; \
> + [ ! $$KIMGVER ] && echo "Unable to extract KIMGVER, exit" && exit 1; \
> + if [ $$KIMGVER != $(SNAPCRAFT_PROJECT_VERSION) ]; then \
> +  echo "Version mismatch:\nInstalled: $$KIMGVER Requested: $(SNAPCRAFT_PROJECT_VERSION)"; \
> +  exit 1; \
> + fi; \
> + }
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

That looks much better.  I guess this has been tested and so:

Acked-by: Andy Whitcroft <[hidden email]>

-apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH v2] deb to snap: recurse down to find the real kernel image package, and chek its version

Kleber Souza
In reply to this post by Paolo Pisati-5
On 11/15/17 17:46, Paolo Pisati wrote:
> Signed-off-by: Paolo Pisati <[hidden email]>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  Makefile | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index dad3156..409bbc3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -8,9 +8,12 @@ ifeq "$(strip $(KERNEL))" ""
>  $(error KERNEL package name is missing, abort)
>  endif
>  
> +ABI := $(shell echo $(SNAPCRAFT_PROJECT_VERSION) | cut -f1-3 -d".")
> +
>  # rewriting variables passed from the outside environment doesn't work in LP,
>  # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
>  KERNELDEB := $(KERNEL)
> +KERNELPRE := linux-image-$(ABI)
>  
>  # linux-pc-image is a meta package used to indicate either
>  # linux-signed-image-generic or linux-image-generic, depending on the building
> @@ -18,6 +21,7 @@ KERNELDEB := $(KERNEL)
>  ifneq (,$(findstring linux-pc-image,$(KERNELDEB)))
>  ifneq (,$(findstring amd64,$(DPKG_ARCH)))
>  KERNELDEB := $(subst linux-pc-image,linux-signed-image-generic,$(KERNELDEB))
> +KERNELPRE := linux-signed-image-$(ABI)
>  else ifneq (,$(findstring i386,$(DPKG_ARCH)))
>  KERNELDEB := $(subst linux-pc-image,linux-image-generic,$(KERNELDEB))
>  else
> @@ -44,6 +48,7 @@ Pin-Priority: 700
>  endef
>  export APTPREF
>  
> +versioncheck: KIMGDEB = $(shell chroot chroot apt-cache depends $(KERNELDEB) | awk '/$(KERNELPRE)/ {print $$2}')
>  install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinuz-//;s/.efi.signed$$//')
>  
>  all:
> @@ -68,7 +73,7 @@ all:
>   umount chroot/sys
>   umount chroot/proc
>  
> -install:
> +install: versioncheck
>   mkdir -p $(DESTDIR)/lib $(DESTDIR)/meta $(DESTDIR)/firmware $(DESTDIR)/modules
>   if [ -f chroot/boot/vmlinu?-*.signed ]; then \
>    mv chroot/boot/vmlinu?-*.signed $(DESTDIR)/kernel.img; \
> @@ -115,3 +120,16 @@ install:
>   cd $(DESTDIR); ln -s kernel.img vmlinuz-$(KVERS)
>   cd $(DESTDIR); ln -s kernel.img vmlinuz
>   cd $(DESTDIR); ln -s initrd.img initrd.img-$(KVERS)
> +
> +versioncheck:
> + { \
> + set -e; \
> + echo $(KIMGDEB); \
> + KIMGVER="$$(dpkg --root=chroot -l | awk '/$(KIMGDEB)/ {print $$3}')"; \
> + echo $$KIMGVER; \
> + [ ! $$KIMGVER ] && echo "Unable to extract KIMGVER, exit" && exit 1; \
> + if [ $$KIMGVER != $(SNAPCRAFT_PROJECT_VERSION) ]; then \
> +  echo "Version mismatch:\nInstalled: $$KIMGVER Requested: $(SNAPCRAFT_PROJECT_VERSION)"; \
> +  exit 1; \
> + fi; \
> + }
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Acked] [PATCH v2] deb to snap: recurse down to find the real kernel image package, and chek its version

Paolo Pisati-5
In reply to this post by Andy Whitcroft-3
Yep, i rebuilt all our kernel snaps before sending it out.

On Wed, Nov 15, 2017 at 6:51 PM, Andy Whitcroft <[hidden email]> wrote:

> On Wed, Nov 15, 2017 at 05:46:14PM +0100, Paolo Pisati wrote:
>> Signed-off-by: Paolo Pisati <[hidden email]>
>> ---
>>  Makefile | 20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index dad3156..409bbc3 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -8,9 +8,12 @@ ifeq "$(strip $(KERNEL))" ""
>>  $(error KERNEL package name is missing, abort)
>>  endif
>>
>> +ABI := $(shell echo $(SNAPCRAFT_PROJECT_VERSION) | cut -f1-3 -d".")
>> +
>>  # rewriting variables passed from the outside environment doesn't work in LP,
>>  # so use KERNELDEB as a temporary local variable to hold the kernel pkg name
>>  KERNELDEB := $(KERNEL)
>> +KERNELPRE := linux-image-$(ABI)
>>
>>  # linux-pc-image is a meta package used to indicate either
>>  # linux-signed-image-generic or linux-image-generic, depending on the building
>> @@ -18,6 +21,7 @@ KERNELDEB := $(KERNEL)
>>  ifneq (,$(findstring linux-pc-image,$(KERNELDEB)))
>>  ifneq (,$(findstring amd64,$(DPKG_ARCH)))
>>  KERNELDEB := $(subst linux-pc-image,linux-signed-image-generic,$(KERNELDEB))
>> +KERNELPRE := linux-signed-image-$(ABI)
>>  else ifneq (,$(findstring i386,$(DPKG_ARCH)))
>>  KERNELDEB := $(subst linux-pc-image,linux-image-generic,$(KERNELDEB))
>>  else
>> @@ -44,6 +48,7 @@ Pin-Priority: 700
>>  endef
>>  export APTPREF
>>
>> +versioncheck: KIMGDEB = $(shell chroot chroot apt-cache depends $(KERNELDEB) | awk '/$(KERNELPRE)/ {print $$2}')
>>  install : KVERS = $(shell ls -1 chroot/boot/vmlinuz-*| tail -1 |sed 's/^.*vmlinuz-//;s/.efi.signed$$//')
>>
>>  all:
>> @@ -68,7 +73,7 @@ all:
>>       umount chroot/sys
>>       umount chroot/proc
>>
>> -install:
>> +install: versioncheck
>>       mkdir -p $(DESTDIR)/lib $(DESTDIR)/meta $(DESTDIR)/firmware $(DESTDIR)/modules
>>       if [ -f chroot/boot/vmlinu?-*.signed ]; then \
>>         mv chroot/boot/vmlinu?-*.signed $(DESTDIR)/kernel.img; \
>> @@ -115,3 +120,16 @@ install:
>>       cd $(DESTDIR); ln -s kernel.img vmlinuz-$(KVERS)
>>       cd $(DESTDIR); ln -s kernel.img vmlinuz
>>       cd $(DESTDIR); ln -s initrd.img initrd.img-$(KVERS)
>> +
>> +versioncheck:
>> +     { \
>> +     set -e; \
>> +     echo $(KIMGDEB); \
>> +     KIMGVER="$$(dpkg --root=chroot -l | awk '/$(KIMGDEB)/ {print $$3}')"; \
>> +     echo $$KIMGVER; \
>> +     [ ! $$KIMGVER ] && echo "Unable to extract KIMGVER, exit" && exit 1; \
>> +     if [ $$KIMGVER != $(SNAPCRAFT_PROJECT_VERSION) ]; then \
>> +       echo "Version mismatch:\nInstalled: $$KIMGVER Requested: $(SNAPCRAFT_PROJECT_VERSION)"; \
>> +       exit 1; \
>> +     fi; \
>> +     }
>> --
>> 2.7.4
>>
>>
>> --
>> kernel-team mailing list
>> [hidden email]
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
> That looks much better.  I guess this has been tested and so:
>
> Acked-by: Andy Whitcroft <[hidden email]>
>
> -apw



--
bye,
p.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [PATCH 0/2] deb to snap: use the kernel version from the

Stefan Bader-2
In reply to this post by Paolo Pisati-5
On 31.10.2017 19:02, Paolo Pisati wrote:

> It was pointed to us that using the version from the linux-image-$BRANCH (e.g.
> linux-image-generic) package is not a good idea, since that version doesn't map
> 1:1 to the real kernel that comes from the linux-image-$VER-$ABI-$BRANCH (e.g.
> linux-image-4.4.0-98-generic) package - in other words, the same linux meta
> package could actually end up using two completely different kernels since it
> doesn't keep track of the upload number, and as such we won't be able to tell
> univocally which kernel image ended up in the produced kernel snap: to fix this,
> i added a mechanism that, given the meta package, it recurse down to find the
> real linux kernel package, extract its version and compares it against the
> expected version (while previously we used the "apt-get install
> $(KERNEL)=$(VERSION)" to force the installation of exactly that linux meta
> version, something we can't do anymore now as explained above).
>
> Patch 0001 is a prerequisite for patch 2, and improve over our apt-pinning
> method to let us use only kernel packages from the proposed pocket, and remove
> the "apt-get install $(KERNEL)=$(VERSION)" mechanism - now we'll just install
> the latest version of the required package available.
> Patch 0002 is where the real version extraction and checking happens, and it
> does so by making the checkversion shell snippet a prerequisite of the install
> rule.
>
> I tested the above by rebuilding all our kernel snaps on LP:
>
> pc-kernel - i386 and amd64
> raspi2-kernel - armhf
> snapdragon-kernel - arm64
> gke, gcp and aws - amd64
>
> and testing all the possible error paths or conditions.
>
> Since these patches don't change anything in the content on the generated kernel
> snap (actually they add a stricter version checking and a better pinning), the
> risk of regression is very low.
>
> This series of patches apply to our 'deb to snap' packaging scripts located
> here:
>    
> git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux-snap/+git/xenial
>
> and should be applied to the master Makefile, while all the topic
> branches should get the version field (version: in snapcraft.yaml) changed to
> point to the version from the linux-image-$VER-$ABI-$BRANCH corresponding
> package.
>
> In other words, and at the time of this writing, these are the version that
> should be used for all the topic branches:
>
> pc-kernel: 4.4.0-98.121 (linux-image-4.4.0-98-generic / linux-signed-image-4.4.0-98-generic)
> raspi2-kernel: 4.4.0-1076.84 (linux-image-4.4.0-1076-raspi2)
> snapdragon-kernel: 4.4.0-1078.83 (linux-image-4.4.0-1078-snapdragon)
> aws-kernel: 4.4.0-1039.48 (linux-image-4.4.0-1039-aws)
> gcp-kernel: 4.10.0-1008.8 (linux-image-4.10.0-1008-gcp)
> gke-kernel: 4.4.0-1033.33 (linux-image-4.4.0-1033-gke)
>
> I could have sent a separate version patch for every branch, but since it takes
> some time to get stuff reviewed and applied, and you might want to use the kernel
> available in -proposed instead of the one in -security/updates, i'll leave the
> version extraction to you.
>
> Paolo Pisati (2):
>   deb to snap: better apt-pinning, use $RELEASE everywhere and
>     explicitly exclude linux-firmware from -proposed
>   deb to snap: recurse down to find the real kernel image package, and
>     chek its version
>
>  Makefile | 42 +++++++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 13 deletions(-)
>
Acked-by: Stefan Bader <[hidden email]>

For completeness another ack for the bundle of 1 and 2v2 of patches.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment