public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCHv3] wic/plugins: fix kernel version detection for bios
@ 2021-02-05 11:48 Henning Schild
  2021-02-05 14:45 ` Anton Mikanovich
  2021-02-10  9:19 ` Anton Mikanovich
  0 siblings, 2 replies; 6+ messages in thread
From: Henning Schild @ 2021-02-05 11:48 UTC (permalink / raw)
  To: isar-users; +Cc: Vijai Kumar K, Henning Schild

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>
---
 meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv3] wic/plugins: fix kernel version detection for bios
  2021-02-05 11:48 [PATCHv3] wic/plugins: fix kernel version detection for bios Henning Schild
@ 2021-02-05 14:45 ` Anton Mikanovich
  2021-02-05 14:59   ` Baurzhan Ismagulov
  2021-02-10  9:19 ` Anton Mikanovich
  1 sibling, 1 reply; 6+ messages in thread
From: Anton Mikanovich @ 2021-02-05 14:45 UTC (permalink / raw)
  To: isar-users

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>

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv3] wic/plugins: fix kernel version detection for bios
  2021-02-05 14:45 ` Anton Mikanovich
@ 2021-02-05 14:59   ` Baurzhan Ismagulov
  2021-02-05 19:55     ` Henning Schild
  0 siblings, 1 reply; 6+ messages in thread
From: Baurzhan Ismagulov @ 2021-02-05 14:59 UTC (permalink / raw)
  To: isar-users

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv3] wic/plugins: fix kernel version detection for bios
  2021-02-05 14:59   ` Baurzhan Ismagulov
@ 2021-02-05 19:55     ` Henning Schild
  2021-02-05 20:07       ` Henning Schild
  0 siblings, 1 reply; 6+ messages in thread
From: Henning Schild @ 2021-02-05 19:55 UTC (permalink / raw)
  To: Baurzhan Ismagulov; +Cc: isar-users

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.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv3] wic/plugins: fix kernel version detection for bios
  2021-02-05 19:55     ` Henning Schild
@ 2021-02-05 20:07       ` Henning Schild
  0 siblings, 0 replies; 6+ messages in thread
From: Henning Schild @ 2021-02-05 20:07 UTC (permalink / raw)
  To: Baurzhan Ismagulov; +Cc: isar-users

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.
> >   
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv3] wic/plugins: fix kernel version detection for bios
  2021-02-05 11:48 [PATCHv3] wic/plugins: fix kernel version detection for bios Henning Schild
  2021-02-05 14:45 ` Anton Mikanovich
@ 2021-02-10  9:19 ` Anton Mikanovich
  1 sibling, 0 replies; 6+ messages in thread
From: Anton Mikanovich @ 2021-02-10  9:19 UTC (permalink / raw)
  To: Henning Schild, isar-users; +Cc: Vijai Kumar K

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.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-10  9:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 11:48 [PATCHv3] wic/plugins: fix kernel version detection for bios Henning Schild
2021-02-05 14:45 ` Anton Mikanovich
2021-02-05 14:59   ` Baurzhan Ismagulov
2021-02-05 19:55     ` Henning Schild
2021-02-05 20:07       ` Henning Schild
2021-02-10  9:19 ` Anton Mikanovich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox