mtd-self-test failure on arm64 (mtdram module)

While we have the luxury of time with kinetic, I'd rather we figure out why the test fails rather than just skipping it. If the test is failing, the plugin may fail too!

It seems that this code should be reading the "size" attribute from sysfs and comparing it
(here https://github.com/fwupd/fwupd/blob/main/plugins/mtd/fu-mtd-device.c#L161) to a static value (here https://github.com/fwupd/fwupd/blob/main/plugins/mtd/fu-self-test.c#L53).

That seems like a coding error to me. Maybe you can take a PR upstream to adjust it?

I suspect the self tests need to check the name is "mtdram test device" before running.

I think the question we'd need to ask first is: Why is there a /dev/mtd0 inside Ubuntu's arm64 autopkgtest VM?

At no point during the tests do we modprobe the "mtdram" driver. So it is expected that /dev/mtd0 is not there and this test is skipped (as is the case for Debian and Ubuntu != arm64). Also, when I launch a Ubuntu Karmic VM in OpenStack, it does not have /dev/mtd0, only after "modprobe mtdram" can I query it:
$ cat /sys/devices/virtual/mtd/mtd0/size
4194304
$ cat /sys/devices/virtual/mtd/mtd0/name
mtdram test device

The test is supposed to run against "mtdram", and 0x400000=4194304 is the default size of that module, so that constant value is actually correct IMO, but the tests should be loading that module prior to executing the test. And it's strange that Ubuntu/arm64 would have a different mtd0 device..

Is autopkgtest running on "real" hardware?

It's running in KVM. (armhf is the exception, running inside LXD container)

Here's the cleaner solution: https://github.com/fwupd/fwupd/commit/eca29c11faa5b1258be8fa253beeebe753bad4fa

Yes, I like this approach much better. LGTM!

I'd also suggest to call "modprobe mtdram" prior to running the tests, so that the new mtd test are actually executed. See attached debdiff.

This way it works for arm64 (skipping the test due to mtd0 not being provided by mtdram) and works for non-arm64 (using modprobe, as suggested above)

https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-slyon-testing/kinetic/arm64/f/fwupd/20220517_123619_edd79@/log.gz

https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-slyon-testing/kinetic/amd64/f/fwupd/20220517_122000_38022@/log.gz

WDYT? Can we upload it this way?

... Actually, we might want to do "modprobe mtdram || true" as this is basically optional.

Can you bring it to upstream packaging in contrib/debian?

I'm not sure what you mean by "contrib/debian"? The patch should apply cleanly to the upstream Debian package, too. (Well, one might want to change the version number.)

We keep the packaging at https://github.com/fwupd/fwupd/tree/main/contrib/debian/tests. If you can bring the review there, that is ideal as this is the root of what goes into Debian and Ubuntu.

Ah! Sure, no problem. I wasn't aware of this. Here you go: https://github.com/fwupd/fwupd/pull/4641

I only included the "modprobe" change in this PR, as IMO it does not really make sense to ship the distro patch in this upstream "contrib/debian" directory, as the upstream repo already contains that fix in the git history.

So we can either wait for the new version to be shipped, or upload an intermediate fix/package to Ubuntu or Debian, which contains the "0002-trivial-skip-mtd-tests-if-not-working-off-mtdram-tes.patch" distro patch (that would need to be dropped again, once it's upgraded to the new version, containing it natively).

> Ah! Sure, no problem. I wasn't aware of this. Here you go: https://github.com/fwupd/fwupd/pull/4641

Thanks!

> I only included the "modprobe" change in this PR, as IMO it does not really make sense to ship the distro patch in this upstream "contrib/debian" directory, as the upstream repo already contains that fix in the git history.

That matches what I thought made sense too.

> So we can either wait for the new version to be shipped, or upload an intermediate fix/package to Ubuntu or Debian, which contains the "0002-trivial-skip-mtd-tests-if-not-working-off-mtdram-tes.patch" distro patch (that would need to be dropped again, once it's upgraded to the new version, containing it natively).

For now to get Ubuntu to migrate (the only place this is failing) I'd say upload to kinetic directly. Debian will take 1.8.1 next which will take this patch and depending on where the cycle ends up in kinetic will sync at that time if so.

Sounds like a plan! Done.

Thanks for all of your feedback!

Thanks Lukas!

> ci FAIL stderr: modprobe: FATAL: Module mtdram not found in directory /lib/modules/5.4.0-109-generic

No good deed goes unpunished. I think you'll want to redirect the stderr output from modprobe to stdout to make autopkgtest happy.

Ugh! Indeed, fixed.

https://github.com/fwupd/fwupd/pull/4645
https://launchpad.net/ubuntu/+source/fwupd/1.7.7-1ubuntu2

This bug was fixed in the package fwupd - 1.7.7-1ubuntu2

---------------
fwupd (1.7.7-1ubuntu2) kinetic; urgency=medium

  * d/t/ci: don't stderr-fail the autopkgtest on modprobe error
    + it's optional as tests can be skipped, if mtdram module isn't there

 -- Lukas Märdian <email address hidden> Wed, 18 May 2022 09:34:56 +0200