[libvirt] [PATCH 1/1] tests/virsh-checkpoint/snapshot: changing 'sed' out filtering
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@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
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@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
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
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.
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
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
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
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
participants (2)
-
Daniel Henrique Barboza -
Eric Blake