public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
@ 2017-09-17 19:32 Heinrich Schuchardt
  2017-09-18 17:33 ` Stephen Warren
  2017-09-18 18:27 ` Stephen Warren
  0 siblings, 2 replies; 11+ messages in thread
From: Heinrich Schuchardt @ 2017-09-17 19:32 UTC (permalink / raw)
  To: u-boot

The necessary parameters for running Python tests on qemu are
tediouus to find.

The patch adds examples for u-boot-test-console and
u-boot-test-reset.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 test/py/README.md | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/py/README.md b/test/py/README.md
index 829c7efbb2..f3ad10df5a 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -197,6 +197,23 @@ simulator includes a virtual reset button! If not, you can launch the
 simulator from `u-boot-test-reset` instead, while arranging for this console
 process to always communicate with the current simulator instance.
 
+With qemu you can use the parameter -monitor to connect the control console to a
+Unix socket, e.g.
+
+    #!/bin/sh
+    touch /tmp/u-boot-monitor-socket
+    qemu-system-x86_64 -bios build-qemu-x86/u-boot.rom -nographic -netdev \
+    user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
+    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
+    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
+
+In `u-boot-test-reset` call the socat command to send a system reset:
+
+    #!/bin/sh
+    echo system_reset | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
+    sleep 1
+    true
+
 #### `u-boot-test-flash`
 
 Prior to running the test suite against a board, some arrangement must be made
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-17 19:32 [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu Heinrich Schuchardt
@ 2017-09-18 17:33 ` Stephen Warren
  2017-09-18 17:48   ` Tom Rini
  2017-09-18 18:13   ` Heinrich Schuchardt
  2017-09-18 18:27 ` Stephen Warren
  1 sibling, 2 replies; 11+ messages in thread
From: Stephen Warren @ 2017-09-18 17:33 UTC (permalink / raw)
  To: u-boot

On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
> The necessary parameters for running Python tests on qemu are
> tediouus to find.
> 
> The patch adds examples for u-boot-test-console and
> u-boot-test-reset.

This README doesn't contain examples for other cases. I'm not sure 
whether we should add them for qemu either. Instead, does it make sense 
to augment my repo, which already contains a variety of examples, and is 
mentioned in the README?

https://github.com/swarren/uboot-test-hooks

Tom Rini already added some qemu examples to that repo, although they 
use much simpler command-lines than in the examples in this patch. 
Perhaps that's because they fire off a new qemu instance every time the 
virtual system is reset, whereas yours seem to create one qemu instance 
and then reset it multiple times. If so, your scripts are more how I 
imagined it would work; perhaps you could update Tom's scripts?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 17:33 ` Stephen Warren
@ 2017-09-18 17:48   ` Tom Rini
  2017-09-18 18:13   ` Heinrich Schuchardt
  1 sibling, 0 replies; 11+ messages in thread
From: Tom Rini @ 2017-09-18 17:48 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 18, 2017 at 11:33:01AM -0600, Stephen Warren wrote:
> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
> >The necessary parameters for running Python tests on qemu are
> >tediouus to find.
> >
> >The patch adds examples for u-boot-test-console and
> >u-boot-test-reset.
> 
> This README doesn't contain examples for other cases. I'm not sure
> whether we should add them for qemu either. Instead, does it make
> sense to augment my repo, which already contains a variety of
> examples, and is mentioned in the README?
> 
> https://github.com/swarren/uboot-test-hooks
> 
> Tom Rini already added some qemu examples to that repo, although
> they use much simpler command-lines than in the examples in this
> patch. Perhaps that's because they fire off a new qemu instance
> every time the virtual system is reset, whereas yours seem to create
> one qemu instance and then reset it multiple times. If so, your
> scripts are more how I imagined it would work; perhaps you could
> update Tom's scripts?

I was thinking about this too.  Heinrich's example is along the lines of
"adapt QEMU to provide what test.py wants" while the uboot-test-hooks
QEMU parts are more along the lines of "plumb QEMU into the frameworks
for real HW too".  What I like about the latter is that I leverage
information from my real HW too (tftp stuff, etc).  Why I think we
should also apply Heinrich's patch, or maybe a v2 with a few wording
tweaks, is that it's less overhead and easier for people to setup if
they just want QEMU available for testing.  Say testing while on the
road at some conference.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170918/206dbad9/attachment.sig>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 17:33 ` Stephen Warren
  2017-09-18 17:48   ` Tom Rini
@ 2017-09-18 18:13   ` Heinrich Schuchardt
  1 sibling, 0 replies; 11+ messages in thread
From: Heinrich Schuchardt @ 2017-09-18 18:13 UTC (permalink / raw)
  To: u-boot

On 09/18/2017 07:33 PM, Stephen Warren wrote:
> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>> The necessary parameters for running Python tests on qemu are
>> tediouus to find.
>>
>> The patch adds examples for u-boot-test-console and
>> u-boot-test-reset.
> 
> This README doesn't contain examples for other cases. I'm not sure
> whether we should add them for qemu either. Instead, does it make sense
> to augment my repo, which already contains a variety of examples, and is
> mentioned in the README?
> 
> https://github.com/swarren/uboot-test-hooks
> 
> Tom Rini already added some qemu examples to that repo, although they
> use much simpler command-lines than in the examples in this patch.
> Perhaps that's because they fire off a new qemu instance every time the
> virtual system is reset, whereas yours seem to create one qemu instance
> and then reset it multiple times. If so, your scripts are more how I
> imagined it would work; perhaps you could update Tom's scripts?
> 

I very much appreciate that your repo makes the test automation on
Travis CI possible. But when looking at your repo I could not find any
file that helped me to get qemu running U-Boot on my system.

When I read a documentation I do not want to be handed over to a repo
where I get lost and which adds another level of external reference:
"The example scripts depend on various external tools, the installation
location of which must be specified in the board configuration files:
As-yet-unpublished scripts to control various USB relay boards."

Sure you are free to use my examples to update your repo to provide
something more accessible. But this does not obsolete this patch.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-17 19:32 [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu Heinrich Schuchardt
  2017-09-18 17:33 ` Stephen Warren
@ 2017-09-18 18:27 ` Stephen Warren
  2017-09-18 19:55   ` Heinrich Schuchardt
  1 sibling, 1 reply; 11+ messages in thread
From: Stephen Warren @ 2017-09-18 18:27 UTC (permalink / raw)
  To: u-boot

On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
> The necessary parameters for running Python tests on qemu are
> tediouus to find.

Nit: tedious

Let's wrap the commit description to 72-74 characters; it's rather 
narrow right now.

> 
> The patch adds examples for u-boot-test-console and
> u-boot-test-reset.

> diff --git a/test/py/README.md b/test/py/README.md
> index 829c7efbb2..f3ad10df5a 100644
> --- a/test/py/README.md
> +++ b/test/py/README.md
> @@ -197,6 +197,23 @@ simulator includes a virtual reset button! If not, you can launch the
>   simulator from `u-boot-test-reset` instead, while arranging for this console
>   process to always communicate with the current simulator instance.

Rather that adding these examples into a section that details one of the 
individual hook scripts, let's create a new section "Simple qemu 
example" and put it right before or after the existing "Examples" section.

> +With qemu you can use the parameter -monitor to connect the control console to a
> +Unix socket, e.g.

Let's state what filename the following example should be saved to; 
u-boot-test-console.

> +    #!/bin/sh
> +    touch /tmp/u-boot-monitor-socket
> +    qemu-system-x86_64 -bios build-qemu-x86/u-boot.rom -nographic -netdev \
> +    user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \

Let's indent the continuation lines so it's more obvious this is a 
multi-line command:

asdfsd fsjkl fsfj lssfjdasdfsjl \
	asdfsd fsjkl fsfj lssfjdasdfsjl \
	asdfsd fsjkl fsfj lssfjdasdfsjl \

I think this (and the other) script should "exec" the commands to avoid 
leaving the shell instance around.

This example seems to enable networking support in qemu, and a TFTP 
server. I believe you'll need to provide an example Python board 
configuration so that test/py knows to enable the network tests.

> +    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
> +    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
> +
> +In `u-boot-test-reset` call the socat command to send a system reset:
> +
> +    #!/bin/sh
> +    echo system_reset | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
> +    sleep 1
> +    true

Why is the sleep needed? The true command shouldn't have any effect 
given set -e isn't in use.

I only see examples for u-boot-test-console and u-boot-test-reset. I 
believe you need to provide a dummy/empty u-boot-test-flash too.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 18:27 ` Stephen Warren
@ 2017-09-18 19:55   ` Heinrich Schuchardt
  2017-09-18 21:28     ` Stephen Warren
  0 siblings, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2017-09-18 19:55 UTC (permalink / raw)
  To: u-boot

On 09/18/2017 08:27 PM, Stephen Warren wrote:
> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>> The necessary parameters for running Python tests on qemu are
>> tediouus to find.
> 
> Nit: tedious
> 
> Let's wrap the commit description to 72-74 characters; it's rather
> narrow right now.
> 
>>
>> The patch adds examples for u-boot-test-console and
>> u-boot-test-reset.
> 
>> diff --git a/test/py/README.md b/test/py/README.md
>> index 829c7efbb2..f3ad10df5a 100644
>> --- a/test/py/README.md
>> +++ b/test/py/README.md
>> @@ -197,6 +197,23 @@ simulator includes a virtual reset button! If
>> not, you can launch the
>>   simulator from `u-boot-test-reset` instead, while arranging for this
>> console
>>   process to always communicate with the current simulator instance.
> 
> Rather that adding these examples into a section that details one of the
> individual hook scripts, let's create a new section "Simple qemu
> example" and put it right before or after the existing "Examples" section.
Ok

> 
>> +With qemu you can use the parameter -monitor to connect the control
>> console to a
>> +Unix socket, e.g.
> 
> Let's state what filename the following example should be saved to;
> u-boot-test-console.

sure

> 
>> +    #!/bin/sh
>> +    touch /tmp/u-boot-monitor-socket
>> +    qemu-system-x86_64 -bios build-qemu-x86/u-boot.rom -nographic
>> -netdev \
>> +   
>> user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
> 
> Let's indent the continuation lines so it's more obvious this is a
> multi-line command:
> 
> asdfsd fsjkl fsfj lssfjdasdfsjl \
>     asdfsd fsjkl fsfj lssfjdasdfsjl \
>     asdfsd fsjkl fsfj lssfjdasdfsjl \
> 
> I think this (and the other) script should "exec" the commands to avoid
> leaving the shell instance around.

Is this really needed? It just adds complexity.
You cannot execute anything in the lines after exec,
e.g. deleting the socket file.

> 
> This example seems to enable networking support in qemu, and a TFTP
> server. I believe you'll need to provide an example Python board
> configuration so that test/py knows to enable the network tests.

tftp is used for testing bootefi hello

Empty file
__init__.py

u_boot_boardenv_qemu_x86.py
env__net_dhcp_server = True
env__efi_loader_helloworld_file = {
        "fn": "helloworld.efi",
        "size": 4298,
        "crc32": "55d96ef8",
}

This is another file needed:

u-boot-test-quit
#!/bin/sh
echo quit | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket

The following script comes in handy to create the .py file:

#!/bin/bash
echo env__efi_loader_$(basename $1 | sed 's/\./_/g') = \{
echo '    "fn":' $(basename $1)
echo '    "len":' $(stat --printf="%s" $1)
echo '    "crc32":' $(crc32 $1)
echo \}

> 
>> +    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
>> +    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
>> +
>> +In `u-boot-test-reset` call the socat command to send a system reset:
>> +
>> +    #!/bin/sh
>> +    echo system_reset | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
>> +    sleep 1
>> +    true
> 
> Why is the sleep needed?

This avoids race conditions.
Qemu will need some milliseconds to actually shut down qemu.
I want to be sure that Python does not execute any command before this
is completed.

> The true command shouldn't have any effect
> given set -e isn't in use.

man dash:
The shell will return the exit status of the last command executed.

If the last command is false running the test suite fails.

> 
> I only see examples for u-boot-test-console and u-boot-test-reset. I
> believe you need to provide a dummy/empty u-boot-test-flash too.
> 
Yes this is what I used:

#!/bin/sh
echo ... u-boot-test-flash ...

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 19:55   ` Heinrich Schuchardt
@ 2017-09-18 21:28     ` Stephen Warren
  2017-09-18 21:37       ` Heinrich Schuchardt
  2017-09-19 20:15       ` Heinrich Schuchardt
  0 siblings, 2 replies; 11+ messages in thread
From: Stephen Warren @ 2017-09-18 21:28 UTC (permalink / raw)
  To: u-boot

On 09/18/2017 01:55 PM, Heinrich Schuchardt wrote:
> On 09/18/2017 08:27 PM, Stephen Warren wrote:
>> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>>> The necessary parameters for running Python tests on qemu are
>>> tediouus to find.
>>
>> Nit: tedious
>>
>> Let's wrap the commit description to 72-74 characters; it's rather
>> narrow right now.
>>
>>>
>>> The patch adds examples for u-boot-test-console and
>>> u-boot-test-reset.
>>
>>> diff --git a/test/py/README.md b/test/py/README.md
>>> index 829c7efbb2..f3ad10df5a 100644
>>> --- a/test/py/README.md
>>> +++ b/test/py/README.md
>>> @@ -197,6 +197,23 @@ simulator includes a virtual reset button! If
>>> not, you can launch the
>>>    simulator from `u-boot-test-reset` instead, while arranging for this console
>>>    process to always communicate with the current simulator instance.

>>> +    #!/bin/sh
>>> +    touch /tmp/u-boot-monitor-socket
>>> +    qemu-system-x86_64 -bios build-qemu-x86/u-boot.rom -nographic
>>> -netdev \
>>> +
>>> user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \

>> I think this (and the other) script should "exec" the commands to avoid
>> leaving the shell instance around.
> 
> Is this really needed? It just adds complexity.
> You cannot execute anything in the lines after exec,
> e.g. deleting the socket file.

Not using exec won't break functionality. However, there's no point 
leaving the shell process hanging about, and since you're editing the 
change anyway it's trivial to fix this up. At present, there's no need 
to run anything after qemu. If we ever have need to, we can remove the 
exec at that time.

>> This example seems to enable networking support in qemu, and a TFTP
>> server. I believe you'll need to provide an example Python board
>> configuration so that test/py knows to enable the network tests.
> 
> tftp is used for testing bootefi hello
> 
> Empty file
> __init__.py

__init__.py isn't the correct filename. You would need to implement 
u_boot_boardenv_sandbox_na.py I believe. Also, the file can't be empty; 
it needs specific content to enable the TFTP test. See the comments in 
e.g. test/py/tests/test_net.py or test/py/tests/test_efi_loader.py. If 
you don't want to complicate the simple example with this, then I'd 
suggest simplifying the qemu command-line to remove all the 
network/TFTP-related options.

> u_boot_boardenv_qemu_x86.py
> env__net_dhcp_server = True
> env__efi_loader_helloworld_file = {
>          "fn": "helloworld.efi",
>          "size": 4298,
>          "crc32": "55d96ef8",
> }
> 
> This is another file needed:
> 
> u-boot-test-quit
> #!/bin/sh
> echo quit | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket

The test/py framework doesn't execute "u-boot-test-quit. Adding such a 
file won't affect anything.

> The following script comes in handy to create the .py file:
> 
> #!/bin/bash
> echo env__efi_loader_$(basename $1 | sed 's/\./_/g') = \{
> echo '    "fn":' $(basename $1)
> echo '    "len":' $(stat --printf="%s" $1)
> echo '    "crc32":' $(crc32 $1)
> echo \}

Let's just specify the content of the Python file (which the user can 
simply cut/past) rather than making life complicated by writing a shell 
script to create the Python file.

>>> +    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
>>> +    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
>>> +
>>> +In `u-boot-test-reset` call the socat command to send a system reset:
>>> +
>>> +    #!/bin/sh
>>> +    echo system_reset | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
>>> +    sleep 1
>>> +    true
>>
>> Why is the sleep needed?
> 
> This avoids race conditions.
> Qemu will need some milliseconds to actually shut down qemu.
> I want to be sure that Python does not execute any command before this
> is completed.

I don't believe there's any issue here. test/py will wait for qemu to 
boot U-Boot before attempting to send any commands after the reset 
occurs, and that wait operation can start as soon as the reset trigger 
is sent. Did you observe any issue in practice?

>> The true command shouldn't have any effect
>> given set -e isn't in use.
> 
> man dash:
> The shell will return the exit status of the last command executed.
> 
> If the last command is false running the test suite fails.

OK. Why would either the echo or sleep fail? If they do, then that 
failure should be passed back to test/py so that it can record the 
problem. Errors shouldn't just be ignored.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 21:28     ` Stephen Warren
@ 2017-09-18 21:37       ` Heinrich Schuchardt
  2017-09-19 16:18         ` Stephen Warren
  2017-09-19 20:15       ` Heinrich Schuchardt
  1 sibling, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2017-09-18 21:37 UTC (permalink / raw)
  To: u-boot

On 09/18/2017 11:28 PM, Stephen Warren wrote:
> On 09/18/2017 01:55 PM, Heinrich Schuchardt wrote:
>> On 09/18/2017 08:27 PM, Stephen Warren wrote:
>>> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>>>> The necessary parameters for running Python tests on qemu are
>>>> tediouus to find.
>>>
>>> Nit: tedious
>>>
>>> Let's wrap the commit description to 72-74 characters; it's rather
>>> narrow right now.
>>>
>>>>
>>>> The patch adds examples for u-boot-test-console and
>>>> u-boot-test-reset.
>>>
>>>> diff --git a/test/py/README.md b/test/py/README.md
>>>> index 829c7efbb2..f3ad10df5a 100644
>>>> --- a/test/py/README.md
>>>> +++ b/test/py/README.md
>>>> @@ -197,6 +197,23 @@ simulator includes a virtual reset button! If
>>>> not, you can launch the
>>>>    simulator from `u-boot-test-reset` instead, while arranging for
>>>> this console
>>>>    process to always communicate with the current simulator instance.
> 
>>>> +    #!/bin/sh
>>>> +    touch /tmp/u-boot-monitor-socket
>>>> +    qemu-system-x86_64 -bios build-qemu-x86/u-boot.rom -nographic
>>>> -netdev \
>>>> +
>>>> user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
> 
>>> I think this (and the other) script should "exec" the commands to avoid
>>> leaving the shell instance around.
>>
>> Is this really needed? It just adds complexity.
>> You cannot execute anything in the lines after exec,
>> e.g. deleting the socket file.
> 
> Not using exec won't break functionality. However, there's no point
> leaving the shell process hanging about, and since you're editing the
> change anyway it's trivial to fix this up. At present, there's no need
> to run anything after qemu. If we ever have need to, we can remove the
> exec at that time.
> 
>>> This example seems to enable networking support in qemu, and a TFTP
>>> server. I believe you'll need to provide an example Python board
>>> configuration so that test/py knows to enable the network tests.
>>
>> tftp is used for testing bootefi hello
>>
>> Empty file
>> __init__.py
> 
> __init__.py isn't the correct filename.

We need 2 *.py files.

__init__.py to make the directory a package directory and the board
file. My example is not for the sandbox but for qemu-x86_defconfig.

> You would need to implement
> u_boot_boardenv_sandbox_na.py I believe. Also, the file can't be empty;
> it needs specific content to enable the TFTP test. See the comments in
> e.g. test/py/tests/test_net.py or test/py/tests/test_efi_loader.py. If
> you don't want to complicate the simple example with this, then I'd
> suggest simplifying the qemu command-line to remove all the
> network/TFTP-related options.
> 
>> u_boot_boardenv_qemu_x86.py
>> env__net_dhcp_server = True
>> env__efi_loader_helloworld_file = {
>>          "fn": "helloworld.efi",
>>          "size": 4298,
>>          "crc32": "55d96ef8",
>> }
>>
>> This is another file needed:
>>
>> u-boot-test-quit
>> #!/bin/sh
>> echo quit | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
> 
> The test/py framework doesn't execute "u-boot-test-quit. Adding such a
> file won't affect anything.
> 
>> The following script comes in handy to create the .py file:
>>
>> #!/bin/bash
>> echo env__efi_loader_$(basename $1 | sed 's/\./_/g') = \{
>> echo '    "fn":' $(basename $1)
>> echo '    "len":' $(stat --printf="%s" $1)
>> echo '    "crc32":' $(crc32 $1)
>> echo \}
> 
> Let's just specify the content of the Python file (which the user can
> simply cut/past) rather than making life complicated by writing a shell
> script to create the Python file.
> 
>>>> +    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
>>>> +    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
>>>> +
>>>> +In `u-boot-test-reset` call the socat command to send a system reset:
>>>> +
>>>> +    #!/bin/sh
>>>> +    echo system_reset | socat -
>>>> UNIX-CONNECT:/tmp/u-boot-monitor-socket
>>>> +    sleep 1
>>>> +    true
>>>
>>> Why is the sleep needed?
>>
>> This avoids race conditions.
>> Qemu will need some milliseconds to actually shut down qemu.
>> I want to be sure that Python does not execute any command before this
>> is completed.
> 
> I don't believe there's any issue here. test/py will wait for qemu to
> boot U-Boot before attempting to send any commands after the reset
> occurs, and that wait operation can start as soon as the reset trigger
> is sent. Did you observe any issue in practice?
> 
>>> The true command shouldn't have any effect
>>> given set -e isn't in use.
>>
>> man dash:
>> The shell will return the exit status of the last command executed.
>>
>> If the last command is false running the test suite fails.
> 
> OK. Why would either the echo or sleep fail? If they do, then that
> failure should be passed back to test/py so that it can record the
> problem. Errors shouldn't just be ignored.
> 

I saw your comments late. So I could not integrated them into v2.

Will do so tomorrow.

Regards

Heinrich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 21:37       ` Heinrich Schuchardt
@ 2017-09-19 16:18         ` Stephen Warren
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Warren @ 2017-09-19 16:18 UTC (permalink / raw)
  To: u-boot

On 09/18/2017 03:37 PM, Heinrich Schuchardt wrote:
> On 09/18/2017 11:28 PM, Stephen Warren wrote:
>> On 09/18/2017 01:55 PM, Heinrich Schuchardt wrote:
>>> On 09/18/2017 08:27 PM, Stephen Warren wrote:
>>>> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>>>>> The necessary parameters for running Python tests on qemu are
>>>>> tediouus to find.

>>> Empty file
>>> __init__.py
>>
>> __init__.py isn't the correct filename.
> 
> We need 2 *.py files.
> 
> __init__.py to make the directory a package directory and the board
> file. My example is not for the sandbox but for qemu-x86_defconfig.

You don't need __init__.py. The directory doesn't need to be a Python 
package. You just need u_boot_boardenv_xxx.py in a directory, and to add 
that directory into $PYTHONPATH.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-18 21:28     ` Stephen Warren
  2017-09-18 21:37       ` Heinrich Schuchardt
@ 2017-09-19 20:15       ` Heinrich Schuchardt
  2017-09-20 15:52         ` Stephen Warren
  1 sibling, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2017-09-19 20:15 UTC (permalink / raw)
  To: u-boot

On 09/18/2017 11:28 PM, Stephen Warren wrote:
> On 09/18/2017 01:55 PM, Heinrich Schuchardt wrote:
>> On 09/18/2017 08:27 PM, Stephen Warren wrote:
>>> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>>>> The necessary parameters for running Python tests on qemu are
>>>> tediouus to find.
>>>
>>> Nit: tedious
>>>
>>> Let's wrap the commit description to 72-74 characters; it's rather
>>> narrow right now.
>>>
>>>>
>>>> The patch adds examples for u-boot-test-console and
>>>> u-boot-test-reset.
>>>
>>>> diff --git a/test/py/README.md b/test/py/README.md
>>>> index 829c7efbb2..f3ad10df5a 100644
>>>> --- a/test/py/README.md
>>>> +++ b/test/py/README.md
>>>> @@ -197,6 +197,23 @@ simulator includes a virtual reset button! If
>>>> not, you can launch the
>>>>    simulator from `u-boot-test-reset` instead, while arranging for
>>>> this console
>>>>    process to always communicate with the current simulator instance.
> 
>>>> +    #!/bin/sh
>>>> +    touch /tmp/u-boot-monitor-socket
>>>> +    qemu-system-x86_64 -bios build-qemu-x86/u-boot.rom -nographic
>>>> -netdev \
>>>> +
>>>> user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
> 
>>> I think this (and the other) script should "exec" the commands to avoid
>>> leaving the shell instance around.
>>
>> Is this really needed? It just adds complexity.
>> You cannot execute anything in the lines after exec,
>> e.g. deleting the socket file.
> 
> Not using exec won't break functionality. However, there's no point
> leaving the shell process hanging about, and since you're editing the
> change anyway it's trivial to fix this up. At present, there's no need
> to run anything after qemu. If we ever have need to, we can remove the
> exec at that time.
> 
>>> This example seems to enable networking support in qemu, and a TFTP
>>> server. I believe you'll need to provide an example Python board
>>> configuration so that test/py knows to enable the network tests.
>>
>> tftp is used for testing bootefi hello
>>
>> Empty file
>> __init__.py
> 
> __init__.py isn't the correct filename. You would need to implement
> u_boot_boardenv_sandbox_na.py I believe. Also, the file can't be empty;
> it needs specific content to enable the TFTP test. See the comments in
> e.g. test/py/tests/test_net.py or test/py/tests/test_efi_loader.py. If
> you don't want to complicate the simple example with this, then I'd
> suggest simplifying the qemu command-line to remove all the
> network/TFTP-related options.
> 
>> u_boot_boardenv_qemu_x86.py
>> env__net_dhcp_server = True
>> env__efi_loader_helloworld_file = {
>>          "fn": "helloworld.efi",
>>          "size": 4298,
>>          "crc32": "55d96ef8",
>> }
>>
>> This is another file needed:
>>
>> u-boot-test-quit
>> #!/bin/sh
>> echo quit | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
> 
> The test/py framework doesn't execute "u-boot-test-quit. Adding such a
> file won't affect anything.
> 
>> The following script comes in handy to create the .py file:
>>
>> #!/bin/bash
>> echo env__efi_loader_$(basename $1 | sed 's/\./_/g') = \{
>> echo '    "fn":' $(basename $1)
>> echo '    "len":' $(stat --printf="%s" $1)
>> echo '    "crc32":' $(crc32 $1)
>> echo \}
> 
> Let's just specify the content of the Python file (which the user can
> simply cut/past) rather than making life complicated by writing a shell
> script to create the Python file.
> 
>>>> +    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
>>>> +    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
>>>> +
>>>> +In `u-boot-test-reset` call the socat command to send a system reset:
>>>> +
>>>> +    #!/bin/sh
>>>> +    echo system_reset | socat -
>>>> UNIX-CONNECT:/tmp/u-boot-monitor-socket
>>>> +    sleep 1
>>>> +    true
>>>
>>> Why is the sleep needed?
>>
>> This avoids race conditions.
>> Qemu will need some milliseconds to actually shut down qemu.
>> I want to be sure that Python does not execute any command before this
>> is completed.
> 
> I don't believe there's any issue here. test/py will wait for qemu to
> boot U-Boot before attempting to send any commands after the reset
> occurs, and that wait operation can start as soon as the reset trigger
> is sent. Did you observe any issue in practice?
> 
>>> The true command shouldn't have any effect
>>> given set -e isn't in use.
>>
>> man dash:
>> The shell will return the exit status of the last command executed.
>>
>> If the last command is false running the test suite fails.
> 
> OK. Why would either the echo or sleep fail? If they do, then that
> failure should be passed back to test/py so that it can record the
> problem. Errors shouldn't just be ignored.
> 

true is really needed here. The return code of the script otherwise is
always false even though the system reset succeeds.

If you really want to analyze if qemu successfully executes the control
commands you have to use qmp.

But I wanted to create a trivial example and not start parsing JSON.

Just for reference, this is how you would do a system reset in qmp:

$ socat - /tmp/u-boot-monitor-socket
{"QMP": {"version": {"qemu": {"micro": 1, "minor": 8, "major": 2},
"package": "(Debian 1:2.8+dfsg-6+deb9u2)"}, "capabilities": []}}
==> { "execute": "qmp_capabilities" }
{"return": {}}
==> { "execute" : "system_reset" }
{"return": {}}
{"timestamp": {"seconds": 1505849828, "microseconds": 349723}, "event":
"RESET"}

This is what a failure looks like:

==> { "execute" : "system_reset0"}
{"error": {"class": "CommandNotFound", "desc": "The command
system_reset0 has not been found"}}

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu
  2017-09-19 20:15       ` Heinrich Schuchardt
@ 2017-09-20 15:52         ` Stephen Warren
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Warren @ 2017-09-20 15:52 UTC (permalink / raw)
  To: u-boot

On 09/19/2017 02:15 PM, Heinrich Schuchardt wrote:
> On 09/18/2017 11:28 PM, Stephen Warren wrote:
>> On 09/18/2017 01:55 PM, Heinrich Schuchardt wrote:
>>> On 09/18/2017 08:27 PM, Stephen Warren wrote:
>>>> On 09/17/2017 01:32 PM, Heinrich Schuchardt wrote:
>>>>> The necessary parameters for running Python tests on qemu are
>>>>> tediouus to find.

>>>>> +    -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \
>>>>> +    -monitor unix:/tmp/u-boot-monitor-socket,server,nowait
>>>>> +
>>>>> +In `u-boot-test-reset` call the socat command to send a system reset:
>>>>> +
>>>>> +    #!/bin/sh
>>>>> +    echo system_reset | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
>>>>> +    sleep 1
>>>>> +    true

>>>> The true command shouldn't have any effect
>>>> given set -e isn't in use.
>>>
>>> man dash:
>>> The shell will return the exit status of the last command executed.
>>>
>>> If the last command is false running the test suite fails.
>>
>> OK. Why would either the echo or sleep fail? If they do, then that
>> failure should be passed back to test/py so that it can record the
>> problem. Errors shouldn't just be ignored.
> 
> true is really needed here. The return code of the script otherwise is
> always false even though the system reset succeeds.

I've tested this on my system, and such a shell script always sets the 
exit code to 0; success. echo or socat should only ever exit non-zero if 
they fail, which shouldn't happen.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-09-20 15:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-17 19:32 [U-Boot] [PATCH 1/1] test/py: provide example scripts for integrating qemu Heinrich Schuchardt
2017-09-18 17:33 ` Stephen Warren
2017-09-18 17:48   ` Tom Rini
2017-09-18 18:13   ` Heinrich Schuchardt
2017-09-18 18:27 ` Stephen Warren
2017-09-18 19:55   ` Heinrich Schuchardt
2017-09-18 21:28     ` Stephen Warren
2017-09-18 21:37       ` Heinrich Schuchardt
2017-09-19 16:18         ` Stephen Warren
2017-09-19 20:15       ` Heinrich Schuchardt
2017-09-20 15:52         ` Stephen Warren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox