Patchwork [v2] testimage: dont use DNS lookup for qemu based testimages

login
register
mail settings
Submitter Saul Wold
Date March 19, 2014, 12:31 a.m.
Message ID <1395189079-18831-1-git-send-email-sgw@linux.intel.com>
Download mbox | patch
Permalink /patch/68859/
State New
Headers show

Comments

Saul Wold - March 19, 2014, 12:31 a.m.
Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 meta/classes/testimage.bbclass | 10 ++++++++++
 1 file changed, 10 insertions(+)
Koen Kooi - March 19, 2014, 11:20 a.m.
Op 19 mrt. 2014, om 01:31 heeft Saul Wold <sgw@linux.intel.com> het volgende geschreven:

> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> ---
> meta/classes/testimage.bbclass | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/meta/classes/testimage.bbclass b/meta/classes/testimage.bbclass
> index 691c7f6..ee028e1 100644
> --- a/meta/classes/testimage.bbclass
> +++ b/meta/classes/testimage.bbclass
> @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall = "qemu-native:do_populate_sysroot qemu-helper-native:d
> TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
> TESTIMAGELOCK_qemuall = ""
> 
> +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"

Overrides and += don't do what you expect most of the time, _append_qemuall = " fix_dns_lookup_for_qemu" is a safer way to specify this. But more importantly, you're missing the trailing semicolon :) 

	IMAGE_POSTPROCESS_COMMAND_append_qemuall = " fix_dns_lookup_for_qemu ;"

regards,

Koen

> +
> +fix_dns_lookup_for_qemu() {
> +	if [ -e ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config ]; then
> +		sed -i -e 's:#UseDNS yes:UseDNS no:' ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config
> +	fi
> +}
> +
> +EXPORT_FUNCTIONS fix_dns_lookup_for_qemu
> +
> python do_testimage() {
>     testimage_main(d)
> }
> -- 
> 1.8.3.1
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
Stanacar, StefanX - March 19, 2014, 12:23 p.m.
On Tue, 2014-03-18 at 17:31 -0700, Saul Wold wrote:
> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> ---
>  meta/classes/testimage.bbclass | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/meta/classes/testimage.bbclass b/meta/classes/testimage.bbclass
> index 691c7f6..ee028e1 100644
> --- a/meta/classes/testimage.bbclass
> +++ b/meta/classes/testimage.bbclass
> @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall = "qemu-native:do_populate_sysroot qemu-helper-native:d
>  TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
>  TESTIMAGELOCK_qemuall = ""
>  
> +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
> +

I might be wrong, but this won't have any effect with the default
(manual) way we use testimage. The testimage class isn't in the global
inherit, so that  IMAGE_POSTPROCESS_COMMAND won't run as the image isn't
reconstructed.

The AB does: bitbake core-image-sato on one step and then adds INHERIT
+="testimage" in local.conf in the next step and runs bitbake
core-image-sato -c testimage, no more do_rootfs here.

As it this it will only work with testimage-auto when you have
TEST_IMAGE="1" in local.conf in the first build step so you only need to
do bitbake core-image-sato and do_testimage gets run after do_rootfs,
but we don't use this. And the fix should work for running the task
manually too.

I liked v1, I don't really see what the problem was there. No mattter
the way you run our qemu images, you still have to set a proper DNS, and
have network connectivity in the image (which usually involves the host
too) so defaulting to UseDNS no for qemu images seems a sensible default
to me.

Cheers,
Stefan

> +fix_dns_lookup_for_qemu() {
> +	if [ -e ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config ]; then
> +		sed -i -e 's:#UseDNS yes:UseDNS no:' ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config
> +	fi
> +}
> +
> +EXPORT_FUNCTIONS fix_dns_lookup_for_qemu
> +
>  python do_testimage() {
>      testimage_main(d)
>  }
> -- 
> 1.8.3.1
>
Paul Eggleton - March 19, 2014, 1:17 p.m.
On Wednesday 19 March 2014 12:23:37 Stanacar, StefanX wrote:
> On Tue, 2014-03-18 at 17:31 -0700, Saul Wold wrote:
> > Signed-off-by: Saul Wold <sgw@linux.intel.com>
> > ---
> > 
> >  meta/classes/testimage.bbclass | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/meta/classes/testimage.bbclass
> > b/meta/classes/testimage.bbclass index 691c7f6..ee028e1 100644
> > --- a/meta/classes/testimage.bbclass
> > +++ b/meta/classes/testimage.bbclass
> > @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall =
> > "qemu-native:do_populate_sysroot qemu-helper-native:d> 
> >  TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
> >  TESTIMAGELOCK_qemuall = ""
> > 
> > +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
> > +
> 
> I might be wrong, but this won't have any effect with the default
> (manual) way we use testimage. The testimage class isn't in the global
> inherit, so that  IMAGE_POSTPROCESS_COMMAND won't run as the image isn't
> reconstructed.
> 
> The AB does: bitbake core-image-sato on one step and then adds INHERIT
> +="testimage" in local.conf in the next step and runs bitbake
> core-image-sato -c testimage, no more do_rootfs here.
> 
> As it this it will only work with testimage-auto when you have
> TEST_IMAGE="1" in local.conf in the first build step so you only need to
> do bitbake core-image-sato and do_testimage gets run after do_rootfs,
> but we don't use this. And the fix should work for running the task
> manually too.

I have to say I agree; this is not the right fix.
 
> I liked v1, I don't really see what the problem was there. No mattter
> the way you run our qemu images, you still have to set a proper DNS, and
> have network connectivity in the image (which usually involves the host
> too) so defaulting to UseDNS no for qemu images seems a sensible default
> to me.

Well, v1 was unconditional in the openssh recipe; I think it still needs to be 
just for qemuall.

Cheers,
Paul
Saul Wold - March 19, 2014, 8:06 p.m.
On 03/19/2014 06:17 AM, Paul Eggleton wrote:
> On Wednesday 19 March 2014 12:23:37 Stanacar, StefanX wrote:
>> On Tue, 2014-03-18 at 17:31 -0700, Saul Wold wrote:
>>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
>>> ---
>>>
>>>   meta/classes/testimage.bbclass | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/meta/classes/testimage.bbclass
>>> b/meta/classes/testimage.bbclass index 691c7f6..ee028e1 100644
>>> --- a/meta/classes/testimage.bbclass
>>> +++ b/meta/classes/testimage.bbclass
>>> @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall =
>>> "qemu-native:do_populate_sysroot qemu-helper-native:d>
>>>   TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
>>>   TESTIMAGELOCK_qemuall = ""
>>>
>>> +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
>>> +
>>
>> I might be wrong, but this won't have any effect with the default
>> (manual) way we use testimage. The testimage class isn't in the global
>> inherit, so that  IMAGE_POSTPROCESS_COMMAND won't run as the image isn't
>> reconstructed.
>>
>> The AB does: bitbake core-image-sato on one step and then adds INHERIT
>> +="testimage" in local.conf in the next step and runs bitbake
>> core-image-sato -c testimage, no more do_rootfs here.
>>
>> As it this it will only work with testimage-auto when you have
>> TEST_IMAGE="1" in local.conf in the first build step so you only need to
>> do bitbake core-image-sato and do_testimage gets run after do_rootfs,
>> but we don't use this. And the fix should work for running the task
>> manually too.
>
> I have to say I agree; this is not the right fix.
>
Is there any reason that we can't include the INHERIT += "testimage" 
during the core build?

The other choice is to move the function into the openssh recipe and 
then the actual setting of the IMAGE_POSTPROCESS_COMMAND can happen in 
the local.conf or auto.conf


>> I liked v1, I don't really see what the problem was there. No mattter
>> the way you run our qemu images, you still have to set a proper DNS, and
>> have network connectivity in the image (which usually involves the host
>> too) so defaulting to UseDNS no for qemu images seems a sensible default
>> to me.
>
> Well, v1 was unconditional in the openssh recipe; I think it still needs to be
> just for qemuall.
>
It was originally for qemuall, but was also for all images and was to 
broad as some may not want it disabled in this way.

Further thoughts?

Sau!
> Cheers,
> Paul
>
Elizabeth Flanagan - March 19, 2014, 9:43 p.m.
We can add the INHERIT either in the nightly*.conf as an atextappend
or in CreateAutoConf. So doing it via the ab isn't difficult at all.
Let me know what you guys think and I'll pull together a patch for it.

-b

On Wed, Mar 19, 2014 at 1:06 PM, Saul Wold <sgw@linux.intel.com> wrote:
> On 03/19/2014 06:17 AM, Paul Eggleton wrote:
>>
>> On Wednesday 19 March 2014 12:23:37 Stanacar, StefanX wrote:
>>>
>>> On Tue, 2014-03-18 at 17:31 -0700, Saul Wold wrote:
>>>>
>>>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
>>>> ---
>>>>
>>>>   meta/classes/testimage.bbclass | 10 ++++++++++
>>>>   1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/meta/classes/testimage.bbclass
>>>> b/meta/classes/testimage.bbclass index 691c7f6..ee028e1 100644
>>>> --- a/meta/classes/testimage.bbclass
>>>> +++ b/meta/classes/testimage.bbclass
>>>> @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall =
>>>> "qemu-native:do_populate_sysroot qemu-helper-native:d>
>>>>   TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
>>>>   TESTIMAGELOCK_qemuall = ""
>>>>
>>>> +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
>>>> +
>>>
>>>
>>> I might be wrong, but this won't have any effect with the default
>>> (manual) way we use testimage. The testimage class isn't in the global
>>> inherit, so that  IMAGE_POSTPROCESS_COMMAND won't run as the image isn't
>>> reconstructed.
>>>
>>> The AB does: bitbake core-image-sato on one step and then adds INHERIT
>>> +="testimage" in local.conf in the next step and runs bitbake
>>> core-image-sato -c testimage, no more do_rootfs here.
>>>
>>> As it this it will only work with testimage-auto when you have
>>> TEST_IMAGE="1" in local.conf in the first build step so you only need to
>>> do bitbake core-image-sato and do_testimage gets run after do_rootfs,
>>> but we don't use this. And the fix should work for running the task
>>> manually too.
>>
>>
>> I have to say I agree; this is not the right fix.
>>
> Is there any reason that we can't include the INHERIT += "testimage" during
> the core build?
>
> The other choice is to move the function into the openssh recipe and then
> the actual setting of the IMAGE_POSTPROCESS_COMMAND can happen in the
> local.conf or auto.conf
>
>
>
>>> I liked v1, I don't really see what the problem was there. No mattter
>>> the way you run our qemu images, you still have to set a proper DNS, and
>>> have network connectivity in the image (which usually involves the host
>>> too) so defaulting to UseDNS no for qemu images seems a sensible default
>>> to me.
>>
>>
>> Well, v1 was unconditional in the openssh recipe; I think it still needs
>> to be
>> just for qemuall.
>>
> It was originally for qemuall, but was also for all images and was to broad
> as some may not want it disabled in this way.
>
> Further thoughts?
>
> Sau!
>>
>> Cheers,
>> Paul
>>
>
Richard Purdie - March 19, 2014, 9:44 p.m.
On Wed, 2014-03-19 at 12:23 +0000, Stanacar, StefanX wrote:
> 
> 
> On Tue, 2014-03-18 at 17:31 -0700, Saul Wold wrote:
> > Signed-off-by: Saul Wold <sgw@linux.intel.com>
> > ---
> >  meta/classes/testimage.bbclass | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/meta/classes/testimage.bbclass b/meta/classes/testimage.bbclass
> > index 691c7f6..ee028e1 100644
> > --- a/meta/classes/testimage.bbclass
> > +++ b/meta/classes/testimage.bbclass
> > @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall = "qemu-native:do_populate_sysroot qemu-helper-native:d
> >  TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
> >  TESTIMAGELOCK_qemuall = ""
> >  
> > +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
> > +
> 
> I might be wrong, but this won't have any effect with the default
> (manual) way we use testimage. The testimage class isn't in the global
> inherit, so that  IMAGE_POSTPROCESS_COMMAND won't run as the image isn't
> reconstructed.

Surely it will have run when the rootfs was originally constructed
though?

Koen is right, that override is incorrect.

Cheers,

Richard
Paul Eggleton - March 19, 2014, 10:57 p.m.
On Wednesday 19 March 2014 13:06:51 Saul Wold wrote:
> On 03/19/2014 06:17 AM, Paul Eggleton wrote:
> > On Wednesday 19 March 2014 12:23:37 Stanacar, StefanX wrote:
> >> On Tue, 2014-03-18 at 17:31 -0700, Saul Wold wrote:
> >>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> >>> ---
> >>> 
> >>>   meta/classes/testimage.bbclass | 10 ++++++++++
> >>>   1 file changed, 10 insertions(+)
> >>> 
> >>> diff --git a/meta/classes/testimage.bbclass
> >>> b/meta/classes/testimage.bbclass index 691c7f6..ee028e1 100644
> >>> --- a/meta/classes/testimage.bbclass
> >>> +++ b/meta/classes/testimage.bbclass
> >>> @@ -45,6 +45,16 @@ TESTIMAGEDEPENDS_qemuall =
> >>> "qemu-native:do_populate_sysroot qemu-helper-native:d>
> >>> 
> >>>   TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
> >>>   TESTIMAGELOCK_qemuall = ""
> >>> 
> >>> +IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
> >>> +
> >> 
> >> I might be wrong, but this won't have any effect with the default
> >> (manual) way we use testimage. The testimage class isn't in the global
> >> inherit, so that  IMAGE_POSTPROCESS_COMMAND won't run as the image isn't
> >> reconstructed.
> >> 
> >> The AB does: bitbake core-image-sato on one step and then adds INHERIT
> >> +="testimage" in local.conf in the next step and runs bitbake
> >> core-image-sato -c testimage, no more do_rootfs here.
> >> 
> >> As it this it will only work with testimage-auto when you have
> >> TEST_IMAGE="1" in local.conf in the first build step so you only need to
> >> do bitbake core-image-sato and do_testimage gets run after do_rootfs,
> >> but we don't use this. And the fix should work for running the task
> >> manually too.
> > 
> > I have to say I agree; this is not the right fix.
> 
> Is there any reason that we can't include the INHERIT += "testimage"
> during the core build?

I don't want testimage.bbclass influencing how images are constructed. It's 
just as valid a workaround for people running qemu manually on their machines 
as it is for the automated testing.

> The other choice is to move the function into the openssh recipe and
> then the actual setting of the IMAGE_POSTPROCESS_COMMAND can happen in
> the local.conf or auto.conf

That can't work. The function has to be defined in the scope of the image not 
the openssh recipe or it can't be called from IMAGE_POSTPROCESS_COMMAND.

To be honest we already have code in image.bbclass for allowing empty 
passwords in OpenSSH / dropbear; this seems to me to be in the same category, 
so I would suggest putting at least the function there (with a more generic 
name that does not contain "qemu").

> >> I liked v1, I don't really see what the problem was there. No mattter
> >> the way you run our qemu images, you still have to set a proper DNS, and
> >> have network connectivity in the image (which usually involves the host
> >> too) so defaulting to UseDNS no for qemu images seems a sensible default
> >> to me.
> > 
> > Well, v1 was unconditional in the openssh recipe; I think it still needs
> > to be just for qemuall.
> 
> It was originally for qemuall, but was also for all images and was to
> broad as some may not want it disabled in this way.

OK, I missed that; but v1 still made openssh machine-specific which is not 
desirable.

Cheers,
Paul

Patch

diff --git a/meta/classes/testimage.bbclass b/meta/classes/testimage.bbclass
index 691c7f6..ee028e1 100644
--- a/meta/classes/testimage.bbclass
+++ b/meta/classes/testimage.bbclass
@@ -45,6 +45,16 @@  TESTIMAGEDEPENDS_qemuall = "qemu-native:do_populate_sysroot qemu-helper-native:d
 TESTIMAGELOCK = "${TMPDIR}/testimage.lock"
 TESTIMAGELOCK_qemuall = ""
 
+IMAGE_POSTPROCESS_COMMAND_qemuall += "fix_dns_lookup_for_qemu"
+
+fix_dns_lookup_for_qemu() {
+	if [ -e ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config ]; then
+		sed -i -e 's:#UseDNS yes:UseDNS no:' ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config
+	fi
+}
+
+EXPORT_FUNCTIONS fix_dns_lookup_for_qemu
+
 python do_testimage() {
     testimage_main(d)
 }