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

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

Hi Claudius,

Do you plan to send your version of patch fixing #56?

If not, I'll apply now my v2 series "vmlinuz/initrd for the same MACHINE 
are overwritten" as the problem exists in the 'next' since 16th of April.

Regards,
Maxim.


On 5/13/19 1:16 PM, Maxim Yu. Osipov wrote:
> 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