Message ID | 20200310165332.140774-1-liran.alon@oracle.com (mailing list archive) |
---|---|
Headers | show |
Series | : hw/i386/vmport: Bug fixes and improvements | expand |
On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: > Hi, > > This series aims to fix several bugs in VMPort and improve it by supporting > more VMPort commands and make command results more configurable to > user via QEMU command-line. > > This functionality was proven to be useful to run various VMware VMs > when attempting to run them as-is on top of QEMU/KVM. > > For more details, see commit messages. Well two versions in one day and some review comments weren't addressed. Some people do this, try to wear the maintainers out by sheer volume. It works sometimes but it's not a nice tactic. I personally think it's worth taking the time to think harder about ways to address all comments, not try to dismiss them. Thanks! > Regards, > -Liran > > v1->v2: > * Fix coding convention [Patchew Bot & MST]. > * Create new header file for vmport.h [MST]. > * Move KVM_APIC_BUS_FREQUENCY from linux-headers/asm-x86/kvm.h > auto-generated header [MST] > * Elaborate more that vmx-version refers to the VMware userspace > VMM in commit message. [MST] > * Use le32_to_cpu() on BIOS_UUID vmport command. [MST] > * Introduce VMPort compatability version property to maintain migration > compatibility. [MST]
On 10/03/2020 19:44, Michael S. Tsirkin wrote: > On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: >> Hi, >> >> This series aims to fix several bugs in VMPort and improve it by supporting >> more VMPort commands and make command results more configurable to >> user via QEMU command-line. >> >> This functionality was proven to be useful to run various VMware VMs >> when attempting to run them as-is on top of QEMU/KVM. >> >> For more details, see commit messages. > Well two versions in one day and some review comments weren't addressed. There is a single review comment that wasn't addressed which is replacing an enum with a comment. And I explicitly mentioned that it's because I want additional opinion on this. I don't see why such a small thing should block review for 15 patches... All the rest of the comments (Which were great) have been addressed. Unless I have mistakenly missed something, which please point it out if I did. > Some people do this, try to wear the maintainers out by sheer volume. > It works sometimes but it's not a nice tactic. I personally think it's > worth taking the time to think harder about ways to address all > comments, not try to dismiss them. That's not what I tried to do. I carefully fixed all comments I saw in the review discussion and run tests. The only thing which wasn't addressed is removing an enum and replacing it with a comment. The hint that I try to manipulate maintainers is disrespectful. I assume that this isn't your intention, as we all just want to collaborate together here. No need to make this a personal discussion. If you think that replacing the enum with a comment is a blocker for v2 patch-series, I will go ahead and submit v3 with that change. Is there any other comment you made on v1 patch-series you think I missed? Thanks, -Liran > > Thanks! > >> Regards, >> -Liran >> >> v1->v2: >> * Fix coding convention [Patchew Bot & MST]. >> * Create new header file for vmport.h [MST]. >> * Move KVM_APIC_BUS_FREQUENCY from linux-headers/asm-x86/kvm.h >> auto-generated header [MST] >> * Elaborate more that vmx-version refers to the VMware userspace >> VMM in commit message. [MST] >> * Use le32_to_cpu() on BIOS_UUID vmport command. [MST] >> * Introduce VMPort compatability version property to maintain migration >> compatibility. [MST]
Patchew URL: https://patchew.org/QEMU/20200310165332.140774-1-liran.alon@oracle.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === PASS 32 test-opts-visitor /visitor/opts/range/beyond PASS 33 test-opts-visitor /visitor/opts/dict/unvisited MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine" ==6415==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-coroutine /basic/no-dangling-access PASS 2 test-coroutine /basic/lifecycle PASS 3 test-coroutine /basic/yield ==6415==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd80047000; bottom 0x7f8d591b3000; size: 0x007026e94000 (481689157632) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 4 test-coroutine /basic/nesting --- PASS 1 fdc-test /x86_64/fdc/cmos PASS 2 fdc-test /x86_64/fdc/no_media_on_start PASS 3 fdc-test /x86_64/fdc/read_without_media ==6426==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 fdc-test /x86_64/fdc/media_change PASS 5 fdc-test /x86_64/fdc/sense_interrupt PASS 6 fdc-test /x86_64/fdc/relative_seek --- PASS 12 test-aio /aio/event/flush PASS 13 test-aio /aio/event/wait/no-flush-cb PASS 14 test-aio /aio/timer/schedule ==6440==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 15 test-aio /aio/coroutine/queue-chaining PASS 16 test-aio /aio-gsource/flush PASS 17 test-aio /aio-gsource/bh/schedule --- PASS 11 fdc-test /x86_64/fdc/read_no_dma_18 PASS 28 test-aio /aio-gsource/timer/schedule MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread" ==6445==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-aio-multithread /aio/multi/lifecycle PASS 2 test-aio-multithread /aio/multi/schedule PASS 3 test-aio-multithread /aio/multi/mutex/contended PASS 12 fdc-test /x86_64/fdc/read_no_dma_19 PASS 13 fdc-test /x86_64/fdc/fuzz-registers MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" ==6472==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 ide-test /x86_64/ide/identify ==6478==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 test-aio-multithread /aio/multi/mutex/handoff PASS 2 ide-test /x86_64/ide/flush PASS 5 test-aio-multithread /aio/multi/mutex/mcs ==6489==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 ide-test /x86_64/ide/bmdma/simple_rw PASS 6 test-aio-multithread /aio/multi/mutex/pthread MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle" ==6506==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-throttle /throttle/leak_bucket PASS 2 test-throttle /throttle/compute_wait PASS 3 test-throttle /throttle/init --- PASS 14 test-throttle /throttle/config/max PASS 15 test-throttle /throttle/config/iops_size MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool" ==6500==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==6510==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-thread-pool /thread-pool/submit PASS 2 test-thread-pool /thread-pool/submit-aio PASS 3 test-thread-pool /thread-pool/submit-co PASS 4 test-thread-pool /thread-pool/submit-many PASS 4 ide-test /x86_64/ide/bmdma/trim ==6564==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 5 test-thread-pool /thread-pool/cancel PASS 6 test-thread-pool /thread-pool/cancel-async MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-hbitmap" --- PASS 15 test-hbitmap /hbitmap/set/overlap PASS 16 test-hbitmap /hbitmap/reset/empty PASS 17 test-hbitmap /hbitmap/reset/general ==6588==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 18 test-hbitmap /hbitmap/reset/all PASS 19 test-hbitmap /hbitmap/truncate/nop PASS 20 test-hbitmap /hbitmap/truncate/grow/negligible --- PASS 28 test-hbitmap /hbitmap/truncate/shrink/medium PASS 29 test-hbitmap /hbitmap/truncate/shrink/large PASS 30 test-hbitmap /hbitmap/meta/zero ==6594==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 31 test-hbitmap /hbitmap/meta/one PASS 32 test-hbitmap /hbitmap/meta/byte PASS 33 test-hbitmap /hbitmap/meta/word --- PASS 44 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4 PASS 45 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_after_truncate MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-drain" ==6601==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-bdrv-drain /bdrv-drain/nested PASS 2 test-bdrv-drain /bdrv-drain/multiparent PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context --- PASS 41 test-bdrv-drain /bdrv-drain/bdrv_drop_intermediate/poll PASS 42 test-bdrv-drain /bdrv-drain/replace_child/mid-drain MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bdrv-graph-mod -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-graph-mod" ==6640==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-bdrv-graph-mod /bdrv-graph-mod/update-perm-tree PASS 2 test-bdrv-graph-mod /bdrv-graph-mod/should-update-child MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-blockjob -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob" ==6644==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-blockjob /blockjob/ids PASS 2 test-blockjob /blockjob/cancel/created PASS 3 test-blockjob /blockjob/cancel/running --- PASS 7 test-blockjob /blockjob/cancel/pending PASS 8 test-blockjob /blockjob/cancel/concluded MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-blockjob-txn -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob-txn" ==6648==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-blockjob-txn /single/success PASS 2 test-blockjob-txn /single/failure PASS 3 test-blockjob-txn /single/cancel --- PASS 6 test-blockjob-txn /pair/cancel PASS 7 test-blockjob-txn /pair/fail-cancel-race MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-block-backend -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-backend" ==6654==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-block-backend /block-backend/drain_aio_error PASS 2 test-block-backend /block-backend/drain_all_aio_error MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-block-iothread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-iothread" ==6658==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-block-iothread /sync-op/pread PASS 2 test-block-iothread /sync-op/pwrite PASS 3 test-block-iothread /sync-op/load_vmstate --- PASS 15 test-block-iothread /propagate/diamond PASS 16 test-block-iothread /propagate/mirror MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-image-locking -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-image-locking" ==6650==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==6681==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-image-locking /image-locking/basic PASS 2 test-image-locking /image-locking/set-perm-abort MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-x86-cpuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid" --- PASS 1 rcutorture /rcu/torture/1reader PASS 2 rcutorture /rcu/torture/10readers MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-list" ==6740==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-rcu-list /rcu/qlist/single-threaded PASS 2 test-rcu-list /rcu/qlist/short-few PASS 3 test-rcu-list /rcu/qlist/long-many MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-simpleq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-simpleq" PASS 1 test-rcu-simpleq /rcu/qsimpleq/single-threaded PASS 2 test-rcu-simpleq /rcu/qsimpleq/short-few ==6806==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 test-rcu-simpleq /rcu/qsimpleq/long-many MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-tailq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-tailq" PASS 1 test-rcu-tailq /rcu/qtailq/single-threaded PASS 2 test-rcu-tailq /rcu/qtailq/short-few ==6845==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 test-rcu-tailq /rcu/qtailq/long-many MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-slist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-slist" PASS 1 test-rcu-slist /rcu/qslist/single-threaded --- PASS 7 test-qdist /qdist/binning/expand PASS 8 test-qdist /qdist/binning/shrink MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qht -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht" ==6891==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==6897==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 5 ide-test /x86_64/ide/bmdma/various_prdts ==6903==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==6903==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe2c103000; bottom 0x7f0636126000; size: 0x00f7f5fdd000 (1064983973888) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 6 ide-test /x86_64/ide/bmdma/no_busmaster --- PASS 7 ide-test /x86_64/ide/flush/nodev PASS 2 test-qht /qht/mode/resize MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qht-par -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht-par" ==6914==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 8 ide-test /x86_64/ide/flush/empty_drive ==6928==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 9 ide-test /x86_64/ide/flush/retry_pci PASS 1 test-qht-par /qht/parallel/2threads-0%updates-1s ==6934==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 10 ide-test /x86_64/ide/flush/retry_isa PASS 2 test-qht-par /qht/parallel/2threads-20%updates-1s MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bitops -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bitops" --- PASS 3 test-bitcnt /bitcnt/ctpop32 PASS 4 test-bitcnt /bitcnt/ctpop64 MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qdev-global-props -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qdev-global-props" ==6946==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-qdev-global-props /qdev/properties/static/default PASS 2 test-qdev-global-props /qdev/properties/static/global PASS 3 test-qdev-global-props /qdev/properties/dynamic/global --- PASS 8 check-qom-proplist /qom/proplist/delchild PASS 9 check-qom-proplist /qom/resolve/partial MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qemu-opts -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qemu-opts" ==6976==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-qemu-opts /qemu-opts/find_unknown_opts PASS 2 test-qemu-opts /qemu-opts/find_opts PASS 3 test-qemu-opts /qemu-opts/opts_create --- PASS 15 test-crypto-secret /crypto/secret/crypt/missingiv PASS 16 test-crypto-secret /crypto/secret/crypt/badiv MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-crypto-tlscredsx509 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlscredsx509" ==7013==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 13 ide-test /x86_64/ide/cdrom/dma MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test" ==7032==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 ahci-test /x86_64/ahci/sanity PASS 1 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectserver PASS 2 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectclient ==7038==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 2 ahci-test /x86_64/ahci/pci_spec PASS 3 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca1 PASS 4 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca2 ==7044==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 ahci-test /x86_64/ahci/pci_enable PASS 5 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca3 PASS 6 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca1 PASS 7 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca2 PASS 8 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca3 ==7050==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 9 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver1 PASS 4 ahci-test /x86_64/ahci/hba_spec PASS 10 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver2 PASS 11 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver3 ==7056==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 5 ahci-test /x86_64/ahci/hba_enable ==7062==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 6 ahci-test /x86_64/ahci/identify PASS 12 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver4 PASS 13 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver5 ==7068==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 7 ahci-test /x86_64/ahci/max PASS 14 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver6 ==7074==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 15 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver7 PASS 16 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badserver1 PASS 17 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badserver2 --- PASS 33 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive2 PASS 34 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive3 PASS 8 ahci-test /x86_64/ahci/reset ==7080==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7080==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe0f455000; bottom 0x7f4860ffe000; size: 0x00b5ae457000 (780312866816) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 35 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain1 --- PASS 39 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingclient MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-crypto-tlssession -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlssession" PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero ==7090==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7090==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc1f65d000; bottom 0x7fae6cffe000; size: 0x004db265f000 (333705506816) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low PASS 1 test-crypto-tlssession /qcrypto/tlssession/psk ==7096==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 2 test-crypto-tlssession /qcrypto/tlssession/basicca ==7096==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffe6f0c000; bottom 0x7fd8b2bfe000; size: 0x00273430e000 (168379342848) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 3 test-crypto-tlssession /qcrypto/tlssession/differentca PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high ==7102==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7102==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc3c960000; bottom 0x7f48799fe000; size: 0x00b3c2f62000 (772070055936) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 4 test-crypto-tlssession /qcrypto/tlssession/altname1 PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero PASS 5 test-crypto-tlssession /qcrypto/tlssession/altname2 ==7108==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7108==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffff1b4a000; bottom 0x7feb3b1fe000; size: 0x0014b694c000 (88962547712) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low ==7114==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7114==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffde1579000; bottom 0x7fce125fe000; size: 0x002fcef7b000 (205335801856) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high PASS 6 test-crypto-tlssession /qcrypto/tlssession/altname3 ==7120==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7120==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd918ce000; bottom 0x7ff4f557c000; size: 0x00089c352000 (36980465664) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero ==7126==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 7 test-crypto-tlssession /qcrypto/tlssession/altname4 ==7126==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff419c0000; bottom 0x7f61c65fe000; size: 0x009d7b3c2000 (676377403392) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low PASS 8 test-crypto-tlssession /qcrypto/tlssession/altname5 PASS 9 test-crypto-tlssession /qcrypto/tlssession/altname6 ==7132==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7132==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd9a2dc000; bottom 0x7fc2ce324000; size: 0x003acbfb8000 (252530360320) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 10 test-crypto-tlssession /qcrypto/tlssession/wildcard1 PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high PASS 11 test-crypto-tlssession /qcrypto/tlssession/wildcard2 ==7138==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero ==7144==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 12 test-crypto-tlssession /qcrypto/tlssession/wildcard3 PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low ==7150==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high PASS 13 test-crypto-tlssession /qcrypto/tlssession/wildcard4 ==7156==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 14 test-crypto-tlssession /qcrypto/tlssession/wildcard5 ==7156==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe0bada000; bottom 0x7f63419fe000; size: 0x009aca0dc000 (664814862336) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero PASS 15 test-crypto-tlssession /qcrypto/tlssession/wildcard6 ==7162==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7162==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdfb406000; bottom 0x7fccccbfe000; size: 0x00312e808000 (211233570816) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low PASS 16 test-crypto-tlssession /qcrypto/tlssession/cachain MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qga -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qga" ==7168==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7168==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc7f590000; bottom 0x7f67647fe000; size: 0x00951ad92000 (640400564224) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 23 ahci-test /x86_64/ahci/io/pio/lba48/simple/high --- PASS 8 test-qga /qga/get-memory-block-info PASS 9 test-qga /qga/get-memory-blocks PASS 10 test-qga /qga/file-ops ==7182==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 11 test-qga /qga/file-write-read PASS 12 test-qga /qga/get-time PASS 13 test-qga /qga/id --- PASS 15 test-qga /qga/invalid-cmd PASS 16 test-qga /qga/invalid-args PASS 17 test-qga /qga/fsfreeze-status ==7182==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffef0b02000; bottom 0x7f9513bfe000; size: 0x0069dcf04000 (454678298624) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero PASS 18 test-qga /qga/blacklist PASS 19 test-qga /qga/config PASS 20 test-qga /qga/guest-exec ==7191==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 21 test-qga /qga/guest-exec-invalid ==7191==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc02ee6000; bottom 0x7fb53ddfe000; size: 0x0046c50e8000 (303953772544) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low ==7209==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 22 test-qga /qga/guest-get-osinfo PASS 23 test-qga /qga/guest-get-host-name PASS 24 test-qga /qga/guest-get-timezone PASS 25 test-qga /qga/guest-get-users MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-timed-average -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-timed-average" ==7209==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd01a46000; bottom 0x7fbcf65fe000; size: 0x00400b448000 (275066945536) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 1 test-timed-average /timed-average/average --- PASS 7 test-util-sockets /socket/fd-pass/num/bad PASS 8 test-util-sockets /socket/fd-pass/num/nocli MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-authz-simple -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-simple" ==7225==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-authz-simple /authz/simple MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-authz-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-list" PASS 1 test-authz-list /auth/list/complex --- PASS 5 test-authz-list /auth/list/explicit/deny PASS 6 test-authz-list /auth/list/explicit/allow MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-authz-listfile -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-listfile" ==7225==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd64491000; bottom 0x7f057d724000; size: 0x00f7e6d6d000 (1064729759744) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 1 test-authz-listfile /auth/list/complex --- PASS 8 test-io-channel-socket /io/channel/socket/unix-fd-pass PASS 9 test-io-channel-socket /io/channel/socket/unix-listen-cleanup MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-io-channel-file -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-file" ==7256==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-io-channel-file /io/channel/file PASS 2 test-io-channel-file /io/channel/file/rdwr PASS 3 test-io-channel-file /io/channel/file/fd PASS 4 test-io-channel-file /io/channel/pipe/sync PASS 5 test-io-channel-file /io/channel/pipe/async MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-io-channel-tls -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-tls" ==7256==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd866c7000; bottom 0x7f0bff5fe000; size: 0x00f1870c9000 (1037352865792) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 1 test-io-channel-tls /qio/channel/tls/basic --- PASS 17 test-crypto-pbkdf /crypto/pbkdf/nonrfc/sha384/iter1200 PASS 18 test-crypto-pbkdf /crypto/pbkdf/nonrfc/ripemd160/iter1200 MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-crypto-ivgen -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-ivgen" ==7319==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-crypto-ivgen /crypto/ivgen/plain/1 PASS 2 test-crypto-ivgen /crypto/ivgen/plain/1f2e3d4c PASS 3 test-crypto-ivgen /crypto/ivgen/plain/1f2e3d4c5b6a7988 --- PASS 3 test-crypto-afsplit /crypto/afsplit/sha256/big PASS 4 test-crypto-afsplit /crypto/afsplit/sha1/1000 MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-crypto-xts -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-xts" ==7319==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff4d3c5000; bottom 0x7f763d3fe000; size: 0x00890ffc7000 (588678721536) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 1 test-crypto-xts /crypto/xts/t-1-key-32-ptx-32/basic --- PASS 3 test-logging /logging/logfile_write_path PASS 4 test-logging /logging/logfile_lock_path MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-replication -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-replication" ==7359==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7352==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-replication /replication/primary/read PASS 30 ahci-test /x86_64/ahci/io/pio/lba48/short/zero PASS 2 test-replication /replication/primary/write ==7367==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low PASS 3 test-replication /replication/primary/start PASS 4 test-replication /replication/primary/stop PASS 5 test-replication /replication/primary/do_checkpoint PASS 6 test-replication /replication/primary/get_error_all ==7373==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 7 test-replication /replication/secondary/read PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high ==7379==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 8 test-replication /replication/secondary/write PASS 33 ahci-test /x86_64/ahci/io/dma/lba28/fragmented ==7385==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 34 ahci-test /x86_64/ahci/io/dma/lba28/retry ==7391==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 35 ahci-test /x86_64/ahci/io/dma/lba28/simple/zero ==7359==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff37f5b000; bottom 0x7ffa0b9bd000; size: 0x00052c59e000 (22218924032) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 ==7409==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 9 test-replication /replication/secondary/start PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low ==7421==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high ==7427==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero ==7433==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low PASS 10 test-replication /replication/secondary/stop ==7439==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high ==7445==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7445==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc4e206000; bottom 0x7f6d31523000; size: 0x008f1cce3000 (614663598080) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero ==7452==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7452==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc106d3000; bottom 0x7f8dcc9fd000; size: 0x006e43cd6000 (473583935488) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 11 test-replication /replication/secondary/continuous_replication PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low ==7459==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7459==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffda91e3000; bottom 0x7fa4597fd000; size: 0x00594f9e6000 (383587868672) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high ==7466==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero ==7472==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 12 test-replication /replication/secondary/do_checkpoint PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low ==7478==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 13 test-replication /replication/secondary/get_error_all PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bufferiszero -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bufferiszero" ==7484==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero ==7493==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low ==7499==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high ==7505==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero ==7511==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low ==7517==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high ==7523==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7523==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff910b6000; bottom 0x7fa0e6bfd000; size: 0x005eaa4b9000 (406584004608) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 53 ahci-test /x86_64/ahci/io/dma/lba48/long/zero ==7530==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7530==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe59690000; bottom 0x7fe027923000; size: 0x001e31d6d000 (129685180416) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 54 ahci-test /x86_64/ahci/io/dma/lba48/long/low ==7537==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7537==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff5334b000; bottom 0x7f3634b23000; size: 0x00c91e828000 (863800295424) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 55 ahci-test /x86_64/ahci/io/dma/lba48/long/high ==7544==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 56 ahci-test /x86_64/ahci/io/dma/lba48/short/zero ==7550==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 57 ahci-test /x86_64/ahci/io/dma/lba48/short/low ==7556==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high ==7562==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 59 ahci-test /x86_64/ahci/io/ncq/simple ==7568==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 60 ahci-test /x86_64/ahci/io/ncq/retry ==7574==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 61 ahci-test /x86_64/ahci/flush/simple ==7580==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 62 ahci-test /x86_64/ahci/flush/retry ==7586==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7592==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 63 ahci-test /x86_64/ahci/flush/migrate ==7600==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7606==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 64 ahci-test /x86_64/ahci/migrate/sanity ==7614==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7620==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple ==7628==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7634==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 1 test-bufferiszero /cutils/bufferiszero MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-uuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-uuid" PASS 1 test-uuid /uuid/is_null --- PASS 21 test-qgraph /qgraph/test_two_test_same_interface PASS 22 test-qgraph /qgraph/test_test_in_path PASS 23 test-qgraph /qgraph/test_double_edge ==7655==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7661==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple ==7669==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7675==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted ==7683==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 69 ahci-test /x86_64/ahci/cdrom/eject ==7688==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single ==7694==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi ==7700==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single ==7706==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7706==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff8b193000; bottom 0x7f95291fe000; size: 0x006a61f95000 (456910262272) False positive error reports may follow For details see https://github.com/google/sanitizers/issues/189 PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi ==7712==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test" PASS 1 hd-geo-test /x86_64/hd-geo/ide/none ==7726==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0 ==7732==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank ==7738==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba ==7744==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs ==7750==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank ==7756==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 7 hd-geo-test /x86_64/hd-geo/ide/device/mbr/lba ==7762==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs ==7768==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs ==7773==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst ==7779==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7783==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7787==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7791==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7795==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7799==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7803==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7807==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7810==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 11 hd-geo-test /x86_64/hd-geo/override/ide ==7817==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7821==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7825==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7829==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7833==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7837==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7841==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7845==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7848==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 12 hd-geo-test /x86_64/hd-geo/override/scsi ==7855==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7859==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7863==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7867==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7871==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7875==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7879==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7883==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7886==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 13 hd-geo-test /x86_64/hd-geo/override/scsi_2_controllers ==7893==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7897==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7901==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7905==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7908==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 14 hd-geo-test /x86_64/hd-geo/override/virtio_blk ==7915==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7919==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7922==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 15 hd-geo-test /x86_64/hd-geo/override/zero_chs ==7929==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7933==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7937==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7941==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7944==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 16 hd-geo-test /x86_64/hd-geo/override/scsi_hot_unplug ==7951==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7955==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7959==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7963==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! ==7966==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 17 hd-geo-test /x86_64/hd-geo/override/virtio_hot_unplug MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test" PASS 1 boot-order-test /x86_64/boot-order/pc --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8035==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP' Using expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8041==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP' Using expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8047==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.bridge' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8053==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.ipmikcs' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8059==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.cphp' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8066==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.memhp' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8072==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.numamem' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8078==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.dimmpxm' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8087==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/pc/FACP.acpihmat' Looking for expected file 'tests/data/acpi/pc/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8094==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.bridge' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8100==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.mmio64' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8106==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.ipmibt' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8112==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.cphp' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8119==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.memhp' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8125==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.numamem' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8131==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.dimmpxm' Looking for expected file 'tests/data/acpi/q35/FACP' --- Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg ==8140==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! Looking for expected file 'tests/data/acpi/q35/FACP.acpihmat' Looking for expected file 'tests/data/acpi/q35/FACP' --- PASS 1 i440fx-test /x86_64/i440fx/defaults PASS 2 i440fx-test /x86_64/i440fx/pam PASS 3 i440fx-test /x86_64/i440fx/firmware/bios ==8232==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test" PASS 1 fw_cfg-test /x86_64/fw_cfg/signature --- MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test" PASS 1 drive_del-test /x86_64/drive_del/without-dev PASS 2 drive_del-test /x86_64/drive_del/after_failed_device_add ==8325==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! PASS 3 drive_del-test /x86_64/blockdev/drive_del_device_del MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/wdt_ib700-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="wdt_ib700-test" PASS 1 wdt_ib700-test /x86_64/wdt_ib700/pause --- dbus-daemon[8495]: Could not get password database information for UID of current process: User "???" unknown or no memory to allocate password entry ** ERROR:/tmp/qemu-test/src/tests/qtest/dbus-vmstate-test.c:114:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/dbus-vmstate-test.c:114:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18) cleaning up pid 8495 make: *** [/tmp/qemu-test/src/tests/Makefile.include:632: check-qtest-x86_64] Error 1 make: *** Waiting for unfinished jobs.... Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in <module> --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=a0f24511df5642f59a3097aa05a1968d', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-bqkyqbus/src/docker-src.2020-03-10-14.44.05.16164:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=a0f24511df5642f59a3097aa05a1968d make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-bqkyqbus/src' make: *** [docker-run-test-debug@fedora] Error 2 real 27m58.009s user 0m8.482s The full log is available at http://patchew.org/logs/20200310165332.140774-1-liran.alon@oracle.com/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: > > On 10/03/2020 19:44, Michael S. Tsirkin wrote: > > On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: > > > Hi, > > > > > > This series aims to fix several bugs in VMPort and improve it by supporting > > > more VMPort commands and make command results more configurable to > > > user via QEMU command-line. > > > > > > This functionality was proven to be useful to run various VMware VMs > > > when attempting to run them as-is on top of QEMU/KVM. > > > > > > For more details, see commit messages. > > Well two versions in one day and some review comments weren't addressed. > There is a single review comment that wasn't addressed which is replacing an > enum with a comment. And I explicitly mentioned that it's because I want > additional opinion on this. > I don't see why such a small thing should block review for 15 patches... > All the rest of the comments (Which were great) have been addressed. Unless > I have mistakenly missed something, which please point it out if I did. OK I just took a quick peek, two things quickly jumped out at me. version property really should be a boolean and have some documentation saying what functionality enables. userspace properties should use the non-abbreviated vm-executable since vmx is easy to confuse with vm extensions. That's just a quick look. > > Some people do this, try to wear the maintainers out by sheer volume. > > It works sometimes but it's not a nice tactic. I personally think it's > > worth taking the time to think harder about ways to address all > > comments, not try to dismiss them. > That's not what I tried to do. I carefully fixed all comments I saw in the > review discussion and run tests. > The only thing which wasn't addressed is removing an enum and replacing it > with a comment. > The hint that I try to manipulate maintainers is disrespectful. I assume > that this isn't your intention, as we all just want to collaborate together > here. No need to make this a personal discussion. > > If you think that replacing the enum with a comment is a blocker for v2 > patch-series, I will go ahead and submit v3 with that change. Yes IMHO it needs to be fixed but please go over the comments and try to address them all as best you can, instead of looking for an explanation why the comments were irrelevant and can be dismissed. Sure someone might propose you introduce a bug, and that can't just be addressed, but that's not the case here. Also please do not send multiple revisions of a large patchset in a day. People need time for review. > Is there any other comment you made on v1 patch-series you think I missed? > > Thanks, > -Liran > > > > > Thanks! > > > > > Regards, > > > -Liran > > > > > > v1->v2: > > > * Fix coding convention [Patchew Bot & MST]. > > > * Create new header file for vmport.h [MST]. > > > * Move KVM_APIC_BUS_FREQUENCY from linux-headers/asm-x86/kvm.h > > > auto-generated header [MST] > > > * Elaborate more that vmx-version refers to the VMware userspace > > > VMM in commit message. [MST] > > > * Use le32_to_cpu() on BIOS_UUID vmport command. [MST] > > > * Introduce VMPort compatability version property to maintain migration > > > compatibility. [MST]
On 10/03/2020 22:56, Michael S. Tsirkin wrote: > On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: >> On 10/03/2020 19:44, Michael S. Tsirkin wrote: >>> On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: >>>> Hi, >>>> >>>> This series aims to fix several bugs in VMPort and improve it by supporting >>>> more VMPort commands and make command results more configurable to >>>> user via QEMU command-line. >>>> >>>> This functionality was proven to be useful to run various VMware VMs >>>> when attempting to run them as-is on top of QEMU/KVM. >>>> >>>> For more details, see commit messages. >>> Well two versions in one day and some review comments weren't addressed. >> There is a single review comment that wasn't addressed which is replacing an >> enum with a comment. And I explicitly mentioned that it's because I want >> additional opinion on this. >> I don't see why such a small thing should block review for 15 patches... >> All the rest of the comments (Which were great) have been addressed. Unless >> I have mistakenly missed something, which please point it out if I did. > OK I just took a quick peek, two things quickly jumped out at me. Thanks for having a look. > > version property really should be a boolean and have some documentation > saying what functionality enables. I thought that having a version number approach is more generic and easy to maintain going forward. If I understand correctly, this is also the approach taken by qxl & qxl-vga. The more elaborate alternative could have been introducing compat_flags (As PVSCSI does) but it seems like it will pollute the property space with a lot of useless VMPort properties. (E.g. x-read-eax-bug, x-no-report-unsupported-cmd, x-no-report-vmx-type and etc.). What is the advantage of having a boolean such as "x-vmport-v2" instead of having a single "version" property? Will it suffice if I would just add documentation above "version" property on what is was the functionality in "version==1"? (Though, it's just easy to scan the vmport.c code for if's involving ">version"... "version" is more of an internal field for machine-type compatibility and not really meant to be used by user) Which approach do you prefer? > userspace properties should use the non-abbreviated > vm-executable since vmx is easy to confuse with vm extensions. I really wish you would reconsider this. VMX is a really common term in VMware terminology. It is found in binary names, ".vmx" file, ".vmx" file properties, VMware Tools prints, open-vm-tools source code and etc. In contrast, even though I have dealt for many years with VMware technologies, I have never known that VMX==vm-executable. I still think it will introduce much confusion. On the other hard, I don't see much confusing with this use of VMX with Intel VT-x because it is only used inside vmport.c and in vmport properties names. And the properties names match the names of the guest code that interface with vmport in open-vm-tools source code. If you still have a strong opinion on this, I will change it as you say in v3... But please consider above arguments. > > That's just a quick look. > > >>> Some people do this, try to wear the maintainers out by sheer volume. >>> It works sometimes but it's not a nice tactic. I personally think it's >>> worth taking the time to think harder about ways to address all >>> comments, not try to dismiss them. >> That's not what I tried to do. I carefully fixed all comments I saw in the >> review discussion and run tests. >> The only thing which wasn't addressed is removing an enum and replacing it >> with a comment. >> The hint that I try to manipulate maintainers is disrespectful. I assume >> that this isn't your intention, as we all just want to collaborate together >> here. No need to make this a personal discussion. >> >> If you think that replacing the enum with a comment is a blocker for v2 >> patch-series, I will go ahead and submit v3 with that change. > Yes IMHO it needs to be fixed but please go over the comments and try to > address them all as best you can, instead of looking for an explanation > why the comments were irrelevant and can be dismissed. I'm not trying to finding explanation on why the comments are irrelevant and can be dismissed... It's not my first time contributing code to QEMU/KVM... > Sure someone > might propose you introduce a bug, and that can't just be addressed, but > that's not the case here. Also please do not send multiple revisions of > a large patchset in a day. People need time for review. OK. I will make note of that for next time. I would have thought maintainers prefer to always have ability to pick up the latest version that is ready to avoid reviewing old code that was already discussed. Assuming all previous comments were addressed. Thanks, -Liran
On Tue, Mar 10, 2020 at 02:29:42PM -0700, Liran Alon wrote: > > On 10/03/2020 22:56, Michael S. Tsirkin wrote: > > On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: > > > On 10/03/2020 19:44, Michael S. Tsirkin wrote: > > > > On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: > > > > > Hi, > > > > > > > > > > This series aims to fix several bugs in VMPort and improve it by supporting > > > > > more VMPort commands and make command results more configurable to > > > > > user via QEMU command-line. > > > > > > > > > > This functionality was proven to be useful to run various VMware VMs > > > > > when attempting to run them as-is on top of QEMU/KVM. > > > > > > > > > > For more details, see commit messages. > > > > Well two versions in one day and some review comments weren't addressed. > > > There is a single review comment that wasn't addressed which is replacing an > > > enum with a comment. And I explicitly mentioned that it's because I want > > > additional opinion on this. > > > I don't see why such a small thing should block review for 15 patches... > > > All the rest of the comments (Which were great) have been addressed. Unless > > > I have mistakenly missed something, which please point it out if I did. > > OK I just took a quick peek, two things quickly jumped out at me. > Thanks for having a look. > > > > version property really should be a boolean and have some documentation > > saying what functionality enables. > I thought that having a version number approach is more generic and easy to > maintain going forward. > If I understand correctly, this is also the approach taken by qxl & qxl-vga. > > The more elaborate alternative could have been introducing compat_flags (As > PVSCSI does) but it seems like it will pollute the property space with a lot > of useless VMPort properties. > (E.g. x-read-eax-bug, x-no-report-unsupported-cmd, x-no-report-vmx-type and > etc.). > > What is the advantage of having a boolean such as "x-vmport-v2" instead of > having a single "version" property? It's not clear what should happen going forward. Let's say version is incremented again. This then becomes challenging for downstreams to backport. > Will it suffice if I would just add documentation above "version" property > on what is was the functionality in "version==1"? > (Though, it's just easy to scan the vmport.c code for if's involving > ">version"... "version" is more of an internal field for machine-type > compatibility and not really meant to be used by user) > > Which approach do you prefer? I just dislike versions, they are hard to maintain. Individual ones is cleanest imho. Self-documenting. But if not, I'd do something like "x-vmport-fixes" and set bits there for each bugfix. > > userspace properties should use the non-abbreviated > > vm-executable since vmx is easy to confuse with vm extensions. > I really wish you would reconsider this. VMX is a really common term in > VMware terminology. > It is found in binary names, ".vmx" file, ".vmx" file properties, VMware > Tools prints, open-vm-tools source code and etc. Well that at least is easy to google. .vmx <vmname>.vmx This is the primary configuration file, which stores settings chosen in the New Virtual Machine Wizard or virtual machine settings editor. If you created the virtual machine under an earlier version of VMware Workstation on a Linux host, this file may have a .cfg extension so .vmx as used here has nothing to do with VM Executable version or type. Looks like it's just a source of confusion on the vmware side too :) > > In contrast, even though I have dealt for many years with VMware > technologies, I have never known that VMX==vm-executable. Well you said that's what it stands for. I have no idea. From what you say now maybe vmx basically is being used as a prefix for all things vmware. In that case vmport-version and vmport-type or even vmware-version and vmware-type will do just as well. > I still think it will introduce much confusion. On the other hard, I don't > see much confusing with this use of VMX with Intel VT-x > because it is only used inside vmport.c and in vmport properties names. And > the properties names match the names of the guest > code that interface with vmport in open-vm-tools source code. > > If you still have a strong opinion on this, I will change it as you say in > v3... But please consider above arguments. I'm just saying don't use vmx. It's too late to try to give it a different meaning. Figure out what it's supposed to stand for and write it out in full. > > > > That's just a quick look. > > > > > > > > Some people do this, try to wear the maintainers out by sheer volume. > > > > It works sometimes but it's not a nice tactic. I personally think it's > > > > worth taking the time to think harder about ways to address all > > > > comments, not try to dismiss them. > > > That's not what I tried to do. I carefully fixed all comments I saw in the > > > review discussion and run tests. > > > The only thing which wasn't addressed is removing an enum and replacing it > > > with a comment. > > > The hint that I try to manipulate maintainers is disrespectful. I assume > > > that this isn't your intention, as we all just want to collaborate together > > > here. No need to make this a personal discussion. > > > > > > If you think that replacing the enum with a comment is a blocker for v2 > > > patch-series, I will go ahead and submit v3 with that change. > > Yes IMHO it needs to be fixed but please go over the comments and try to > > address them all as best you can, instead of looking for an explanation > > why the comments were irrelevant and can be dismissed. > > I'm not trying to finding explanation on why the comments are irrelevant and > can be dismissed... It's not my first time contributing code to QEMU/KVM... > > > Sure someone > > might propose you introduce a bug, and that can't just be addressed, but > > that's not the case here. Also please do not send multiple revisions of > > a large patchset in a day. People need time for review. > OK. I will make note of that for next time. > I would have thought maintainers prefer to always have ability to pick up > the latest version that is ready to avoid reviewing old code that was > already discussed. Assuming all previous comments were addressed. > > Thanks, > -Liran >
On 10/03/2020 23:44, Michael S. Tsirkin wrote: > On Tue, Mar 10, 2020 at 02:29:42PM -0700, Liran Alon wrote: >> On 10/03/2020 22:56, Michael S. Tsirkin wrote: >>> On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: >>>> On 10/03/2020 19:44, Michael S. Tsirkin wrote: >>>>> On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: >>>>>> Hi, >>>>>> >>>>>> This series aims to fix several bugs in VMPort and improve it by supporting >>>>>> more VMPort commands and make command results more configurable to >>>>>> user via QEMU command-line. >>>>>> >>>>>> This functionality was proven to be useful to run various VMware VMs >>>>>> when attempting to run them as-is on top of QEMU/KVM. >>>>>> >>>>>> For more details, see commit messages. >>>>> Well two versions in one day and some review comments weren't addressed. >>>> There is a single review comment that wasn't addressed which is replacing an >>>> enum with a comment. And I explicitly mentioned that it's because I want >>>> additional opinion on this. >>>> I don't see why such a small thing should block review for 15 patches... >>>> All the rest of the comments (Which were great) have been addressed. Unless >>>> I have mistakenly missed something, which please point it out if I did. >>> OK I just took a quick peek, two things quickly jumped out at me. >> Thanks for having a look. >>> version property really should be a boolean and have some documentation >>> saying what functionality enables. >> I thought that having a version number approach is more generic and easy to >> maintain going forward. >> If I understand correctly, this is also the approach taken by qxl & qxl-vga. >> >> The more elaborate alternative could have been introducing compat_flags (As >> PVSCSI does) but it seems like it will pollute the property space with a lot >> of useless VMPort properties. >> (E.g. x-read-eax-bug, x-no-report-unsupported-cmd, x-no-report-vmx-type and >> etc.). >> >> What is the advantage of having a boolean such as "x-vmport-v2" instead of >> having a single "version" property? > It's not clear what should happen going forward. Let's say version is > incremented again. This then becomes challenging for downstreams to > backport. As all conditions are in the form of "if (s->version > X)" then incrementing version from 1 to 2 doesn't break any condition of "if (s->version > 1)". What is the challenge of backporting I'm missing? > >> Will it suffice if I would just add documentation above "version" property >> on what is was the functionality in "version==1"? >> (Though, it's just easy to scan the vmport.c code for if's involving >> ">version"... "version" is more of an internal field for machine-type >> compatibility and not really meant to be used by user) >> >> Which approach do you prefer? > I just dislike versions, they are hard to maintain. > > Individual ones is cleanest imho. Self-documenting. I agree. That's the PVSCSI approach of compat_flags. Have many properties but each define bit in a compat_flags that specifies behavior. The disadvantage it have is that it over-complicates code and introduce many properties that will never be used as it's just for internal binding to machine-type. > But if not, I'd do something like "x-vmport-fixes" and > set bits there for each bugfix. Hmm this could a nice and simple approach. Will it be OK then in this case to define "x-vmport-fixes" value in hw_compat_4_2[] to a hard-coded value (e.g. "20") without directly encoding each individual bit via vmport.h constants? I will note though that it seems this "x-vmport-fixes" bitmap seems to be the first of it's kind. But I'm OK with this approach. > > >>> userspace properties should use the non-abbreviated >>> vm-executable since vmx is easy to confuse with vm extensions. >> I really wish you would reconsider this. VMX is a really common term in >> VMware terminology. >> It is found in binary names, ".vmx" file, ".vmx" file properties, VMware >> Tools prints, open-vm-tools source code and etc. > Well that at least is easy to google. > > .vmx > > <vmname>.vmx > > This is the primary configuration file, which stores settings > chosen in the New Virtual Machine Wizard or virtual machine settings > editor. If you created the virtual machine under an earlier version of > VMware Workstation on a Linux host, this file may have a .cfg extension > > so .vmx as used here has nothing to do with VM Executable version or > type. Looks like it's just a source of confusion on the vmware > side too :) Well, the ".vmx" file is the configuration file for the VM given to VMX. But I agree VMware terminology is weird. :) >> In contrast, even though I have dealt for many years with VMware >> technologies, I have never known that VMX==vm-executable. > Well you said that's what it stands for. I have no idea. From what you > say now maybe vmx basically is being used as a prefix for all things > vmware. No. It's just use to specify things related to VMX. i.e. The host VMM. > In that case vmport-version and vmport-type or even > vmware-version and vmware-type will do just as well. vmware-version is also confusing. As one could confuse it with the product version number. VMware called this field "vmx-version" and "vmx-type". I don't know if they have another field that maybe is called "vmware-version"... >> I still think it will introduce much confusion. On the other hard, I don't >> see much confusing with this use of VMX with Intel VT-x >> because it is only used inside vmport.c and in vmport properties names. And >> the properties names match the names of the guest >> code that interface with vmport in open-vm-tools source code. >> >> If you still have a strong opinion on this, I will change it as you say in >> v3... But please consider above arguments. > I'm just saying don't use vmx. It's too late to try to give > it a different meaning. We are giving it here the same meaning VMware gave it. In the context of VMware VMPort. > Figure out what it's supposed to > stand for and write it out in full. VMX stands for the host VMM. But I don't see why I need to be in the position explaining the reason behind VMware terminology. I'm just suggesting to use it as-is to avoid confusion. It seems you are still not convinced by above arguments, so I will change this in v3 to what you preferred "vm-exec-version" & "vm-exec-type". I think this is a mistake but you have the final call as the maintainer and I accept that. -Liran
On Tue, Mar 10, 2020 at 11:57:49PM +0200, Liran Alon wrote: > > On 10/03/2020 23:44, Michael S. Tsirkin wrote: > > On Tue, Mar 10, 2020 at 02:29:42PM -0700, Liran Alon wrote: > > > On 10/03/2020 22:56, Michael S. Tsirkin wrote: > > > > On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: > > > > > On 10/03/2020 19:44, Michael S. Tsirkin wrote: > > > > > > On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: > > > > > > > Hi, > > > > > > > > > > > > > > This series aims to fix several bugs in VMPort and improve it by supporting > > > > > > > more VMPort commands and make command results more configurable to > > > > > > > user via QEMU command-line. > > > > > > > > > > > > > > This functionality was proven to be useful to run various VMware VMs > > > > > > > when attempting to run them as-is on top of QEMU/KVM. > > > > > > > > > > > > > > For more details, see commit messages. > > > > > > Well two versions in one day and some review comments weren't addressed. > > > > > There is a single review comment that wasn't addressed which is replacing an > > > > > enum with a comment. And I explicitly mentioned that it's because I want > > > > > additional opinion on this. > > > > > I don't see why such a small thing should block review for 15 patches... > > > > > All the rest of the comments (Which were great) have been addressed. Unless > > > > > I have mistakenly missed something, which please point it out if I did. > > > > OK I just took a quick peek, two things quickly jumped out at me. > > > Thanks for having a look. > > > > version property really should be a boolean and have some documentation > > > > saying what functionality enables. > > > I thought that having a version number approach is more generic and easy to > > > maintain going forward. > > > If I understand correctly, this is also the approach taken by qxl & qxl-vga. > > > > > > The more elaborate alternative could have been introducing compat_flags (As > > > PVSCSI does) but it seems like it will pollute the property space with a lot > > > of useless VMPort properties. > > > (E.g. x-read-eax-bug, x-no-report-unsupported-cmd, x-no-report-vmx-type and > > > etc.). > > > > > > What is the advantage of having a boolean such as "x-vmport-v2" instead of > > > having a single "version" property? > > It's not clear what should happen going forward. Let's say version is > > incremented again. This then becomes challenging for downstreams to > > backport. > As all conditions are in the form of "if (s->version > X)" then incrementing > version from 1 to 2 doesn't break any condition of "if (s->version > 1)". > What is the challenge of backporting I'm missing? the challenge is figuring out which parts does version apply to. It might be easy if there's just code, harder if there's also data, etc. > > > > > Will it suffice if I would just add documentation above "version" property > > > on what is was the functionality in "version==1"? > > > (Though, it's just easy to scan the vmport.c code for if's involving > > > ">version"... "version" is more of an internal field for machine-type > > > compatibility and not really meant to be used by user) > > > > > > Which approach do you prefer? > > I just dislike versions, they are hard to maintain. > > > > Individual ones is cleanest imho. Self-documenting. > I agree. That's the PVSCSI approach of compat_flags. Have many properties > but each define bit in a compat_flags that specifies behavior. > The disadvantage it have is that it over-complicates code and introduce many > properties that will never be used as it's just for internal binding to > machine-type. > > But if not, I'd do something like "x-vmport-fixes" and > > set bits there for each bugfix. > Hmm this could a nice and simple approach. > Will it be OK then in this case to define "x-vmport-fixes" value in > hw_compat_4_2[] to a hard-coded value (e.g. "20") without directly encoding > each individual bit via vmport.h constants? Well how are you going to check a specific flag then? > I will note though that it seems this "x-vmport-fixes" bitmap seems to be > the first of it's kind. But I'm OK with this approach. > > > > > > > > userspace properties should use the non-abbreviated > > > > vm-executable since vmx is easy to confuse with vm extensions. > > > I really wish you would reconsider this. VMX is a really common term in > > > VMware terminology. > > > It is found in binary names, ".vmx" file, ".vmx" file properties, VMware > > > Tools prints, open-vm-tools source code and etc. > > Well that at least is easy to google. > > > > .vmx > > > > <vmname>.vmx > > > > This is the primary configuration file, which stores settings > > chosen in the New Virtual Machine Wizard or virtual machine settings > > editor. If you created the virtual machine under an earlier version of > > VMware Workstation on a Linux host, this file may have a .cfg extension > > > > so .vmx as used here has nothing to do with VM Executable version or > > type. Looks like it's just a source of confusion on the vmware > > side too :) > Well, the ".vmx" file is the configuration file for the VM given to VMX. But > I agree VMware terminology is weird. :) > > > In contrast, even though I have dealt for many years with VMware > > > technologies, I have never known that VMX==vm-executable. > > Well you said that's what it stands for. I have no idea. From what you > > say now maybe vmx basically is being used as a prefix for all things > > vmware. > No. It's just use to specify things related to VMX. i.e. The host VMM. > > In that case vmport-version and vmport-type or even > > vmware-version and vmware-type will do just as well. > vmware-version is also confusing. As one could confuse it with the product > version number. > VMware called this field "vmx-version" and "vmx-type". I don't know if they > have another field that maybe is called "vmware-version"... > > > I still think it will introduce much confusion. On the other hard, I don't > > > see much confusing with this use of VMX with Intel VT-x > > > because it is only used inside vmport.c and in vmport properties names. And > > > the properties names match the names of the guest > > > code that interface with vmport in open-vm-tools source code. > > > > > > If you still have a strong opinion on this, I will change it as you say in > > > v3... But please consider above arguments. > > I'm just saying don't use vmx. It's too late to try to give > > it a different meaning. > We are giving it here the same meaning VMware gave it. In the context of > VMware VMPort. > > Figure out what it's supposed to > > stand for and write it out in full. > VMX stands for the host VMM. But I don't see why I need to be in the > position explaining the reason behind VMware terminology. > I'm just suggesting to use it as-is to avoid confusion. > > It seems you are still not convinced by above arguments, so I will change > this in v3 to what you preferred "vm-exec-version" & "vm-exec-type". > I think this is a mistake but you have the final call as the maintainer and > I accept that. > > -Liran >
On 11/03/2020 0:00, Michael S. Tsirkin wrote: > On Tue, Mar 10, 2020 at 11:57:49PM +0200, Liran Alon wrote: >> On 10/03/2020 23:44, Michael S. Tsirkin wrote: >>> On Tue, Mar 10, 2020 at 02:29:42PM -0700, Liran Alon wrote: >>>> On 10/03/2020 22:56, Michael S. Tsirkin wrote: >>>>> On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: >>>>>> On 10/03/2020 19:44, Michael S. Tsirkin wrote: >>>>>>> On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> This series aims to fix several bugs in VMPort and improve it by supporting >>>>>>>> more VMPort commands and make command results more configurable to >>>>>>>> user via QEMU command-line. >>>>>>>> >>>>>>>> This functionality was proven to be useful to run various VMware VMs >>>>>>>> when attempting to run them as-is on top of QEMU/KVM. >>>>>>>> >>>>>>>> For more details, see commit messages. >>>>>>> Well two versions in one day and some review comments weren't addressed. >>>>>> There is a single review comment that wasn't addressed which is replacing an >>>>>> enum with a comment. And I explicitly mentioned that it's because I want >>>>>> additional opinion on this. >>>>>> I don't see why such a small thing should block review for 15 patches... >>>>>> All the rest of the comments (Which were great) have been addressed. Unless >>>>>> I have mistakenly missed something, which please point it out if I did. >>>>> OK I just took a quick peek, two things quickly jumped out at me. >>>> Thanks for having a look. >>>>> version property really should be a boolean and have some documentation >>>>> saying what functionality enables. >>>> I thought that having a version number approach is more generic and easy to >>>> maintain going forward. >>>> If I understand correctly, this is also the approach taken by qxl & qxl-vga. >>>> >>>> The more elaborate alternative could have been introducing compat_flags (As >>>> PVSCSI does) but it seems like it will pollute the property space with a lot >>>> of useless VMPort properties. >>>> (E.g. x-read-eax-bug, x-no-report-unsupported-cmd, x-no-report-vmx-type and >>>> etc.). >>>> >>>> What is the advantage of having a boolean such as "x-vmport-v2" instead of >>>> having a single "version" property? >>> It's not clear what should happen going forward. Let's say version is >>> incremented again. This then becomes challenging for downstreams to >>> backport. >> As all conditions are in the form of "if (s->version > X)" then incrementing >> version from 1 to 2 doesn't break any condition of "if (s->version > 1)". >> What is the challenge of backporting I'm missing? > the challenge is figuring out which parts does version apply to. > It might be easy if there's just code, harder if there's > also data, etc. You mean things such as the following? s->some_field = (s->version > X) ? A : B; True that it could be a bit more difficult to spot. >>>> Will it suffice if I would just add documentation above "version" property >>>> on what is was the functionality in "version==1"? >>>> (Though, it's just easy to scan the vmport.c code for if's involving >>>> ">version"... "version" is more of an internal field for machine-type >>>> compatibility and not really meant to be used by user) >>>> >>>> Which approach do you prefer? >>> I just dislike versions, they are hard to maintain. >>> >>> Individual ones is cleanest imho. Self-documenting. >> I agree. That's the PVSCSI approach of compat_flags. Have many properties >> but each define bit in a compat_flags that specifies behavior. >> The disadvantage it have is that it over-complicates code and introduce many >> properties that will never be used as it's just for internal binding to >> machine-type. >>> But if not, I'd do something like "x-vmport-fixes" and >>> set bits there for each bugfix. >> Hmm this could a nice and simple approach. >> Will it be OK then in this case to define "x-vmport-fixes" value in >> hw_compat_4_2[] to a hard-coded value (e.g. "20") without directly encoding >> each individual bit via vmport.h constants? > Well how are you going to check a specific flag then? In the code itself I will have constants of course. I meant just in hw_compat_4_2[] machine-type compat entry because the bitmask value there should be specified as a string value. > >> I will note though that it seems this "x-vmport-fixes" bitmap seems to be >> the first of it's kind. But I'm OK with this approach. So just to be clear before implementing your suggesting approach, this doesn't bother you right?
On Wed, Mar 11, 2020 at 12:19:59AM +0200, Liran Alon wrote: > > On 11/03/2020 0:00, Michael S. Tsirkin wrote: > > On Tue, Mar 10, 2020 at 11:57:49PM +0200, Liran Alon wrote: > > > On 10/03/2020 23:44, Michael S. Tsirkin wrote: > > > > On Tue, Mar 10, 2020 at 02:29:42PM -0700, Liran Alon wrote: > > > > > On 10/03/2020 22:56, Michael S. Tsirkin wrote: > > > > > > On Tue, Mar 10, 2020 at 08:09:09PM +0200, Liran Alon wrote: > > > > > > > On 10/03/2020 19:44, Michael S. Tsirkin wrote: > > > > > > > > On Tue, Mar 10, 2020 at 06:53:16PM +0200, Liran Alon wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > This series aims to fix several bugs in VMPort and improve it by supporting > > > > > > > > > more VMPort commands and make command results more configurable to > > > > > > > > > user via QEMU command-line. > > > > > > > > > > > > > > > > > > This functionality was proven to be useful to run various VMware VMs > > > > > > > > > when attempting to run them as-is on top of QEMU/KVM. > > > > > > > > > > > > > > > > > > For more details, see commit messages. > > > > > > > > Well two versions in one day and some review comments weren't addressed. > > > > > > > There is a single review comment that wasn't addressed which is replacing an > > > > > > > enum with a comment. And I explicitly mentioned that it's because I want > > > > > > > additional opinion on this. > > > > > > > I don't see why such a small thing should block review for 15 patches... > > > > > > > All the rest of the comments (Which were great) have been addressed. Unless > > > > > > > I have mistakenly missed something, which please point it out if I did. > > > > > > OK I just took a quick peek, two things quickly jumped out at me. > > > > > Thanks for having a look. > > > > > > version property really should be a boolean and have some documentation > > > > > > saying what functionality enables. > > > > > I thought that having a version number approach is more generic and easy to > > > > > maintain going forward. > > > > > If I understand correctly, this is also the approach taken by qxl & qxl-vga. > > > > > > > > > > The more elaborate alternative could have been introducing compat_flags (As > > > > > PVSCSI does) but it seems like it will pollute the property space with a lot > > > > > of useless VMPort properties. > > > > > (E.g. x-read-eax-bug, x-no-report-unsupported-cmd, x-no-report-vmx-type and > > > > > etc.). > > > > > > > > > > What is the advantage of having a boolean such as "x-vmport-v2" instead of > > > > > having a single "version" property? > > > > It's not clear what should happen going forward. Let's say version is > > > > incremented again. This then becomes challenging for downstreams to > > > > backport. > > > As all conditions are in the form of "if (s->version > X)" then incrementing > > > version from 1 to 2 doesn't break any condition of "if (s->version > 1)". > > > What is the challenge of backporting I'm missing? > > the challenge is figuring out which parts does version apply to. > > It might be easy if there's just code, harder if there's > > also data, etc. > You mean things such as the following? > s->some_field = (s->version > X) ? A : B; > > True that it could be a bit more difficult to spot. > > > > > > Will it suffice if I would just add documentation above "version" property > > > > > on what is was the functionality in "version==1"? > > > > > (Though, it's just easy to scan the vmport.c code for if's involving > > > > > ">version"... "version" is more of an internal field for machine-type > > > > > compatibility and not really meant to be used by user) > > > > > > > > > > Which approach do you prefer? > > > > I just dislike versions, they are hard to maintain. > > > > > > > > Individual ones is cleanest imho. Self-documenting. > > > I agree. That's the PVSCSI approach of compat_flags. Have many properties > > > but each define bit in a compat_flags that specifies behavior. > > > The disadvantage it have is that it over-complicates code and introduce many > > > properties that will never be used as it's just for internal binding to > > > machine-type. > > > > But if not, I'd do something like "x-vmport-fixes" and > > > > set bits there for each bugfix. > > > Hmm this could a nice and simple approach. > > > Will it be OK then in this case to define "x-vmport-fixes" value in > > > hw_compat_4_2[] to a hard-coded value (e.g. "20") without directly encoding > > > each individual bit via vmport.h constants? > > Well how are you going to check a specific flag then? > In the code itself I will have constants of course. > I meant just in hw_compat_4_2[] machine-type compat entry because the > bitmask value there should be specified as a string value. > > > > > I will note though that it seems this "x-vmport-fixes" bitmap seems to be > > > the first of it's kind. But I'm OK with this approach. > So just to be clear before implementing your suggesting approach, this > doesn't bother you right? So it would be like this: { "x-vmport-fixes" : "0x7" /* VMPORT_FOO | VMPORT_BAR */ } I prefer DEFINE_PROP_BIT myself. But this version is not too terrible I think.