Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
Hi, Saul
Sorry. I do not understand. What does the patch fix? After this patch is
applied, what is the difference?
Zhu Yanjun
On 01/23/2014 02:50 PM, Saul Wold wrote:
From: Joao Henrique Ferreira de Freitas
Previous change (086ce22b88f5ef5f75a83119a32c8b3fdcfa296d) broke
the creating of vmdk images. This protects shell expansion variables
and let dd generate the image to be transformed to vmdk by image-vmdk.class.
Signed-off-by: Joao Henrique Ferreira de Freitas
[edit to change the usage of IMAGE_FSTYPE to IS_VMDK]
Signed-off-by: Saul Wold
---
meta/classes/boot-directdisk.bbclass | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/meta/classes/boot-directdisk.bbclass
b/meta/classes/boot-directdisk.bbclass
index 3277666..afab843 100644
--- a/meta/classes/boot-directdisk.bbclass
+++ b/meta/classes/boot-directdisk.bbclass
@@ -61,6 +61,8 @@ DISK_SIGNATURE ?= "${DISK_SIGNATURE_GENERATED}"
SYSLINUX_ROOT ?= "root=/dev/sda2"
SYSLINUX_TIMEOUT ?= "10"
+IS_VMDK = '${@base_contains("IMAGE_FSTYPES", "vmdk", "true", "false", d)}'
+
boot_direct_populate() {
dest=$1
install -d $dest
@@ -88,10 +90,10 @@ build_boot_dd() {
grubefi_hddimg_populate $HDDDIR
fi
- if [ ${IMAGE_FSTYPE} = "vmdk" ]; then
- if [ x${AUTO_SYSLINUXMENU} = x1 ] ; then
+ if [ "${IS_VMDK}" ]; then
+ if [ "x${AUTO_SYSLINUXMENU}" = "x1" ] ; then
install -m 0644
${STAGING_DIR}/${MACHINE}/usr/share/syslinux/vesamenu.c32
${HDDDIR}${SYSLINUXDIR}/vesamenu.c32
- if [ x${SYSLINUX_SPLASH} != x ] ; then
+ if [ "x${SYSLINUX_SPLASH}" != "x" ] ; then
install -m 0644 ${SYSLINUX_SPLASH}
${HDDDIR}${SYSLINUXDIR}/splash.lss
fi
fi
@@ -129,9 +131,7 @@ build_boot_dd() {
parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
parted $IMAGE set 1 boot on
- if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
- parted $IMAGE print
- fi
+ parted $IMAGE print
awk "BEGIN { printf \"$(echo ${DISK_SIGNATURE} | fold -w 2 | tac | paste -sd '' | sed 's/\(..\)/\\x&/g')\" }" | \
dd of=$IMAGE bs=1 seek=440 conv=notrunc
@@ -141,10 +141,8 @@ build_boot_dd() {
dd if=${STAGING_DATADIR}/syslinux/mbr.bin of=$IMAGE conv=notrunc
fi
- if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
- dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
- dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
- fi
+ 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}
rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.hdddirect
--
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
On Thu, Jan 23, 2014 at 10:21 AM, Richard Purdie
wrote:
> On Thu, 2014-01-23 at 07:04 +, Phil Blundell wrote:
>> On Wed, 2014-01-22 at 22:50 -0800, Saul Wold wrote:
>> > +IS_VMDK = '${@base_contains("IMAGE_FSTYPES", "vmdk", "true", "false", d)}'
>> > +
>> > + if [ "${IS_VMDK}" ]; then
>>
>> Does that really work? I can't quite see how it would, but I might be
>> overlooking something. Presumably you did test this, right?
>
> I've merged this since I really would like to try and get the builds
> stabilised. I did fix the patch since it would always evaluate as true
> the way it was...
What is the point in having:
+ if [ "${IS_VMDK}" = "true" ]; then
This is an indirection that buys nothing.
- if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
- dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
- dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
- fi
+ dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
+ dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
This changes the logic but commit log is not clear /what/ it fixes. I
did valid comments in the patch and you didn't considered those.
I just wasted time reviewing it...
--
Otavio Salvador O.S. Systems
http://www.ossystems.com.brhttp://code.ossystems.com.br
Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
On Thu, 2014-01-23 at 07:04 +, Phil Blundell wrote:
> On Wed, 2014-01-22 at 22:50 -0800, Saul Wold wrote:
> > +IS_VMDK = '${@base_contains("IMAGE_FSTYPES", "vmdk", "true", "false", d)}'
> > +
> > + if [ "${IS_VMDK}" ]; then
>
> Does that really work? I can't quite see how it would, but I might be
> overlooking something. Presumably you did test this, right?
I've merged this since I really would like to try and get the builds
stabilised. I did fix the patch since it would always evaluate as true
the way it was...
Cheers,
Richard
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
On Thu, Jan 23, 2014 at 4:50 AM, Saul Wold wrote: > From: Joao Henrique Ferreira de Freitas > > Previous change (086ce22b88f5ef5f75a83119a32c8b3fdcfa296d) broke > the creating of vmdk images. This protects shell expansion variables > and let dd generate the image to be transformed to vmdk by image-vmdk.class. > > Signed-off-by: Joao Henrique Ferreira de Freitas > > [edit to change the usage of IMAGE_FSTYPE to IS_VMDK] > > Signed-off-by: Saul Wold This is hard to review as it has two mixed changes. It changes the IS_VMDK change and logical changes inside same patch. Please split it in two patches. Shell checking for true and false is Bash specific. $ if [ false ]; then echo "Is true"; else echo "Is false"; fi Is true So this does not work with Dash. Please use IMAGE_FSTYPE = "vmdk" for easy reading of the code. -- Otavio Salvador O.S. Systems http://www.ossystems.com.brhttp://code.ossystems.com.br Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750 ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
On Wed, 2014-01-22 at 22:50 -0800, Saul Wold wrote:
> +IS_VMDK = '${@base_contains("IMAGE_FSTYPES", "vmdk", "true", "false", d)}'
> +
> + if [ "${IS_VMDK}" ]; then
Does that really work? I can't quite see how it would, but I might be
overlooking something. Presumably you did test this, right?
p.
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
On Friday 20 December 2013 12:32:38 Martin Jansa wrote:
> On Fri, Dec 20, 2013 at 08:10:24AM -0200, Joao Henrique Ferreira de Freitas
> wrote:
> > Previous change (086ce22b88f5ef5f75a83119a32c8b3fdcfa296d) broke
> > the creating of vmdk images. This protects shell expansion variables
> > and let dd generate the image to be transformed to vmdk by
> > image-vmdk.class. ---
> >
> > meta/classes/boot-directdisk.bbclass | 16 ++--
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/meta/classes/boot-directdisk.bbclass
> > b/meta/classes/boot-directdisk.bbclass index 3277666..4f26f47 100644
> > --- a/meta/classes/boot-directdisk.bbclass
> > +++ b/meta/classes/boot-directdisk.bbclass
> > @@ -88,10 +88,10 @@ build_boot_dd() {
> >
> > grubefi_hddimg_populate $HDDDIR
> >
> > fi
> >
> > - if [ ${IMAGE_FSTYPE} = "vmdk" ]; then
> > - if [ x${AUTO_SYSLINUXMENU} = x1 ] ; then
> > + if [ "${IMAGE_FSTYPES}" = "vmdk" ]; then
> > + if [ "x${AUTO_SYSLINUXMENU}" = "x1" ] ; then
>
> IMAGE_FSTYPE looks really as typo, but what if there are multiple
> entries in IMAGE_FSTYPES?
>
> OE @ ~/openembedded-core $ git grep IMAGE_FSTYPE | grep -v IMAGE_FSTYPES
> meta/classes/boot-directdisk.bbclass: if [ ${IMAGE_FSTYPE} = "vmdk" ];
> then meta/classes/boot-directdisk.bbclass: if [ ${IMAGE_FSTYPE} != "vmdk"
> ]; then meta/classes/boot-directdisk.bbclass: if [ ${IMAGE_FSTYPE} !=
> "vmdk" ]; then
>
> Maybe you can use this variable instead:
> meta/classes/image.bbclass:IMAGE_TYPE_vmdk =
> '${@base_contains("IMAGE_FSTYPES", "vmdk", "image-vmdk", "", d)}'
I'd suggest not using that variable as it's an internal implementation detail
of image.bbclass - just use a similar base_contains() statement.
Cheers,
Paul
--
Paul Eggleton
Intel Open Source Technology Centre
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] boot-directdisk: fix the support of vmdk
On Fri, Dec 20, 2013 at 08:10:24AM -0200, Joao Henrique Ferreira de Freitas
wrote:
> Previous change (086ce22b88f5ef5f75a83119a32c8b3fdcfa296d) broke
> the creating of vmdk images. This protects shell expansion variables
> and let dd generate the image to be transformed to vmdk by image-vmdk.class.
> ---
> meta/classes/boot-directdisk.bbclass | 16 ++--
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/meta/classes/boot-directdisk.bbclass
> b/meta/classes/boot-directdisk.bbclass
> index 3277666..4f26f47 100644
> --- a/meta/classes/boot-directdisk.bbclass
> +++ b/meta/classes/boot-directdisk.bbclass
> @@ -88,10 +88,10 @@ build_boot_dd() {
> grubefi_hddimg_populate $HDDDIR
> fi
>
> - if [ ${IMAGE_FSTYPE} = "vmdk" ]; then
> - if [ x${AUTO_SYSLINUXMENU} = x1 ] ; then
> + if [ "${IMAGE_FSTYPES}" = "vmdk" ]; then
> + if [ "x${AUTO_SYSLINUXMENU}" = "x1" ] ; then
IMAGE_FSTYPE looks really as typo, but what if there are multiple
entries in IMAGE_FSTYPES?
OE @ ~/openembedded-core $ git grep IMAGE_FSTYPE | grep -v IMAGE_FSTYPES
meta/classes/boot-directdisk.bbclass: if [ ${IMAGE_FSTYPE} = "vmdk" ]; then
meta/classes/boot-directdisk.bbclass: if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
meta/classes/boot-directdisk.bbclass: if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
Maybe you can use this variable instead:
meta/classes/image.bbclass:IMAGE_TYPE_vmdk = '${@base_contains("IMAGE_FSTYPES",
"vmdk", "image-vmdk", "", d)}'
> install -m 0644
> ${STAGING_DIR}/${MACHINE}/usr/share/syslinux/vesamenu.c32
> ${HDDDIR}${SYSLINUXDIR}/vesamenu.c32
> - if [ x${SYSLINUX_SPLASH} != x ] ; then
> + if [ "x${SYSLINUX_SPLASH}" != "x" ] ; then
> install -m 0644 ${SYSLINUX_SPLASH}
> ${HDDDIR}${SYSLINUXDIR}/splash.lss
> fi
> fi
> @@ -129,9 +129,7 @@ build_boot_dd() {
> parted $IMAGE unit B mkpart primary ext2 ${END2}B ${END3}B
> parted $IMAGE set 1 boot on
>
> - if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
> - parted $IMAGE print
> - fi
> + parted $IMAGE print
>
> awk "BEGIN { printf \"$(echo ${DISK_SIGNATURE} | fold -w 2 | tac |
> paste -sd '' | sed 's/\(..\)/\\x&/g')\" }" | \
> dd of=$IMAGE bs=1 seek=440 conv=notrunc
> @@ -141,10 +139,8 @@ build_boot_dd() {
> dd if=${STAGING_DATADIR}/syslinux/mbr.bin of=$IMAGE conv=notrunc
> fi
>
> - if [ ${IMAGE_FSTYPE} != "vmdk" ]; then
> - dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
> - dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
> - fi
> + 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}
> rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.hdddirect
> --
> 1.8.3.2
>
> ___
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
--
Martin 'JaMa' Jansa jabber: martin.ja...@gmail.com
signature.asc
Description: Digital signature
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
