[kernel-snaps-u24.04][pc] components: add support for nvidia-550 kernel components

Thu Oct 3 15:56:48 UTC 2024

Oh this raises so many questions :-)

On Thu,  3 Oct 2024 15:42:11 +1000
Portia Stephens <portia.stephens at canonical.com> wrote:

> This adds the kernel component for nvidia 550 kernel drivers to be published
> alongside the pc-kernel snap. There exist two hooks, assemble.sh which
> is run when the component is installed and provides the linking of the
> ko files and remove.sh which removes the ko files when the component is
> uninstalled.

We probably want a buglink that explains what this is and why we want to
add this to a stable snap. Based on the commit message alone it can't be
acked. Links to internal docs, spec, .. are OK IMO.

> 
> Signed-off-by: Portia Stephens <portia.stephens at canonical.com>
> Signed-off-by: Aaron Jauregui <aaron.jauregui at canonical.com>
> ---
>  files/meta/kernel.yaml      |  1 +
>  nvidia-ko-hooks/assemble.sh | 18 ++++++++++++++++++
>  nvidia-ko-hooks/remove.sh   |  6 ++++++

Are these two general scripts that will be reused in future snaps? If so,
shouldn't they go into https://git.launchpad.net/canonical-kernel-snaps
where the other general scripts live (like copy-firmware).

Speaking of that repo, there are also nvidia-assemble thingies, are these
related somehow? If not, what they for? Confused...

>  snapcraft.yaml              | 32 ++++++++++++++++++++++++++++++++
>  4 files changed, 57 insertions(+)
>  create mode 100644 files/meta/kernel.yaml
>  create mode 100644 nvidia-ko-hooks/assemble.sh
>  create mode 100644 nvidia-ko-hooks/remove.sh
> 
> diff --git a/files/meta/kernel.yaml b/files/meta/kernel.yaml
> new file mode 100644
> index 0000000..aa09f00
> --- /dev/null
> +++ b/files/meta/kernel.yaml
> @@ -0,0 +1 @@
> +dynamic-modules: $SNAP_DATA
> diff --git a/nvidia-ko-hooks/assemble.sh b/nvidia-ko-hooks/assemble.sh
> new file mode 100644
> index 0000000..e1c0834
> --- /dev/null
> +++ b/nvidia-ko-hooks/assemble.sh
> @@ -0,0 +1,18 @@
> +#!/bin/bash
> +
> +set -eux
> +
> +mkdir $SNAP_COMMON/nvidia-ko
> +cp -r $SNAP_COMPONENT/bits $SNAP_COMMON/nvidia-ko/bits
> +
> +cd $SNAP_COMMON/nvidia-ko/bits
> +
> +sed -i "s|/usr/bin/ld.bfd|$SNAP_COMPONENT/bin/ld.bfd|" BUILD
> +sed -i "s|make|$SNAP_COMPONENT/bin/make|" BUILD
> +
> +# TODO rewrite lib path for more generic
> +LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$SNAP_COMPONENT/lib/x86_64-linux-gnu sh BUILD

aarch64?

> +
> +mkdir -p $SNAP_DATA/modules/$(uname -r)
> +
> +mv ../*.ko $SNAP_DATA/modules/$(uname -r)/

$ shellcheck assemble.sh 

In assemble.sh line 5:
mkdir $SNAP_COMMON/nvidia-ko
      ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
mkdir "$SNAP_COMMON"/nvidia-ko

In assemble.sh line 6:
cp -r $SNAP_COMPONENT/bits $SNAP_COMMON/nvidia-ko/bits
      ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                           ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
cp -r "$SNAP_COMPONENT"/bits "$SNAP_COMMON"/nvidia-ko/bits

In assemble.sh line 8:
cd $SNAP_COMMON/nvidia-ko/bits
   ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
cd "$SNAP_COMMON"/nvidia-ko/bits

In assemble.sh line 16:
mkdir -p $SNAP_DATA/modules/$(uname -r)
         ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                            ^---------^ SC2046 (warning): Quote this to prevent word splitting.

Did you mean: 
mkdir -p "$SNAP_DATA"/modules/$(uname -r)

In assemble.sh line 18:
mv ../*.ko $SNAP_DATA/modules/$(uname -r)/
           ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                              ^---------^ SC2046 (warning): Quote this to prevent word splitting.

Did you mean: 
mv ../*.ko "$SNAP_DATA"/modules/$(uname -r)/

For more information:
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

> diff --git a/nvidia-ko-hooks/remove.sh b/nvidia-ko-hooks/remove.sh
> new file mode 100644
> index 0000000..b0547cc
> --- /dev/null
> +++ b/nvidia-ko-hooks/remove.sh
> @@ -0,0 +1,6 @@
> +#!/bin/bash
> +
> +set -eux
> +
> +rm -rf $SNAP_COMMON/nvidia-ko
> +rm -rf $SNAP_DATA/modules/$(uname -r)

$ shellcheck remove.sh 

In remove.sh line 5:
rm -rf $SNAP_COMMON/nvidia-ko
       ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
rm -rf "$SNAP_COMMON"/nvidia-ko

In remove.sh line 6:
rm -rf $SNAP_DATA/modules/$(uname -r)
       ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                          ^---------^ SC2046 (warning): Quote this to prevent word splitting.

Did you mean: 
rm -rf "$SNAP_DATA"/modules/$(uname -r)

For more information:
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

> diff --git a/snapcraft.yaml b/snapcraft.yaml
> index c08095e..64a6a84 100644
> --- a/snapcraft.yaml
> +++ b/snapcraft.yaml
> @@ -14,6 +14,13 @@ platforms:
>    amd64:
>    arm64:
>  
> +components:
> +  nvidia-550-ko:

I'm not familiar with components. Do we need it for both amd64 and arm64?

> +    type: test

test?

> +    summary: nvidia 550 kernel objects
> +    description: nvidia 550 kernel objects
> +    version: "0.1"

Shouldn't we have a dynamic version here that contains the kernel's ABI or
even match the kernel snap version?
Currently this produces:
pc-kernel+nvidia-550-ko_0.1.comp

It's hard to tell what versioning makes sense without knowing all possible
use cases of these components.

> +
>  parts:
>    kernel:
>      source: https://git.launchpad.net/canonical-kernel-snaps
> @@ -78,3 +85,28 @@ parts:
>        mkdir "$CRAFT_PART_INSTALL"/firmware/updates
>  
>        craftctl default
> +
> +  nvidia-550-ko-comp:
> +    source: ./nvidia-ko-hooks
> +    plugin: nil
> +
> +    stage-packages:
> +      - binutils
> +      - make
> +
> +    override-build: |
> +      craftctl default
> +      #clean up unnecessary libs
> +      rm -f -- $CRAFT_PART_INSTALL/usr/lib/x86_64-linux-gnu/libc.so.6

Yikes, why? And what about aarch64?

> +      apt-get download linux-objects-nvidia-550-server-6.8.0-41-generic

Hard-coded kernel version? That doesn't seem right.

> +      dpkg-deb -x *.deb nvidia-objects
> +      mv nvidia-objects/lib/modules/*/kernel/nvidia-550srv/bits $CRAFT_PART_INSTALL/
> +      mv assemble.sh $CRAFT_PART_INSTALL/assemble.sh
> +      mv remove.sh $CRAFT_PART_INSTALL/remove.sh
> +
> +    organize:
> +      bits/: (component/nvidia-550-ko)/bits
> +      usr/bin: (component/nvidia-550-ko)/bin
> +      usr/lib: (component/nvidia-550-ko)/lib
> +      assemble.sh: (component/nvidia-550-ko)/snap/hooks/install
> +      remove.sh: (component/nvidia-550-ko)/snap/hooks/remove

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20241003/bd8fd74c/attachment.sig>