Patchwork [2/3] boot-directdisk: Scope HDDDIR and HDDIMG variables to avoid conflicts

login
register
mail settings
Submitter Jonathan Liu
Date May 9, 2013, 3:24 a.m.
Message ID <1368069844-13706-2-git-send-email-net147@gmail.com>
Download mbox | patch
Permalink /patch/49609/
State Accepted
Commit f5b103ff47d6137326618328fa5803f0963e50f4
Headers show

Comments

Jonathan Liu - May 9, 2013, 3:24 a.m.
These variables should not be shared with other image classes.
The bootimg class also has an HDDDIR variable that could be overwritten
if executing concurrently in the same image recipe.

Signed-off-by: Jonathan Liu <net147@gmail.com>
---
 meta/classes/boot-directdisk.bbclass | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)
Darren Hart - June 7, 2013, 11:31 p.m.
On 05/08/2013 08:24 PM, Jonathan Liu wrote:
> These variables should not be shared with other image classes.
> The bootimg class also has an HDDDIR variable that could be overwritten
> if executing concurrently in the same image recipe.
> 

Nice catch. Would you considering sending a patch against the bootimg
class as well?

Thanks,

Darren

> Signed-off-by: Jonathan Liu <net147@gmail.com>
> ---
>  meta/classes/boot-directdisk.bbclass | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass
> index aa172c6..9673760 100644
> --- a/meta/classes/boot-directdisk.bbclass
> +++ b/meta/classes/boot-directdisk.bbclass
> @@ -28,9 +28,6 @@ do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
>  PACKAGES = " "
>  EXCLUDE_FROM_WORLD = "1"
>  
> -HDDDIR = "${S}/hdd/boot"
> -HDDIMG = "${S}/hdd.image"
> -
>  BOOTDD_VOLUME_ID   ?= "boot"
>  BOOTDD_EXTRA_SPACE ?= "16384"
>  
> @@ -43,14 +40,16 @@ SYSLINUX_TIMEOUT ?= "10"
>  inherit syslinux
>  		
>  build_boot_dd() {
> +	HDDDIR="${S}/hdd/boot"
> +	HDDIMG="${S}/hdd.image"
>  	IMAGE=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hdddirect
>  
> -	install -d ${HDDDIR}
> -	install -m 0644 ${STAGING_KERNEL_DIR}/bzImage ${HDDDIR}/vmlinuz
> -	install -m 0644 ${S}/syslinux.cfg ${HDDDIR}/syslinux.cfg
> -	install -m 444 ${STAGING_DATADIR}/syslinux/ldlinux.sys ${HDDDIR}/ldlinux.sys
> +	install -d $HDDDIR
> +	install -m 0644 ${STAGING_KERNEL_DIR}/bzImage $HDDDIR/vmlinuz
> +	install -m 0644 ${S}/syslinux.cfg $HDDDIR/syslinux.cfg
> +	install -m 444 ${STAGING_DATADIR}/syslinux/ldlinux.sys $HDDDIR/ldlinux.sys
>  
> -	BLOCKS=`du -bks ${HDDDIR} | cut -f 1`
> +	BLOCKS=`du -bks $HDDDIR | cut -f 1`
>  	BLOCKS=`expr $BLOCKS + ${BOOTDD_EXTRA_SPACE}`
>  
>  	# Ensure total sectors is an integral number of sectors per
> @@ -59,11 +58,11 @@ build_boot_dd() {
>  	# done in blocks, thus the mod by 16 instead of 32.
>  	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
>  
> -	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C ${HDDIMG} $BLOCKS 
> -	mcopy -i ${HDDIMG} -s ${HDDDIR}/* ::/
> +	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS 
> +	mcopy -i $HDDIMG -s $HDDDIR/* ::/
>  
> -	syslinux ${HDDIMG}
> -	chmod 644 ${HDDIMG}
> +	syslinux $HDDIMG
> +	chmod 644 $HDDIMG
>  
>  	ROOTFSBLOCKS=`du -Lbks ${ROOTFS} | cut -f 1`
>  	TOTALSIZE=`expr $BLOCKS + $ROOTFSBLOCKS`
> @@ -83,7 +82,7 @@ build_boot_dd() {
>  
>  	OFFSET=`expr $END2 / 512`
>  	dd if=${STAGING_DATADIR}/syslinux/mbr.bin of=$IMAGE conv=notrunc
> -	dd if=${HDDIMG} of=$IMAGE conv=notrunc seek=1 bs=512
> +	dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
>  	dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
>  
>  	cd ${DEPLOY_DIR_IMAGE}
>
Jonathan Liu - June 9, 2013, 12:11 p.m.
On 8 June 2013 09:31, Darren Hart <dvhart@linux.intel.com> wrote:
> On 05/08/2013 08:24 PM, Jonathan Liu wrote:
>> These variables should not be shared with other image classes.
>> The bootimg class also has an HDDDIR variable that could be overwritten
>> if executing concurrently in the same image recipe.
>
> Nice catch. Would you considering sending a patch against the bootimg
> class as well?

I did originally submit a patch against the bootimg class as well but
it turns out those variables are used by grub-efi.bbclass and
syslinux.bbclass. See
http://lists.openembedded.org/pipermail/openembedded-core/2013-May/078011.html.

Regards,
Jonathan
Darren Hart - June 10, 2013, 4:36 p.m.
On 06/09/2013 05:11 AM, Jonathan Liu wrote:
> On 8 June 2013 09:31, Darren Hart <dvhart@linux.intel.com> wrote:
>> On 05/08/2013 08:24 PM, Jonathan Liu wrote:
>>> These variables should not be shared with other image classes.
>>> The bootimg class also has an HDDDIR variable that could be overwritten
>>> if executing concurrently in the same image recipe.
>>
>> Nice catch. Would you considering sending a patch against the bootimg
>> class as well?
> 
> I did originally submit a patch against the bootimg class as well but
> it turns out those variables are used by grub-efi.bbclass and
> syslinux.bbclass. See
> http://lists.openembedded.org/pipermail/openembedded-core/2013-May/078011.html.

Ah yes. And boot-directdisk.bbclass also makes use of those. Sounds like
some investigation into how to better modularize this is in order.

Patch

diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass
index aa172c6..9673760 100644
--- a/meta/classes/boot-directdisk.bbclass
+++ b/meta/classes/boot-directdisk.bbclass
@@ -28,9 +28,6 @@  do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
 PACKAGES = " "
 EXCLUDE_FROM_WORLD = "1"
 
-HDDDIR = "${S}/hdd/boot"
-HDDIMG = "${S}/hdd.image"
-
 BOOTDD_VOLUME_ID   ?= "boot"
 BOOTDD_EXTRA_SPACE ?= "16384"
 
@@ -43,14 +40,16 @@  SYSLINUX_TIMEOUT ?= "10"
 inherit syslinux
 		
 build_boot_dd() {
+	HDDDIR="${S}/hdd/boot"
+	HDDIMG="${S}/hdd.image"
 	IMAGE=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hdddirect
 
-	install -d ${HDDDIR}
-	install -m 0644 ${STAGING_KERNEL_DIR}/bzImage ${HDDDIR}/vmlinuz
-	install -m 0644 ${S}/syslinux.cfg ${HDDDIR}/syslinux.cfg
-	install -m 444 ${STAGING_DATADIR}/syslinux/ldlinux.sys ${HDDDIR}/ldlinux.sys
+	install -d $HDDDIR
+	install -m 0644 ${STAGING_KERNEL_DIR}/bzImage $HDDDIR/vmlinuz
+	install -m 0644 ${S}/syslinux.cfg $HDDDIR/syslinux.cfg
+	install -m 444 ${STAGING_DATADIR}/syslinux/ldlinux.sys $HDDDIR/ldlinux.sys
 
-	BLOCKS=`du -bks ${HDDDIR} | cut -f 1`
+	BLOCKS=`du -bks $HDDDIR | cut -f 1`
 	BLOCKS=`expr $BLOCKS + ${BOOTDD_EXTRA_SPACE}`
 
 	# Ensure total sectors is an integral number of sectors per
@@ -59,11 +58,11 @@  build_boot_dd() {
 	# done in blocks, thus the mod by 16 instead of 32.
 	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
 
-	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C ${HDDIMG} $BLOCKS 
-	mcopy -i ${HDDIMG} -s ${HDDDIR}/* ::/
+	mkdosfs -n ${BOOTDD_VOLUME_ID} -S 512 -C $HDDIMG $BLOCKS 
+	mcopy -i $HDDIMG -s $HDDDIR/* ::/
 
-	syslinux ${HDDIMG}
-	chmod 644 ${HDDIMG}
+	syslinux $HDDIMG
+	chmod 644 $HDDIMG
 
 	ROOTFSBLOCKS=`du -Lbks ${ROOTFS} | cut -f 1`
 	TOTALSIZE=`expr $BLOCKS + $ROOTFSBLOCKS`
@@ -83,7 +82,7 @@  build_boot_dd() {
 
 	OFFSET=`expr $END2 / 512`
 	dd if=${STAGING_DATADIR}/syslinux/mbr.bin of=$IMAGE conv=notrunc
-	dd if=${HDDIMG} of=$IMAGE conv=notrunc seek=1 bs=512
+	dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
 	dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
 
 	cd ${DEPLOY_DIR_IMAGE}