| Message ID | 20210205114812.6370-1-henning.schild@siemens.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [PATCHv3] wic/plugins: fix kernel version detection for bios | expand |
Thanks for finding. This issue can be found not on even kernel version numbers, but on all version end on 4 and 6, because '-amd64' is used as char values to be stripped both from begin and from end. Usage of strip() looks completely wrong here. Looks good to me. 05.02.2021 14:48, Henning Schild wrote: > From: Henning Schild <henning.schild@siemens.com> > > When building an image with legacy bios using wic, wic can fail to pick > up the initrd. > > ERROR: _exec_cmd: install -m 0644 /build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64 /tmp/tmp.7x9N9Wo4wZ/bla-image-debian-buster-qemux86-64.wic/tmp.wic.r10macew/hdd/boot/initrd.img-4.19.0-1-amd64 returned '1' instead of 0 > output: install: cannot stat '/build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64': No such file or directory > > The mechanism used to "cut off the end" seems to cut off too much. > >>>> "vmlinuz-4.19.0-14-amd64".strip('-' + 'amd64') > 'vmlinuz-4.19.0-1' > > But indeed we would hope for 'vmlinuz-4.19.0-14'. Seems that "even" > revision numbers trigger the problem, while "odd" works. > > Signed-off-by: Henning Schild <henning.schild@siemens.com>
On Fri, Feb 05, 2021 at 05:45:27PM +0300, Anton Mikanovich wrote: > Thanks for finding. > This issue can be found not on even kernel version numbers, but on all > version end on 4 and 6, because '-amd64' is used as char values to be > stripped both from begin and from end. Usage of strip() looks completely > wrong here. > Looks good to me. If there are no objections, I'd suggest to apply this on Monday. 1. Henning, would it be ok if we commit with the following message: From: Henning Schild <henning.schild@siemens.com> When building an image with legacy bios using wic, wic can fail to pick up the initrd. ERROR: _exec_cmd: install -m 0644 /build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64 /tmp/tmp.7x9N9Wo4wZ/bla-image-debian-buster-qemux86-64.wic/tmp.wic.r10macew/hdd/boot/initrd.img-4.19.0-1-amd64 returned '1' instead of 0 output: install: cannot stat '/build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64': No such file or directory The mechanism used to "cut off the end" seems to cut off too much. >>> "vmlinuz-4.19.0-14-amd64".strip('-' + 'amd64') 'vmlinuz-4.19.0-1' But indeed we would hope for 'vmlinuz-4.19.0-14'. The issue affects all versions ending with 4 and 6, because '-amd64' is used as a set of characters to be stripped anywhere in the string. Using strip() is completely wrong here. Signed-off-by: Henning Schild <henning.schild@siemens.com> 2. We haven't seen this on Isar CI. Care to add a test case? With kind regards, Baurzhan.
Am Fri, 5 Feb 2021 15:59:54 +0100 schrieb Baurzhan Ismagulov <ibr@radix50.net>: > On Fri, Feb 05, 2021 at 05:45:27PM +0300, Anton Mikanovich wrote: > > Thanks for finding. > > This issue can be found not on even kernel version numbers, but on > > all version end on 4 and 6, because '-amd64' is used as char values > > to be stripped both from begin and from end. Usage of strip() looks > > completely wrong here. > > Looks good to me. > > If there are no objections, I'd suggest to apply this on Monday. Given that it can not be reproduced by you, i would take back the "rush". Probably still worth merging, my layer is currently fixed with a backport using the kas patch mechanism. > > 1. Henning, would it be ok if we commit with the following message: Please send me a diff or go ahead and change it. > From: Henning Schild <henning.schild@siemens.com> > > When building an image with legacy bios using wic, wic can fail to > pick up the initrd. > > ERROR: _exec_cmd: install -m 0644 > /build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64 > /tmp/tmp.7x9N9Wo4wZ/bla-image-debian-buster-qemux86-64.wic/tmp.wic.r10macew/hdd/boot/initrd.img-4.19.0-1-amd64 > returned '1' instead of 0 output: install: cannot stat > '/build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64': > No such file or directory > > The mechanism used to "cut off the end" seems to cut off too much. > > >>> "vmlinuz-4.19.0-14-amd64".strip('-' + 'amd64') > 'vmlinuz-4.19.0-1' > > But indeed we would hope for 'vmlinuz-4.19.0-14'. The issue affects > all versions ending with 4 and 6, because '-amd64' is used as a set of > characters to be stripped anywhere in the string. Using strip() is > completely wrong here. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > > > 2. We haven't seen this on Isar CI. Care to add a test case? I am a big fan of test cases, but am not sure how to add one here. As far as i understand it the pipeline should fail for amd64. And for all other arches that have numbers that could be in the revisions. So i would say there is no need, sorry i do not understand why you do not see it. Might also be related to the python version, but we should be using the one from the buildchroot. Henning > > With kind regards, > Baurzhan. >
Am Fri, 5 Feb 2021 20:55:42 +0100 schrieb "[ext] Henning Schild" <henning.schild@siemens.com>: > Am Fri, 5 Feb 2021 15:59:54 +0100 > schrieb Baurzhan Ismagulov <ibr@radix50.net>: > > > On Fri, Feb 05, 2021 at 05:45:27PM +0300, Anton Mikanovich wrote: > > > Thanks for finding. > > > This issue can be found not on even kernel version numbers, but on > > > all version end on 4 and 6, because '-amd64' is used as char > > > values to be stripped both from begin and from end. Usage of > > > strip() looks completely wrong here. > > > Looks good to me. > > > > If there are no objections, I'd suggest to apply this on Monday. > > Given that it can not be reproduced by you, i would take back the > "rush". Probably still worth merging, my layer is currently fixed with > a backport using the kas patch mechanism. > > > > > 1. Henning, would it be ok if we commit with the following message: > > > > Please send me a diff or go ahead and change it. > > > From: Henning Schild <henning.schild@siemens.com> > > > > When building an image with legacy bios using wic, wic can fail to > > pick up the initrd. > > > > ERROR: _exec_cmd: install -m 0644 > > /build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64 > > /tmp/tmp.7x9N9Wo4wZ/bla-image-debian-buster-qemux86-64.wic/tmp.wic.r10macew/hdd/boot/initrd.img-4.19.0-1-amd64 > > returned '1' instead of 0 output: install: cannot stat > > '/build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64': > > No such file or directory > > > > The mechanism used to "cut off the end" seems to cut off too much. > > > > >>> "vmlinuz-4.19.0-14-amd64".strip('-' + 'amd64') > > 'vmlinuz-4.19.0-1' > > > > But indeed we would hope for 'vmlinuz-4.19.0-14'. The issue affects > > all versions ending with 4 and 6, because '-amd64' is used as a set > > of characters to be stripped anywhere in the string. Using strip() > > is completely wrong here. > > > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > > > > > > 2. We haven't seen this on Isar CI. Care to add a test case? > > I am a big fan of test cases, but am not sure how to add one here. As > far as i understand it the pipeline should fail for amd64. And for all > other arches that have numbers that could be in the revisions. > > So i would say there is no need, sorry i do not understand why you do > not see it. Might also be related to the python version, but we should > be using the one from the buildchroot. I think the regular ci does not find it because the arch is "i386" or "686-pae", non of the numbers in "14". So a testcase would likely be adding "amd64" for WKS_FILE ?= "directdisk-isar". Or waiting for a "-16" kernel which should hit "386" "686" and "amd64". Henning > Henning > > > > > With kind regards, > > Baurzhan. > > >
05.02.2021 14:48, Henning Schild wrote: > From: Henning Schild <henning.schild@siemens.com> > > When building an image with legacy bios using wic, wic can fail to pick > up the initrd. > > ERROR: _exec_cmd: install -m 0644 /build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64 /tmp/tmp.7x9N9Wo4wZ/bla-image-debian-buster-qemux86-64.wic/tmp.wic.r10macew/hdd/boot/initrd.img-4.19.0-1-amd64 returned '1' instead of 0 > output: install: cannot stat '/build/tmp/work/debian-buster-amd64/bla-image-qemux86-64-wic-img/0.2-r0/rootfs/boot/initrd.img-4.19.0-1-amd64': No such file or directory > > The mechanism used to "cut off the end" seems to cut off too much. > >>>> "vmlinuz-4.19.0-14-amd64".strip('-' + 'amd64') > 'vmlinuz-4.19.0-1' > > But indeed we would hope for 'vmlinuz-4.19.0-14'. Seems that "even" > revision numbers trigger the problem, while "odd" works. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> Applied to next, thanks.
diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py b/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py index 493615b481..ece08efa76 100644 --- a/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py +++ b/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py @@ -135,7 +135,7 @@ class BootimgPcbiosIsarPlugin(SourcePlugin): kernel_name = get_bitbake_var("KERNEL_NAME") rootfs_dir = get_bitbake_var("IMAGE_ROOTFS") kernel = os.path.basename(os.path.realpath(os.path.join(rootfs_dir, kernel_file))) - kernel_version = kernel.strip('-' + kernel_name).strip(kernel_file + '-') + kernel_version = kernel[len(kernel_file)+1:-(len(kernel_name)+1)] initrd = "initrd.img-%s-%s" % (kernel_version, kernel_name) syslinux_conf += "KERNEL " + kernel + "\n" @@ -165,7 +165,7 @@ class BootimgPcbiosIsarPlugin(SourcePlugin): kernel_name = get_bitbake_var("KERNEL_NAME") rootfs_dir = rootfs_dir['ROOTFS_DIR'] kernel = os.path.basename(os.path.realpath(os.path.join(rootfs_dir, kernel_file))) - kernel_version = kernel.strip('-' + kernel_name).strip(kernel_file + '-') + kernel_version = kernel[len(kernel_file)+1:-(len(kernel_name)+1)] initrd = "initrd.img-%s-%s" % (kernel_version, kernel_name) config = "config-%s-%s" % (kernel_version, kernel_name) mapfile = "System.map-%s-%s" % (kernel_version, kernel_name)