Re: [PATCH v2 2/3] Revert "image.bbclass: make KERNEL_IMAGE & INITRD_IMAGE variable fixed"

["Maxim Yu. Osipov" <mosipov@ilbers.de>]

On 5/13/19 1:05 PM, Claudius Heine wrote:
> Hi Maxim,
> 
> On 13/05/2019 11.01, Maxim Yu. Osipov wrote:
>> On 5/13/19 10:11 AM, Claudius Heine wrote:
>>> Hi Maxim,
>>>
>>> On 13/05/2019 09.39, Maxim Yu. Osipov wrote:
>>>> On 5/13/19 9:31 AM, Claudius Heine wrote:
>>>>> Hi Maxim,
>>>>>
>>>>> On 12/05/2019 21.30, Maxim Yu. Osipov wrote:
>>>>>> This reverts commit b92589c574596ac98f97cf14369cbf62872e6481.
>>>>>>
>>>>>> vmlinuz and initrd are getting overwritten for the same machine
>>>>>> by other distros under deploy/images/${MACHINE}
>>>>>
>>>>> NACK. I don't see a patch that addresses the issue described in the 
>>>>> commit message of that reverted patch in this patchset in a 
>>>>> different way, so this is a regression.
>>>>
>>>>
>>>> Do you confirm that issue #53 appeared after applying your series 
>>>> into the 'next'?
>>>>
>>>> just look in
>>>> http://isar-build.org:8080/job/isar_next/ws/build/tmp/deploy/images/qemuarm/ 
>>>>
>>>>
>>>> vmlinuz/initrd for stretch was overwritten by subsequent build for 
>>>> buster.
>>>
>>> So would not the fix for that be to add the DISTRO variable to the 
>>> image deploy path or the the vmlinuz/initrd file name?
>>
>> I agree with the intention of your patch b92589c5 "image.bbclass: make 
>> KERNEL_IMAGE & INITRD_IMAGE variable fixed"  - I also had headaches 
>> with variables KERNEL_IMAGE/INITRD_IMAGE when trying to use them. 
>> Unfortunately, this patch leads to the mentioned problem with 
>> overwriting vmlinuz/initrd images for the same MACHINE. >
>> In my opinion the MACHINE deploy images w/o the patch use good naming 
>> scheme (see below).
>>
>> 1) The version of kernel is worth to see,
> 
> I disagree, since the version of the kernel/initrd makes the file names 
> in the deploy directory unpredictable, which is difficult to write 
> documentation and scripts for.

 From developer's point of view yes, from user's perspective these names 
are OK.

> Have you tried something like this?
> 
>    KERNEL_IMAGE ?= "${PF}.vmlinuz"
>    INITRD_IMAGE ?= "${PF}.initrd.img"

No, I've not tried.

If you have your proposal regarding the issue #56 fix - your patches are 
always welcome.

The problem that the 'next' branch has serious bug for a long time and 
this has to be fixed ASAP.

Regards,
Maxim.

> regards,
> Claudius
> 
>> not sure that one should add another directory DISTRO and distro name 
>> is included into vmlinuz/initrd.
>>
>>
>> http://isar-build.org:8080/job/isar_master/ws/build/tmp/deploy/images/qemuarm/ 
>>
>>
>> isar-image-base-debian-buster-qemuarm.ext4.img
>> isar-image-base-debian-buster-qemuarm.initrd.img-4.19.0-4-armmp
>> isar-image-base-debian-buster-qemuarm.vmlinuz-4.19.0-4-armmp
>> isar-image-base-debian-stretch-qemuarm.ext4.img
>> isar-image-base-debian-stretch-qemuarm.initrd.img-4.9.0-9-armmp
>> isar-image-base-debian-stretch-qemuarm.vmlinuz-4.9.0-9-armmp
>>
>>
>> 2) Another point is the image name itself - just compare 
>> isar-image-base and isar-image-debug - it's also worth to add image 
>> name into vmlinuz/initrd, f.e.:
>>
>> isar/build/tmp/deploy/images/qemuamd64:
>> isar-image-base-debian-stretch-qemuamd64.initrd.img-4.9.0-9-amd64
>> isar-image-base-debian-stretch-qemuamd64.wic.img 
>> isar-image-base-debian-stretch-qemuamd64.vmlinuz-4.9.0-9-amd64
>> isar-image-debug-debian-stretch-qemuamd64.vmlinuz-4.9.0-9-amd64
>> isar-image-debug-debian-stretch-qemuamd64.initrd.img-4.9.0-9-amd64 
>> isar-image-debug-debian-stretch-qemuamd64.wic.img
> 
> Your concerns are valid. We need to figure out a good system for the 
> file names and paths.
> 
>>
>> Reagrds,
>> Maxim.
>>
>>
>>
>>> regards,
>>> Claudius
>>>
>>>>
>>>> Regards,
>>>> Maxim.
>>>>
>>>>> regards,
>>>>> Claudius
>>>>>
>>>>>>
>>>>>> Closes: #56
>>>>>> Signed-off-by: Maxim Yu. Osipov <mosipov@ilbers.de>
>>>>>> ---
>>>>>>   meta/classes/image.bbclass | 6 ++++--
>>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>>>>>> index fb35a4c..8e6f2c9 100644
>>>>>> --- a/meta/classes/image.bbclass
>>>>>> +++ b/meta/classes/image.bbclass
>>>>>> @@ -13,8 +13,8 @@ IMAGE_INSTALL += "${@ ("linux-image-" + 
>>>>>> d.getVar("KERNEL_NAME", True)) if d.getV
>>>>>>   IMAGE_FULLNAME = "${PF}"
>>>>>>   # These variables are used by wic and start_vm
>>>>>> -KERNEL_IMAGE ?= "vmlinuz"
>>>>>> -INITRD_IMAGE ?= "initrd.img"
>>>>>> +KERNEL_IMAGE ?= "${@get_image_name(d, 'vmlinuz')[1]}"
>>>>>> +INITRD_IMAGE ?= "${@get_image_name(d, 'initrd.img')[1]}"
>>>>>>   # Useful variables for imager implementations:
>>>>>>   PP = "/home/builder/${PN}"
>>>>>> @@ -149,10 +149,12 @@ addtask rootfs_install before do_build after 
>>>>>> do_unpack
>>>>>>   do_copy_boot_files[dirs] = "${DEPLOY_DIR_IMAGE}"
>>>>>>   do_copy_boot_files() {
>>>>>> +    KERNEL_IMAGE=${@get_image_name(d, 'vmlinuz')[1]}
>>>>>>       if [ -n "${KERNEL_IMAGE}" ]; then
>>>>>>           cp -f ${IMAGE_ROOTFS}/boot/${@get_image_name(d, 
>>>>>> 'vmlinuz')[0]} ${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGE}
>>>>>>       fi
>>>>>> +    INITRD_IMAGE=${@get_image_name(d, 'initrd.img')[1]}
>>>>>>       if [ -n "${INITRD_IMAGE}" ]; then
>>>>>>           sudo cp -f ${IMAGE_ROOTFS}/boot/${@get_image_name(d, 
>>>>>> 'initrd.img')[0]} ${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}
>>>>>>       fi
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
> 


-- 
Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mosipov@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov