4:58 p.m.
There is a chance that the current sed filtering used in
these new tests might fail in some machines due to the
repetition of the 'virsh #' prompt at the same line,
together with valid output that shouldn't be filtered.
This is output of virsh-snapshot test in my T480 dev box:
./virsh-snapshot
--- exp 2019-07-31 18:42:31.107399428 -0300
+++ out.cooked 2019-07-31 18:42:31.108399437 -0300
@@ -1,8 +1,3 @@
-
-
-Domain snapshot s3 created from 's3.xml'
-Domain snapshot s2 created from 's2.xml'
-Name: s2
Domain: test
Current: yes
State: running
There are 3 valid lines missing. This is the unfiltered output:
=== out ===
Welcome to lt-virsh, the virtualization interactive terminal.
Type: 'help' for help with commands
'quit' to quit
virsh # virsh #
virsh #
virsh # virsh # Domain snapshot s3 created from 's3.xml'
virsh # Domain snapshot s2 created from 's2.xml'
virsh # Name: s2
Domain: test
Current: yes
State: running
Location: internal
Parent: s3
Children: 0
Descendants: 0
Metadata: yes
virsh #
============
We can see that the 3 lines being erased are being followed
by the 'virsh #' prompt and the filtering is erasing those.
A similar situation happens with virsh-checkpoint as well.
This patch makes the 'sed' filtering less elegant and more crude
than the current version, but more reliable to these outputs
that may vary from each dev machine. We're also removing
the blank lines in the expected output to make it less
prone to errors as well.
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
Eric, feel free to accept this patch, tweak it, discard it and
try something else or whatever. I was going to send the
commit msg as a reply to your query in the ML, then realized
that I might as well propose a fix for it.
tests/virsh-checkpoint | 6 +-----
tests/virsh-snapshot | 6 ++----
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/tests/virsh-checkpoint b/tests/virsh-checkpoint
index 75bdc293be..a3cad74f74 100755
--- a/tests/virsh-checkpoint
+++ b/tests/virsh-checkpoint
@@ -152,20 +152,16 @@ $abs_top_builddir/tools/virsh -c test:///default >out 2>err
<<EOF || fail=1
EOF
cat <<\EOF > exp || fail=1
-
-
Domain checkpoint c3 created from 'c3.xml'
Domain checkpoint c2 created from 'c2.xml'
c2
-
Name: c2
Domain: test
Parent: c3
Children: 0
Descendants: 0
-
EOF
-sed '1,/^virsh #/d; /virsh #/d' < out > out.cooked || fail=1
+sed '1,/^virsh #/d; s/virsh #\s//g; /^$/d' < out > out.cooked || fail=1
compare exp out.cooked || fail=1
cat <<EOF > exp || fail=1
diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot
index 20ff966a51..874093ea3c 100755
--- a/tests/virsh-snapshot
+++ b/tests/virsh-snapshot
@@ -201,9 +201,8 @@ $abs_top_builddir/tools/virsh -c test:///default >out 2>err
<<EOF || fail=1
snapshot-info test --current
EOF
-cat <<\EOF > exp || fail=1
-
+cat <<\EOF > exp || fail=1
Domain snapshot s3 created from 's3.xml'
Domain snapshot s2 created from 's2.xml'
Name: s2
@@ -215,9 +214,8 @@ Parent: s3
Children: 0
Descendants: 0
Metadata: yes
-
EOF
-sed '1,/^virsh #/d; /virsh #/d' < out > out.cooked || fail=1
+sed '1,/^virsh #/d; s/virsh #\s//g; /^$/d' < out > out.cooked || fail=1
compare exp out.cooked || fail=1
cat <<EOF > exp || fail=1
--
2.21.0
6:30 a.m.
On 7/31/19 4:58 PM, Daniel Henrique Barboza wrote:
There is a chance that the current sed filtering used in
these new tests might fail in some machines due to the
repetition of the 'virsh #' prompt at the same line,
together with valid output that shouldn't be filtered.
Ah, so it is a data race where the prompts produced by virsh don't
always flush in relation to other data being output.
There are 3 valid lines missing. This is the unfiltered output:
=== out ===
Welcome to lt-virsh, the virtualization interactive terminal.
Type: 'help' for help with commands
'quit' to quit
virsh # virsh #
virsh #
virsh # virsh # Domain snapshot s3 created from 's3.xml'
virsh # Domain snapshot s2 created from 's2.xml'
virsh # Name: s2
This patch makes the 'sed' filtering less elegant and more
crude
than the current version, but more reliable to these outputs
that may vary from each dev machine. We're also removing
the blank lines in the expected output to make it less
prone to errors as well.
Yes, eating all blank lines and just the prompt sub-string rather than
the entire line with a prompt is a nice fix, and safe for freeze.
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
Eric, feel free to accept this patch, tweak it, discard it and
try something else or whatever. I was going to send the
commit msg as a reply to your query in the ML, then realized
that I might as well propose a fix for it.
ACK.
EOF
-sed '1,/^virsh #/d; /virsh #/d' < out > out.cooked || fail=1
+sed '1,/^virsh #/d; s/virsh #\s//g; /^$/d' < out > out.cooked || fail=1
So this is now: delete the welcome message up to the first prompt, then
remove all prompts and blank lines.
compare exp out.cooked || fail=1
cat <<EOF > exp || fail=1
diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot
index 20ff966a51..874093ea3c 100755
--- a/tests/virsh-snapshot
+++ b/tests/virsh-snapshot
@@ -201,9 +201,8 @@ $abs_top_builddir/tools/virsh -c test:///default >out 2>err
<<EOF || fail=1
snapshot-info test --current
EOF
-cat <<\EOF > exp || fail=1
-
+cat <<\EOF > exp || fail=1
Looks like you actually added a blank line between the previous EOF and
the cat. Easy enough to tweak; I'll push this soon.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
6:43 a.m.
On 8/1/19 6:30 AM, Eric Blake wrote:
On 7/31/19 4:58 PM, Daniel Henrique Barboza wrote:
> There is a chance that the current sed filtering used in
> these new tests might fail in some machines due to the
> repetition of the 'virsh #' prompt at the same line,
> together with valid output that shouldn't be filtered.
Ah, so it is a data race where the prompts produced by virsh don't
always flush in relation to other data being output.
Yes, eating all blank lines and just the prompt sub-string rather
than
the entire line with a prompt is a nice fix, and safe for freeze.
Actually, when I apply your patch, I run into failures on my end:
--- exp 2019-08-01 06:37:33.108617030 -0500
+++ out.cooked 2019-08-01 06:37:33.110617032 -0500
@@ -1,5 +1,11 @@
+ snapshot-create test --redefine s2.xml --validate
+ echo --err marker
+ # This is the right order
+ snapshot-create test --redefine s3.xml --validate
Domain snapshot s3 created from 's3.xml'
+ snapshot-create test --redefine s2.xml --current --validate
Domain snapshot s2 created from 's2.xml'
+ snapshot-info test --current
Name: s2
Domain: test
Current: yes
FAIL virsh-snapshot (exit status: 1)
So I think the difference is that your dev box is not echoing the
commands, and the real problem is that the test is dependent on the
current environment (is there some configuration file that determines
whether virsh in batch mode will echo what is typed?)
It would be possible to change the tests to use
virsh ... 'sequence of commands'
instead of
virsh ... <<EOF
sequence of commands
EOF
(in fact, that's what we do in the first half of the script); when you
do that, you no longer get the 'virsh #' prompts, nor any problem with
command echo. But before doing that, I'd still like to understand what
is different about your environment that suppresses the echo in the
first place, to encounter the output data race.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
7:49 a.m.
On 8/1/19 8:43 AM, Eric Blake wrote:
On 8/1/19 6:30 AM, Eric Blake wrote:
> On 7/31/19 4:58 PM, Daniel Henrique Barboza wrote:
>> There is a chance that the current sed filtering used in
>> these new tests might fail in some machines due to the
>> repetition of the 'virsh #' prompt at the same line,
>> together with valid output that shouldn't be filtered.
> Ah, so it is a data race where the prompts produced by virsh don't
> always flush in relation to other data being output.
>
> Yes, eating all blank lines and just the prompt sub-string rather than
> the entire line with a prompt is a nice fix, and safe for freeze.
Actually, when I apply your patch, I run into failures on my end:
--- exp 2019-08-01 06:37:33.108617030 -0500
+++ out.cooked 2019-08-01 06:37:33.110617032 -0500
@@ -1,5 +1,11 @@
+ snapshot-create test --redefine s2.xml --validate
+ echo --err marker
+ # This is the right order
+ snapshot-create test --redefine s3.xml --validate
Domain snapshot s3 created from 's3.xml'
+ snapshot-create test --redefine s2.xml --current --validate
Domain snapshot s2 created from 's2.xml'
+ snapshot-info test --current
Name: s2
Domain: test
Current: yes
FAIL virsh-snapshot (exit status: 1)
So I think the difference is that your dev box is not echoing the
commands, and the real problem is that the test is dependent on the
current environment (is there some configuration file that determines
whether virsh in batch mode will echo what is typed?)
It would be possible to change the tests to use
virsh ... 'sequence of commands'
instead of
virsh ... <<EOF
sequence of commands
EOF
(in fact, that's what we do in the first half of the script); when you
do that, you no longer get the 'virsh #' prompts, nor any problem with
command echo. But before doing that, I'd still like to understand what
is different about your environment that suppresses the echo in the
first place, to encounter the output data race.
Sure. This is how I build Libvirt and run the tests:
./autogen.sh --prefix=$PWD/usr
make -j && make syntax-check && make check
General machine info follows:
[danielhb@rekt libvirt]$ uname -a
Linux rekt 5.1.15-300.fc30.x86_64 #1 SMP Tue Jun 25 14:07:22 UTC 2019
x86_64 x86_64 x86_64 GNU/Linux
[danielhb@rekt libvirt]$
[danielhb@rekt libvirt]$ lscpu
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
Address sizes: 39 bits physical, 48 bits virtual
CPU(s): 8
On-line CPU(s) list: 0-7
Thread(s) per core: 2
Core(s) per socket: 4
Socket(s): 1
NUMA node(s): 1
Vendor ID: GenuineIntel
CPU family: 6
Model: 142
Model name: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz
Stepping: 10
CPU MHz: 3200.113
CPU max MHz: 4200.0000
CPU min MHz: 400.0000
BogoMIPS: 4224.00
Virtualization: VT-x
L1d cache: 32K
L1i cache: 32K
L2 cache: 256K
L3 cache: 8192K
NUMA node0 CPU(s): 0-7
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts
rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni
pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr
pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave
avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb invpcid_single
pti ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority ept vpid ept_ad
fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm mpx rdseed
adx smap clflushopt intel_pt xsaveopt xsavec xgetbv1 xsaves dtherm ida
arat pln pts hwp hwp_notify hwp_act_window hwp_epp md_clear flush_l1d
[danielhb@rekt libvirt]$
[danielhb@rekt libvirt]$ lsb_release -a
LSB Version:
:core-4.1-amd64:core-4.1-noarch:cxx-4.1-amd64:cxx-4.1-noarch:desktop-4.1-amd64:desktop-4.1-noarch:languages-4.1-amd64:languages-4.1-noarch:printing-4.1-amd64:printing-4.1-noarch
Distributor ID: Fedora
Description: Fedora release 30 (Thirty)
Release: 30
Codename: Thirty
[danielhb@rekt libvirt]$
[danielhb@rekt libvirt]$ cat /proc/meminfo
MemTotal: 32682444 kB
MemFree: 2585380 kB
MemAvailable: 22865820 kB
Buffers: 1780684 kB
Cached: 19278004 kB
SwapCached: 768 kB
Active: 18504408 kB
Inactive: 9039088 kB
Active(anon): 6574348 kB
Inactive(anon): 1445640 kB
Active(file): 11930060 kB
Inactive(file): 7593448 kB
Unevictable: 656044 kB
Mlocked: 64 kB
SwapTotal: 16408572 kB
SwapFree: 16400452 kB
Dirty: 2220 kB
Writeback: 0 kB
AnonPages: 7140812 kB
Mapped: 1772148 kB
Shmem: 1535216 kB
KReclaimable: 1232012 kB
Slab: 1529188 kB
SReclaimable: 1232012 kB
SUnreclaim: 297176 kB
KernelStack: 29600 kB
PageTables: 102972 kB
NFS_Unstable: 0 kB
Bounce: 0 kB
WritebackTmp: 0 kB
CommitLimit: 32749792 kB
Committed_AS: 27285012 kB
VmallocTotal: 34359738367 kB
VmallocUsed: 0 kB
VmallocChunk: 0 kB
Percpu: 12608 kB
HardwareCorrupted: 0 kB
AnonHugePages: 0 kB
ShmemHugePages: 0 kB
ShmemPmdMapped: 0 kB
CmaTotal: 0 kB
CmaFree: 0 kB
HugePages_Total: 0
HugePages_Free: 0
HugePages_Rsvd: 0
HugePages_Surp: 0
Hugepagesize: 2048 kB
Hugetlb: 0 kB
DirectMap4k: 976512 kB
DirectMap2M: 29175808 kB
DirectMap1G: 4194304 kB
I'll see if I can get different results running the tests in a Power 8/9
server.
I'll also do a clean 'git clone' run to see if the results differ.
9:24 a.m.
On 8/1/19 9:49 AM, Daniel Henrique Barboza wrote:
On 8/1/19 8:43 AM, Eric Blake wrote:
> On 8/1/19 6:30 AM, Eric Blake wrote:
>> On 7/31/19 4:58 PM, Daniel Henrique Barboza wrote:
>>> There is a chance that the current sed filtering used in
>>> these new tests might fail in some machines due to the
>>> repetition of the 'virsh #' prompt at the same line,
>>> together with valid output that shouldn't be filtered.
>> Ah, so it is a data race where the prompts produced by virsh don't
>> always flush in relation to other data being output.
>>
>> Yes, eating all blank lines and just the prompt sub-string rather than
>> the entire line with a prompt is a nice fix, and safe for freeze.
> Actually, when I apply your patch, I run into failures on my end:
>
> --- exp 2019-08-01 06:37:33.108617030 -0500
> +++ out.cooked 2019-08-01 06:37:33.110617032 -0500
> @@ -1,5 +1,11 @@
> + snapshot-create test --redefine s2.xml --validate
> + echo --err marker
> + # This is the right order
> + snapshot-create test --redefine s3.xml --validate
> Domain snapshot s3 created from 's3.xml'
> + snapshot-create test --redefine s2.xml --current --validate
> Domain snapshot s2 created from 's2.xml'
> + snapshot-info test --current
> Name: s2
> Domain: test
> Current: yes
> FAIL virsh-snapshot (exit status: 1)
>
> So I think the difference is that your dev box is not echoing the
> commands, and the real problem is that the test is dependent on the
> current environment (is there some configuration file that determines
> whether virsh in batch mode will echo what is typed?)
>
> It would be possible to change the tests to use
> virsh ... 'sequence of commands'
> instead of
> virsh ... <<EOF
> sequence of commands
> EOF
>
> (in fact, that's what we do in the first half of the script); when you
> do that, you no longer get the 'virsh #' prompts, nor any problem with
> command echo. But before doing that, I'd still like to understand what
> is different about your environment that suppresses the echo in the
> first place, to encounter the output data race.
Sure. This is how I build Libvirt and run the tests:
./autogen.sh --prefix=$PWD/usr
make -j && make syntax-check && make check
[...]
I'll see if I can get different results running the tests in a Power
8/9 server.
I'll also do a clean 'git clone' run to see if the results differ.
Tested with a fresh git clone on my dev box, a Power 8 server and a Power 9
server. The result is the same:
[danielhb@ltc-boston74 libvirt]$ ./tests/virsh-snapshot
--- exp 2019-08-01 10:12:47.344963622 -0400
+++ out.cooked 2019-08-01 10:12:47.344963622 -0400
@@ -1,8 +1,3 @@
-
-
-Domain snapshot s3 created from 's3.xml'
-Domain snapshot s2 created from 's2.xml'
-Name: s2
Domain: test
Current: yes
State: running
[danielhb@ltc-boston74 libvirt]$ ./tests/virsh-checkpoint
--- exp 2019-08-01 10:13:06.564240177 -0400
+++ out.cooked 2019-08-01 10:13:06.564240177 -0400
@@ -1,10 +1,4 @@
-
-Domain checkpoint c3 created from 'c3.xml'
-Domain checkpoint c2 created from 'c2.xml'
-c2
-
-Name: c2
Domain: test
Parent: c3
Children: 0
[danielhb@ltc-boston74 libvirt]$
So yeah. I am not sure what's different from the environments I use versus
the one you're using (and probably the env of every other committer here,
otherwise this test failure would be pointed out earlier). I'll need to
think
more about it.
Thanks,
DHB
10:31 a.m.
On 8/1/19 9:24 AM, Daniel Henrique Barboza wrote:
>>
>> So I think the difference is that your dev box is not echoing the
>> commands, and the real problem is that the test is dependent on the
>> current environment (is there some configuration file that determines
>> whether virsh in batch mode will echo what is typed?)
> I'll see if I can get different results running the tests in
a Power
> 8/9 server.
> I'll also do a clean 'git clone' run to see if the results differ.
>
Tested with a fresh git clone on my dev box, a Power 8 server and a Power 9
server. The result is the same:
Do you have some file under ~/.config/libvirt, or maybe contents in
~/.inputrc, or some similar problem that is being pulled in regardless
of your build setup? Would an strace prove what file is getting opened
outside of the control of the test, where we need to fix our
sanitization to avoid the test depending on your home directory?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
3:36 p.m.
On 8/1/19 11:24 AM, Daniel Henrique Barboza wrote:
[...]
So yeah. I am not sure what's different from the environments I use
versus
the one you're using (and probably the env of every other committer here,
otherwise this test failure would be pointed out earlier). I'll need
to think
more about it.
I had to take a look at vsh.c to understand, but I finally got it that
it was
the absence of 'readline-devel' library in all the systems I tested.
Installing
this library made the test work as you intended in my box. It also fixed in
the Power servers as well.
Without the library, virsh interactive mode doesn't stop work per se,
but the
commands aren't echoed back to the output file (as we can see in the
code that is conditional of WITH_READLINE inside vsh.c).
Eric, feel free to change the tests as you see fit. Your idea a couple of
messages earlier (avoid using virsh interactive mode, use batch mode
instead) looks sound. It also avoid making the test dependent on the
install of readline-devel.
Thanks,
DHB
Thanks,
DHB
>
>
8:28 a.m.
On 8/1/19 3:36 PM, Daniel Henrique Barboza wrote:
On 8/1/19 11:24 AM, Daniel Henrique Barboza wrote:
>
> [...]
> So yeah. I am not sure what's different from the environments I use
> versus
> the one you're using (and probably the env of every other committer here,
> otherwise this test failure would be pointed out earlier). I'll need
> to think
> more about it.
>
I had to take a look at vsh.c to understand, but I finally got it that
it was
the absence of 'readline-devel' library in all the systems I tested.
Installing
this library made the test work as you intended in my box. It also fixed in
the Power servers as well.
Without the library, virsh interactive mode doesn't stop work per se,
but the
commands aren't echoed back to the output file (as we can see in the
code that is conditional of WITH_READLINE inside vsh.c).
Eric, feel free to change the tests as you see fit. Your idea a couple of
messages earlier (avoid using virsh interactive mode, use batch mode
instead) looks sound. It also avoid making the test dependent on the
install of readline-devel.
Okay, I've posted that alternative proposal of avoiding virsh
interactive mode (we can worry about the difference in command echoing
without readline-devel later).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1941
days inactive
1943
days old
7 comments
2 participants
participants (2)
-
Daniel Henrique Barboza -
Eric Blake