Re: [Qemu-devel] [PATCH 0/3] Audio: misc fixes for "Audio 20190821 patches", part two
Hi -- not sure what's happened with this patchset, but can we at least get the first patch (which fixes some coverity errors) in, even if the other two need more consideration? (cc'ing Gerd as audio maintainer.) thanks -- PMM On Wed, 11 Sep 2019 at 00:27, Kővágó, Zoltán wrote: > > Hi, > > This series contains some random fixes for the "Audio 20190821 patches": > a coverity bugfix and pulseaudio connection/stream names fix. > > Since there wasn't a clear consensus about naming the pa streams, I've > split it into two commits: the first time just uses the audiodev id > unconditionally, while the last commit adds a new option to qapi to > override it. This way we can easily drop the last commit if it turns > out to be unnecessary. > > Regards, > Zoltan > > > Kővágó, Zoltán (3): > audio: fix parameter dereference before NULL check > audio: paaudio: fix connection and stream name > audio: paaudio: ability to specify stream name > > audio/audio_template.h | 7 +-- > audio/paaudio.c| 9 ++--- > qapi/audio.json| 6 ++ > 3 files changed, 17 insertions(+), 5 deletions(-)
Re: [Qemu-devel] [PATCH 0/3] migration/postcopy: unsentmap is not necessary
* Wei Yang (richardw.y...@linux.intel.com) wrote: > Three patches to cleanup postcopy: > > [1]: split canonicalize bitmap and discard page > [2]: remove unsentmap since it is not necessary > [3]: cleanup the get_queued_page_not_dirty Queued > > Wei Yang (3): > migration/postcopy: not necessary to do discard when canonicalizing > bitmap > migration/postcopy: unsentmap is not necessary for postcopy > migration: remove sent parameter in get_queued_page_not_dirty > > include/exec/ram_addr.h | 6 --- > migration/ram.c | 94 +++-- > migration/trace-events | 2 +- > 3 files changed, 16 insertions(+), 86 deletions(-) > > -- > 2.17.1 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] migration/postcopy: unsentmap is not necessary
Hi, Dave and Juan Would you mind taking a look at this one? On Mon, Aug 19, 2019 at 02:18:40PM +0800, Wei Yang wrote: >Three patches to cleanup postcopy: > >[1]: split canonicalize bitmap and discard page >[2]: remove unsentmap since it is not necessary >[3]: cleanup the get_queued_page_not_dirty > >Wei Yang (3): > migration/postcopy: not necessary to do discard when canonicalizing >bitmap > migration/postcopy: unsentmap is not necessary for postcopy > migration: remove sent parameter in get_queued_page_not_dirty > > include/exec/ram_addr.h | 6 --- > migration/ram.c | 94 +++-- > migration/trace-events | 2 +- > 3 files changed, 16 insertions(+), 86 deletions(-) > >-- >2.17.1 -- Wei Yang Help you, Help me
Re: [Qemu-devel] [PATCH 0/3] proper locking on bitmap add/remove paths
12.09.2019 0:46, John Snow wrote:
>
>
> On 9/11/19 10:09 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 10.09.2019 23:37, no-re...@patchew.org wrote:
>>> Patchew URL:
>>> https://patchew.org/QEMU/20190910162724.79574-1-vsement...@virtuozzo.com/
>>>
>>>
>>>
>>> Hi,
>>>
>>> This series failed the docker-quick@centos7 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
>>> make docker-image-centos7 V=1 NETWORK=1
>>> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
>
> (Was patchew even using clang?)
hmm gcc 9.2 don't show such error but clang does.
>
>>> === TEST SCRIPT END ===
>>>
>>> libudev no
>>> default devices yes
>>>
>>> warning: Python 2 support is deprecated
>>> warning: Python 3 will be required for building future versions of QEMU
>>>
>>> NOTE: cross-compilers enabled: 'cc'
>>> GEN x86_64-softmmu/config-devices.mak.tmp
>>> ---
>>> CC block/qed-cluster.o
>>> CC block/qed-check.o
>>> /tmp/qemu-test/src/block/qcow2-bitmap.c: In function
>>> 'qcow2_co_remove_persistent_dirty_bitmap':
>>> /tmp/qemu-test/src/block/qcow2-bitmap.c:502:8: error: 'bm' may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>if (bm == NULL) {
>>> ^
>>> /tmp/qemu-test/src/block/qcow2-bitmap.c:1413:18: note: 'bm' was declared
>>> here
>>>
>>>
>>> The full log is available at
>>> http://patchew.org/logs/20190910162724.79574-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
>>> ---
>>> Email generated automatically by Patchew [https://patchew.org/].
>>> Please send your feedback to patchew-de...@redhat.com
>>>
>>
>> Who knows, how to clang Qemu?
>>
>> I try with
>> ./configure --target-list=x86_64-softmmu --enable-debug --disable-virtfs
>> --enable-werror --audio-drv-list=oss --extra-cflags=-Wall
>> --enable-sanitizers --cc=clang --cxx=clang++
>> make -j9
>>
>
> ../../configure --target-list="x86_64-softmmu" --cc=clang --cxx=clang++
> --host-cc=clang
>
> works OK for me in fedora 30.
So, now I've moved to fedora from rhel-based virtuozzo, and it just works :)
>
> --target-list="x86_64-softmmu" --cc=clang --cxx=clang++ --host-cc=clang
> --enable-werror --extra-cflags=-Wall
>
> Seems OK too, and finally adding
>
> --enable-sanitizers
>
> also appears to work alright.
>
>
> clang version 8.0.0 (Fedora 8.0.0-1.fc30)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /usr/bin
>
>
> Maybe something bad in your ccache or some intermediate state in your
> build dir. I use separate build directories for gcc and clang just in case.
>
Thanks for help!
--
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH 0/3] proper locking on bitmap add/remove paths
On 9/11/19 10:09 AM, Vladimir Sementsov-Ogievskiy wrote:
> 10.09.2019 23:37, no-re...@patchew.org wrote:
>> Patchew URL:
>> https://patchew.org/QEMU/20190910162724.79574-1-vsement...@virtuozzo.com/
>>
>>
>>
>> Hi,
>>
>> This series failed the docker-quick@centos7 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
>> make docker-image-centos7 V=1 NETWORK=1
>> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
(Was patchew even using clang?)
>> === TEST SCRIPT END ===
>>
>> libudev no
>> default devices yes
>>
>> warning: Python 2 support is deprecated
>> warning: Python 3 will be required for building future versions of QEMU
>>
>> NOTE: cross-compilers enabled: 'cc'
>>GEN x86_64-softmmu/config-devices.mak.tmp
>> ---
>>CC block/qed-cluster.o
>>CC block/qed-check.o
>> /tmp/qemu-test/src/block/qcow2-bitmap.c: In function
>> 'qcow2_co_remove_persistent_dirty_bitmap':
>> /tmp/qemu-test/src/block/qcow2-bitmap.c:502:8: error: 'bm' may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>> if (bm == NULL) {
>> ^
>> /tmp/qemu-test/src/block/qcow2-bitmap.c:1413:18: note: 'bm' was declared here
>>
>>
>> The full log is available at
>> http://patchew.org/logs/20190910162724.79574-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
>> ---
>> Email generated automatically by Patchew [https://patchew.org/].
>> Please send your feedback to patchew-de...@redhat.com
>>
>
> Who knows, how to clang Qemu?
>
> I try with
> ./configure --target-list=x86_64-softmmu --enable-debug --disable-virtfs
> --enable-werror --audio-drv-list=oss --extra-cflags=-Wall --enable-sanitizers
> --cc=clang --cxx=clang++
> make -j9
>
../../configure --target-list="x86_64-softmmu" --cc=clang --cxx=clang++
--host-cc=clang
works OK for me in fedora 30.
--target-list="x86_64-softmmu" --cc=clang --cxx=clang++ --host-cc=clang
--enable-werror --extra-cflags=-Wall
Seems OK too, and finally adding
--enable-sanitizers
also appears to work alright.
clang version 8.0.0 (Fedora 8.0.0-1.fc30)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Maybe something bad in your ccache or some intermediate state in your
build dir. I use separate build directories for gcc and clang just in case.
--js
Re: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock
Patchew URL: https://patchew.org/QEMU/20190911164202.31136-1-dgilb...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock Message-id: 20190911164202.31136-1-dgilb...@redhat.com Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' bf33be9 migration: Use automatic rcu_read unlock in rdma.c c64f2f4 migration: Use automatic rcu_read unlock in ram.c af6a608 rcu: Add automatically released rcu_read_lock variant === OUTPUT BEGIN === 1/3 Checking commit af6a608b908d (rcu: Add automatically released rcu_read_lock variant) ERROR: Macros with multiple statements should be enclosed in a do - while loop #33: FILE: include/qemu/rcu.h:165: +#define RCU_READ_LOCK_AUTO g_auto(rcu_read_auto_t) \ +_rcu_read_auto = 'x'; \ +rcu_read_lock(); total: 1 errors, 0 warnings, 18 lines checked Patch 1/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/3 Checking commit c64f2f4c923f (migration: Use automatic rcu_read unlock in ram.c) 3/3 Checking commit bf33be959c2b (migration: Use automatic rcu_read unlock in rdma.c) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190911164202.31136-1-dgilb...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Wed, Sep 11, 2019 at 05:41:59PM +0100, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" > > > > This patch uses glib's g_auto mechanism to automatically free > > rcu_read_lock's at the end of the block. Given that humans > > have a habit of forgetting an error path somewhere it's > > best to leave it to the compiler. > > > > In particular: > > https://bugzilla.redhat.com/show_bug.cgi?id=1746787 > > suggests we've forgotten an unlock case somewhere in the > > rdma migration code. > > Probably worth mentioning this in the commit message of the 3rd patch > so someone reading history sees that the patch wasn't just a no-op > conversion, but instead actively fixing a bug. Added. Dave > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock
On Wed, Sep 11, 2019 at 05:41:59PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > This patch uses glib's g_auto mechanism to automatically free > rcu_read_lock's at the end of the block. Given that humans > have a habit of forgetting an error path somewhere it's > best to leave it to the compiler. > > In particular: > https://bugzilla.redhat.com/show_bug.cgi?id=1746787 > suggests we've forgotten an unlock case somewhere in the > rdma migration code. Probably worth mentioning this in the commit message of the 3rd patch so someone reading history sees that the patch wasn't just a no-op conversion, but instead actively fixing a bug. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] proper locking on bitmap add/remove paths
10.09.2019 23:37, no-re...@patchew.org wrote:
> Patchew URL:
> https://patchew.org/QEMU/20190910162724.79574-1-vsement...@virtuozzo.com/
>
>
>
> Hi,
>
> This series failed the docker-quick@centos7 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
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
>
> libudev no
> default devices yes
>
> warning: Python 2 support is deprecated
> warning: Python 3 will be required for building future versions of QEMU
>
> NOTE: cross-compilers enabled: 'cc'
>GEN x86_64-softmmu/config-devices.mak.tmp
> ---
>CC block/qed-cluster.o
>CC block/qed-check.o
> /tmp/qemu-test/src/block/qcow2-bitmap.c: In function
> 'qcow2_co_remove_persistent_dirty_bitmap':
> /tmp/qemu-test/src/block/qcow2-bitmap.c:502:8: error: 'bm' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> if (bm == NULL) {
> ^
> /tmp/qemu-test/src/block/qcow2-bitmap.c:1413:18: note: 'bm' was declared here
>
>
> The full log is available at
> http://patchew.org/logs/20190910162724.79574-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-de...@redhat.com
>
Who knows, how to clang Qemu?
I try with
./configure --target-list=x86_64-softmmu --enable-debug --disable-virtfs
--enable-werror --audio-drv-list=oss --extra-cflags=-Wall --enable-sanitizers
--cc=clang --cxx=clang++
make -j9
but get the following error:
/tmp/linuxboot_dma-6712c0.s: Assembler messages:
/tmp/linuxboot_dma-6712c0.s:473: Error: inconsistent uses of .cfi_sections
clang: error: assembler command failed with exit code 1 (use -v to see
invocation)
make[1]: *** [linuxboot_dma.o] Error 1
make[1]: *** Waiting for unfinished jobs
SIGNpc-bios/optionrom/linuxboot.bin
LINKivshmem-client
/tmp/pvh_main-3bbc75.s: Assembler messages:
/tmp/pvh_main-3bbc75.s:651: Error: inconsistent uses of .cfi_sections
clang: error: assembler command failed with exit code 1 (use -v to see
invocation)
make[1]: *** [pvh_main.o] Error 1
make: *** [pc-bios/optionrom/all] Error 2
make: *** Waiting for unfinished jobs
make: *** wait: No child processes. Stop.
--
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH 0/3] cputlb: Adjust tlb bswap implementation
Patchew URL: https://patchew.org/QEMU/20190911014353.5926-1-richard.hender...@linaro.org/ Hi, This series failed the docker-quick@centos7 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 make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === libudev no default devices yes warning: Python 2 support is deprecated warning: Python 3 will be required for building future versions of QEMU NOTE: cross-compilers enabled: 'cc' GEN x86_64-softmmu/config-devices.mak.tmp --- Memory content inconsistency at 401aa000 first_byte = e4 last_byte = e4 current = e5 hit_edge = 0 TESTcheck-qtest-x86_64: tests/ahci-test and in another 25173 pages** ERROR:/tmp/qemu-test/src/tests/migration-test.c:342:check_guests_ram: assertion failed: (bad == 0) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/migration-test.c:342:check_guests_ram: assertion failed: (bad == 0) make: *** [check-qtest-aarch64] Error 1 make: *** Waiting for unfinished jobs TESTcheck-unit: tests/test-aio-multithread --- TESTiotest-qcow2: 252 Passed all 106 tests ** ERROR:/tmp/qemu-test/src/tests/boot-sector.c:161:boot_sector_test: assertion failed (signature == SIGNATURE): (0x == 0xdead) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/boot-sector.c:161:boot_sector_test: assertion failed (signature == SIGNATURE): (0x == 0xdead) make: *** [check-qtest-x86_64] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 659, in The full log is available at http://patchew.org/logs/20190911014353.5926-1-richard.hender...@linaro.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] cputlb: Adjust tlb bswap implementation
Patchew URL: https://patchew.org/QEMU/20190911014353.5926-1-richard.hender...@linaro.org/ Hi, This series failed the docker-quick@centos7 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 make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === libudev no default devices yes warning: Python 2 support is deprecated warning: Python 3 will be required for building future versions of QEMU NOTE: cross-compilers enabled: 'cc' GEN x86_64-softmmu/config-devices.mak.tmp --- TESTcheck-qtest-x86_64: tests/ahci-test TESTcheck-unit: tests/test-aio-multithread and in another 25208 pages** ERROR:/tmp/qemu-test/src/tests/migration-test.c:342:check_guests_ram: assertion failed: (bad == 0) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/migration-test.c:342:check_guests_ram: assertion failed: (bad == 0) make: *** [check-qtest-aarch64] Error 1 make: *** Waiting for unfinished jobs TESTcheck-unit: tests/test-throttle --- TESTiotest-qcow2: 252 Passed all 106 tests ** ERROR:/tmp/qemu-test/src/tests/boot-sector.c:161:boot_sector_test: assertion failed (signature == SIGNATURE): (0x == 0xdead) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/boot-sector.c:161:boot_sector_test: assertion failed (signature == SIGNATURE): (0x == 0xdead) make: *** [check-qtest-x86_64] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 659, in The full log is available at http://patchew.org/logs/20190911014353.5926-1-richard.hender...@linaro.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] target/mips: Convert to do_transaction_failed hook
02.08.2019. 18.29, "Philippe Mathieu-Daudé" је написао/ла: > > Cc'ing broader MIPS audience. > > On 8/2/19 6:04 PM, Peter Maydell wrote: > > This patchset converts the MIPS target away from the > > old broken do_unassigned_access hook to the new (added in > > 2017...) do_transaction_failed hook. > > Since Herve now tested this series, unless somebody objects, I am going to include it in the next Mips queue, scheduled in next few days. Thanks to all involved! Aleksandar > > The motivation here is: > > * do_unassigned_access is broken because: > > + it will be called for any kind of access to physical addresses > > where there is no assigned device, whether that access is by the > > CPU or by something else (like a DMA controller!), so it can > > result in spurious guest CPU exceptions. > > + It will also get called even when using KVM, when there's nothing > > useful it can do. > > + It isn't passed in the return-address within the TCG generated > > code, so it isn't able to correctly restore the CPU state > > before generating the exception, and so the exception will > > often be generated with the wrong faulting guest PC value > > * there are now only a few targets still using the old hook, > >so if we can convert them we can delete all the old code > >and complete this API transation. (Patches for SPARC are on > >the list; the other user is RISCV, which accidentally > >implemented the old hook rather than the new one recently.) > > > > The general approach to the conversion is to check the target for > > load/store-by-physical-address operations which were previously > > implicitly causing exceptions, to see if they now need to explicitly > > check for and handle memory access failures. (The 'git grep' regexes > > in docs/devel/loads-stores.rst are useful here: the API families to > > look for are ld*_phys/st*_phys, address_space_ld/st*, and > > cpu_physical_memory*.) > > > > For MIPS, there are none of these (the usual place where targets do > > this is hardware page table walks where the page table entries are > > loaded by physical address, and MIPS doesn't seem to have those). > > > > Code audit out of the way, the actual hook changeover is pretty > > simple. > > > > The complication here is the MIPS Jazz board, which has some rather > > dubious code that intercepts the do_unassigned_access hook to suppress > > generation of exceptions for invalid accesses due to data accesses, > > while leaving exceptions for invalid instruction fetches in place. I'm > > a bit dubious about whether the behaviour we have implemented here is > > really what the hardware does -- it seems pretty odd to me to not > > generate exceptions for d-side accesses but to generate them for > > i-side accesses, and looking back through git and mailing list history > > this code is here mainly as "put back the behaviour we had before a > > previous commit broke it", and that older behaviour in turn I think is > > more historical-accident than because anybody deliberately checked the > > hardware behaviour and made QEMU work that way. However, I don't have > > any real hardware to do comparative tests on, so this series retains > > the same behaviour we had before on this board, by making it intercept > > the new hook in the same way it did the old one. I've beefed up the > > comment somewhat to indicate what we're doing, why, and why it might > > not be right. > > > > The patch series is structured in three parts: > > * make the Jazz board code support CPUs regardless of which > >of the two hooks they implement > > * switch the MIPS CPUs over to implementing the new hook > > * remove the no-longer-needed Jazz board code for the old > >hook > > (This seemed cleaner to me than squashing the whole thing into > > a single patch that touched core mips code and the jazz board > > at the same time.) > > > > I have tested this with: > > * the ARC Multiboot BIOS linked to from the bug > >https://bugs.launchpad.net/qemu/+bug/1245924 (and which > >was the test case for needing the hook intercept) > > * a Linux kernel for the 'mips' mips r4k machine > > * 'make check' > > Obviously more extensive testing would be useful, but I > > don't have any other test images. I also don't have > > a KVM MIPS host, which would be worth testing to confirm > > that it also still works. > > > > If anybody happens by some chance to still have a working > > real-hardware Magnum or PICA61 board, we could perhaps test > > how it handles accesses to invalid memory, but I suspect that > > nobody does any more :-) > > > > thanks > > -- PMM > > > > > > Peter Maydell (3): > > hw/mips/mips_jazz: Override do_transaction_failed hook > > target/mips: Switch to do_transaction_failed() hook > > hw/mips/mips_jazz: Remove no-longer-necessary override of > > do_unassigned_access > > > > target/mips/internal.h | 8 --- > > hw/mips/mips_jazz.c | 47
Re: [Qemu-devel] [PATCH 0/3] cputlb: Adjust tlb bswap implementation
Patchew URL: https://patchew.org/QEMU/20190911014353.5926-1-richard.hender...@linaro.org/ Hi, This series failed the docker-quick@centos7 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 make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === libudev no default devices yes warning: Python 2 support is deprecated warning: Python 3 will be required for building future versions of QEMU NOTE: cross-compilers enabled: 'cc' GEN aarch64-softmmu/config-devices.mak.tmp --- Memory content inconsistency at 4018f000 first_byte = e4 last_byte = e4 current = e5 hit_edge = 0 Memory content inconsistency at 4019 first_byte = e4 last_byte = e4 current = e5 hit_edge = 0 and in another 25199 pages** ERROR:/tmp/qemu-test/src/tests/migration-test.c:342:check_guests_ram: assertion failed: (bad == 0) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/migration-test.c:342:check_guests_ram: assertion failed: (bad == 0) make: *** [check-qtest-aarch64] Error 1 make: *** Waiting for unfinished jobs TESTcheck-unit: tests/test-throttle --- TESTiotest-qcow2: 252 Passed all 106 tests ** ERROR:/tmp/qemu-test/src/tests/boot-sector.c:161:boot_sector_test: assertion failed (signature == SIGNATURE): (0x == 0xdead) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/boot-sector.c:161:boot_sector_test: assertion failed (signature == SIGNATURE): (0x == 0xdead) make: *** [check-qtest-x86_64] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 659, in The full log is available at http://patchew.org/logs/20190911014353.5926-1-richard.hender...@linaro.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] some fix in tests/migration
Patchew URL:
https://patchew.org/QEMU/20190910120927.1669283-1-maozhon...@cmss.chinamobile.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 0/3] some fix in tests/migration
Message-id: 20190910120927.1669283-1-maozhon...@cmss.chinamobile.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
1a1d7b1 tests/migration：fix unreachable path in stress test
5a19b9e tests/migration: fix a typo in comment
3b1a8b7 tests/migration: mem leak fix
=== OUTPUT BEGIN ===
1/3 Checking commit 3b1a8b7a617e (tests/migration: mem leak fix)
ERROR: braces {} are necessary for all arms of this statement
#22: FILE: tests/migration/stress.c:184:
+if (data)
[...]
total: 1 errors, 0 warnings, 8 lines checked
Patch 1/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit 5a19b9eb57d9 (tests/migration: fix a typo in comment)
3/3 Checking commit 1a1d7b1097b4 (tests/migration：fix unreachable path in
stress test)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190910120927.1669283-1-maozhon...@cmss.chinamobile.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] proper locking on bitmap add/remove paths
Patchew URL:
https://patchew.org/QEMU/20190910162724.79574-1-vsement...@virtuozzo.com/
Hi,
This series failed the docker-quick@centos7 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
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
libudev no
default devices yes
warning: Python 2 support is deprecated
warning: Python 3 will be required for building future versions of QEMU
NOTE: cross-compilers enabled: 'cc'
GEN x86_64-softmmu/config-devices.mak.tmp
---
CC block/qed-cluster.o
CC block/qed-check.o
/tmp/qemu-test/src/block/qcow2-bitmap.c: In function
'qcow2_co_remove_persistent_dirty_bitmap':
/tmp/qemu-test/src/block/qcow2-bitmap.c:502:8: error: 'bm' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
if (bm == NULL) {
^
/tmp/qemu-test/src/block/qcow2-bitmap.c:1413:18: note: 'bm' was declared here
The full log is available at
http://patchew.org/logs/20190910162724.79574-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] some fix in tests/migration
Patchew URL:
https://patchew.org/QEMU/20190910120927.1669283-1-maozhon...@cmss.chinamobile.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 0/3] some fix in tests/migration
Message-id: 20190910120927.1669283-1-maozhon...@cmss.chinamobile.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
e4c5de2 tests/migration：fix unreachable path in stress test
9a3b77f tests/migration: fix a typo in comment
b277d56 tests/migration: mem leak fix
=== OUTPUT BEGIN ===
1/3 Checking commit b277d56926b6 (tests/migration: mem leak fix)
ERROR: braces {} are necessary for all arms of this statement
#22: FILE: tests/migration/stress.c:184:
+if (data)
[...]
total: 1 errors, 0 warnings, 8 lines checked
Patch 1/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit 9a3b77f10015 (tests/migration: fix a typo in comment)
3/3 Checking commit e4c5de2759b8 (tests/migration：fix unreachable path in
stress test)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190910120927.1669283-1-maozhon...@cmss.chinamobile.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] target/mips: Convert to do_transaction_failed hook
ping for Hervé. 22.08.2019. 20.16, "Aleksandar Markovic" је написао/ла: > > > 02.08.2019. 18.05, "Peter Maydell" је написао/ла: > > > > This patchset converts the MIPS target away from the > > old broken do_unassigned_access hook to the new (added in > > 2017...) do_transaction_failed hook. > > > > Herve, bonjour. > > As far as I can see these changes are fine. May I ask you for your opinion? Can you run your Jazz tests without regressions with this change? > > Mille mercis, > Aleksandar > > > The motivation here is: > > * do_unassigned_access is broken because: > > + it will be called for any kind of access to physical addresses > > where there is no assigned device, whether that access is by the > > CPU or by something else (like a DMA controller!), so it can > > result in spurious guest CPU exceptions. > > + It will also get called even when using KVM, when there's nothing > > useful it can do. > > + It isn't passed in the return-address within the TCG generated > > code, so it isn't able to correctly restore the CPU state > > before generating the exception, and so the exception will > > often be generated with the wrong faulting guest PC value > > * there are now only a few targets still using the old hook, > >so if we can convert them we can delete all the old code > >and complete this API transation. (Patches for SPARC are on > >the list; the other user is RISCV, which accidentally > >implemented the old hook rather than the new one recently.) > > > > The general approach to the conversion is to check the target for > > load/store-by-physical-address operations which were previously > > implicitly causing exceptions, to see if they now need to explicitly > > check for and handle memory access failures. (The 'git grep' regexes > > in docs/devel/loads-stores.rst are useful here: the API families to > > look for are ld*_phys/st*_phys, address_space_ld/st*, and > > cpu_physical_memory*.) > > > > For MIPS, there are none of these (the usual place where targets do > > this is hardware page table walks where the page table entries are > > loaded by physical address, and MIPS doesn't seem to have those). > > > > Code audit out of the way, the actual hook changeover is pretty > > simple. > > > > The complication here is the MIPS Jazz board, which has some rather > > dubious code that intercepts the do_unassigned_access hook to suppress > > generation of exceptions for invalid accesses due to data accesses, > > while leaving exceptions for invalid instruction fetches in place. I'm > > a bit dubious about whether the behaviour we have implemented here is > > really what the hardware does -- it seems pretty odd to me to not > > generate exceptions for d-side accesses but to generate them for > > i-side accesses, and looking back through git and mailing list history > > this code is here mainly as "put back the behaviour we had before a > > previous commit broke it", and that older behaviour in turn I think is > > more historical-accident than because anybody deliberately checked the > > hardware behaviour and made QEMU work that way. However, I don't have > > any real hardware to do comparative tests on, so this series retains > > the same behaviour we had before on this board, by making it intercept > > the new hook in the same way it did the old one. I've beefed up the > > comment somewhat to indicate what we're doing, why, and why it might > > not be right. > > > > The patch series is structured in three parts: > > * make the Jazz board code support CPUs regardless of which > >of the two hooks they implement > > * switch the MIPS CPUs over to implementing the new hook > > * remove the no-longer-needed Jazz board code for the old > >hook > > (This seemed cleaner to me than squashing the whole thing into > > a single patch that touched core mips code and the jazz board > > at the same time.) > > > > I have tested this with: > > * the ARC Multiboot BIOS linked to from the bug > >https://bugs.launchpad.net/qemu/+bug/1245924 (and which > >was the test case for needing the hook intercept) > > * a Linux kernel for the 'mips' mips r4k machine > > * 'make check' > > Obviously more extensive testing would be useful, but I > > don't have any other test images. I also don't have > > a KVM MIPS host, which would be worth testing to confirm > > that it also still works. > > > > If anybody happens by some chance to still have a working > > real-hardware Magnum or PICA61 board, we could perhaps test > > how it handles accesses to invalid memory, but I suspect that > > nobody does any more :-) > > > > thanks > > -- PMM > > > > > > Peter Maydell (3): > > hw/mips/mips_jazz: Override do_transaction_failed hook > > target/mips: Switch to do_transaction_failed() hook > > hw/mips/mips_jazz: Remove no-longer-necessary override of > > do_unassigned_access > > > > target/mips/internal.h | 8 --- > > hw/mips/mips_jazz.c
Re: [Qemu-devel] [PATCH 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335
On Fri, 2019-09-06 at 20:31 +0300, Maxim Levitsky wrote: > Commit 8ac0f15f335 accidently broke the COW of non changed areas > of newly allocated clusters, when the write spans multiple clusters, > and needs COW both prior and after the write. > This results in 'after' COW area beeing encrypted with wrong > sector address, which render it corrupted. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1745922 > > CC: qemu-stable I forgot to mention, huge thanks to Kevin Wolf, for helping me to define the proper fix for this bug. Best regards, Maxim Levitsky
Re: [Qemu-devel] [PATCH 0/3] UUID validation during migration
On Tue, Sep 03, 2019 at 12:21:40PM +0100, Dr. David Alan Gilbert wrote: > * Yury Kotov (yury-ko...@yandex-team.ru) wrote: > > Hi, > > > > This series adds an UUID validation at the start of the migration > > on the target side. The idea is to identify the source of migration. > > > > Possible case of problem: > > 1. There are 3 servers: A, B and C > > 2. Server A has a VM 1, server B has a VM 2 > > 3. VM 1 and VM 2 want to migrate to the server C > > 4. Target of VM 1 starts on the server C and dies too quickly for some > > reason > > 5. Target of VM 2 starts just after that and listen the same tcp port X, > > which > >the target of VM 1 wanted to use > > 6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source > > That shouldn't be possible in practice; you specify the destination tcp > port when you start the destination qemu; so unless the management code > that starts the migration is very broken it should know which port it's > migrating to. > However, if it is very broken then this is a good check. In some, but not all, cases allowing the wrong source QEMU to connect to a target QEMU could be considered a serious security flaw. Historically live migration security was pretty minimal, to non-existant, but we do now have the ability todo much better with TLS. With the way libvirt currently uses TLS for migration, we're just protecting against a rogue host connecting, as any host with a valid cert gets allowed. We could do better by using the new ACLs feature to whitelist just the particular virt host that we know the source VM is on. This still allows for an accident if libvirt is migrating 2 VMs on the same source host at the same time. What's needed is a unique identity for each individual migration operation. The use of the UUID here is aiming to serve that role. Assuming libvirt has protected its TLS certificates on the source host, then this solution is secure. An attacker would need to become root on the source host to access libvirt's TLS certs, at which point no other strategy libvirt used instead of UUID would be secure either. So I think from a security POV, the use of UUID is acceptable. An alternative would be to not use TLS certificates, and instead use the TLS pre-shared keys credential type, generating a new set of PSK creds for each migration operation. In this case, UUID would not be required at all. I don't see this as a reason to reject the UUID check though. It is reasonable for mgmt apps to have a choice in which approach they pick. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] UUID validation during migration
* Yury Kotov (yury-ko...@yandex-team.ru) wrote: > Hi, > > This series adds an UUID validation at the start of the migration > on the target side. The idea is to identify the source of migration. > > Possible case of problem: > 1. There are 3 servers: A, B and C > 2. Server A has a VM 1, server B has a VM 2 > 3. VM 1 and VM 2 want to migrate to the server C > 4. Target of VM 1 starts on the server C and dies too quickly for some reason > 5. Target of VM 2 starts just after that and listen the same tcp port X, which >the target of VM 1 wanted to use > 6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source That shouldn't be possible in practice; you specify the destination tcp port when you start the destination qemu; so unless the management code that starts the migration is very broken it should know which port it's migrating to. However, if it is very broken then this is a good check. Dave > 7. It's possible that migration might be successful (e.g., devices are the > same) > 8. So, the target of VM 2 is in undefined state > > The series adds a capability to prevent successful (by mistake) migration. > > The new capability x-validate-uuid only affects the source so that it sends > its UUID to the target. The target will validate the received UUID and stop > the migration if UUIDs are not equal. > > Regards, > Yury > > Yury Kotov (3): > migration: Add x-validate-uuid capability > tests/libqtest: Allow to set expected exit status > tests/migration: Add a test for x-validate-uuid capability > > migration/migration.c | 9 +++ > migration/migration.h | 1 + > migration/savevm.c | 45 + > qapi/migration.json| 5 +- > tests/libqtest.c | 14 - > tests/libqtest.h | 9 +++ > tests/migration-test.c | 140 - > 7 files changed, 189 insertions(+), 34 deletions(-) > > -- > 2.22.0 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] Acceptance test x86_cpu_model_versions: fixes and minor improvements
On Wed, Aug 28, 2019 at 03:36:25PM -0400, Cleber Rosa wrote: > This corrects some of the messages given on failures (aligning them to > the test code), and splits a larger test into smaller and more > manageable pieces. Thanks! I'm queueing this on python-next. -- Eduardo
Re: [Qemu-devel] [PATCH 0/3] Acceptance test x86_cpu_model_versions: fixes and minor improvements
Patchew URL: https://patchew.org/QEMU/20190828193628.7687-1-cr...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 0/3] Acceptance test x86_cpu_model_versions: fixes
and minor improvements
Message-id: 20190828193628.7687-1-cr...@redhat.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
2a57147 Acceptance test x86_cpu_model_versions: split into smaller tests
30c6782 Acceptance test x86_cpu_model_versions: fix mismatches between test and
messages
f63b60f Acceptance test x86_cpu_model_versions: shutdown VMs
=== OUTPUT BEGIN ===
1/3 Checking commit f63b60f49e7a (Acceptance test x86_cpu_model_versions:
shutdown VMs)
2/3 Checking commit 30c678277001 (Acceptance test x86_cpu_model_versions: fix
mismatches between test and messages)
ERROR: line over 90 characters
#22: FILE: tests/acceptance/x86_cpu_model_versions.py:283:
+ 'pc-i440fx-4.0 + Cascadelake-Server-v1 should not
have arch-capabilities')
ERROR: line over 90 characters
#31: FILE: tests/acceptance/x86_cpu_model_versions.py:292:
+ 'pc-i440fx-4.0 + Cascadelake-Server-v2 should have
arch-capabilities')
ERROR: line over 90 characters
#39: FILE: tests/acceptance/x86_cpu_model_versions.py:299:
+vm.add_args('-cpu',
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
total: 3 errors, 0 warnings, 24 lines checked
Patch 2/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/3 Checking commit 2a57147a862b (Acceptance test x86_cpu_model_versions: split
into smaller tests)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190828193628.7687-1-cr...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] mailmap: Clean up
Aleksandar Markovic writes: > 23.08.2019. 08.13, "Markus Armbruster" је написао/ла: >> >> Philippe Mathieu-Daudé writes: >> >> > Trivial cleanup of .mailmap to have a nice 'git shortlog' output. >> > >> > Philippe Mathieu-Daudé (3): >> > mailmap: Reorder by sections >> > mailmap: Update philmd email address >> > mailmap: Add many entries to improve 'git shortlog' statistics >> > >> > .mailmap | 123 +++ >> > 1 file changed, 115 insertions(+), 8 deletions(-) >> >> Series >> Reviewed-by: Markus Armbruster >> >> However, it increases the difference to contrib/gitdm/aliases. > > Alex' initial gitdm effort, as I understood it, was not meant to cover all > history from 2007 or so, but just to give reasonable statistics for 2018 > (amd future years). > > In that light, .mailmap and gitdm aliases do not need to be equivalent. > > But perhaps Alex would now want gitdm to be used for all QEMU history? Is > this desirable? It would be of interest historically but not something I'd want to spend a lot of time adding code churn for. > > Aleksandar > >> I'm just >> as guilty; my recent "[PATCH 2/2] contrib/gitdm: Add arm...@pond.sub.org >> to group-map-redhat" updates only that. and not .mailmap. >> >> Perhaps we want to keep the two in sync manually. We should then add >> suitable comments to each file. >> >> Could we instead teach gitdm to use .mailmap, and ditch >> contrib/gitdm/aliases? >> >> aliases' format is documented in gitdm's README. Each line maps a >> non-canonical e-mail address to a canonical one. >> >> .mailmap's format is documented in git-shortlog(1). It can do a bit >> more. Even the common part differs: it has two addresses in different >> order *boggle*. >> -- Alex Bennée
Re: [Qemu-devel] [PATCH 0/3] mailmap: Clean up
23.08.2019. 08.13, "Markus Armbruster" је написао/ла: > > Philippe Mathieu-Daudé writes: > > > Trivial cleanup of .mailmap to have a nice 'git shortlog' output. > > > > Philippe Mathieu-Daudé (3): > > mailmap: Reorder by sections > > mailmap: Update philmd email address > > mailmap: Add many entries to improve 'git shortlog' statistics > > > > .mailmap | 123 +++ > > 1 file changed, 115 insertions(+), 8 deletions(-) > > Series > Reviewed-by: Markus Armbruster > > However, it increases the difference to contrib/gitdm/aliases. Alex' initial gitdm effort, as I understood it, was not meant to cover all history from 2007 or so, but just to give reasonable statistics for 2018 (amd future years). In that light, .mailmap and gitdm aliases do not need to be equivalent. But perhaps Alex would now want gitdm to be used for all QEMU history? Is this desirable? Aleksandar > I'm just > as guilty; my recent "[PATCH 2/2] contrib/gitdm: Add arm...@pond.sub.org > to group-map-redhat" updates only that. and not .mailmap. > > Perhaps we want to keep the two in sync manually. We should then add > suitable comments to each file. > > Could we instead teach gitdm to use .mailmap, and ditch > contrib/gitdm/aliases? > > aliases' format is documented in gitdm's README. Each line maps a > non-canonical e-mail address to a canonical one. > > .mailmap's format is documented in git-shortlog(1). It can do a bit > more. Even the common part differs: it has two addresses in different > order *boggle*. >
Re: [Qemu-devel] [PATCH 0/3] mailmap: Clean up
Philippe Mathieu-Daudé writes: > Trivial cleanup of .mailmap to have a nice 'git shortlog' output. > > Philippe Mathieu-Daudé (3): > mailmap: Reorder by sections > mailmap: Update philmd email address > mailmap: Add many entries to improve 'git shortlog' statistics > > .mailmap | 123 +++ > 1 file changed, 115 insertions(+), 8 deletions(-) Series Reviewed-by: Markus Armbruster However, it increases the difference to contrib/gitdm/aliases. I'm just as guilty; my recent "[PATCH 2/2] contrib/gitdm: Add arm...@pond.sub.org to group-map-redhat" updates only that. and not .mailmap. Perhaps we want to keep the two in sync manually. We should then add suitable comments to each file. Could we instead teach gitdm to use .mailmap, and ditch contrib/gitdm/aliases? aliases' format is documented in gitdm's README. Each line maps a non-canonical e-mail address to a canonical one. .mailmap's format is documented in git-shortlog(1). It can do a bit more. Even the common part differs: it has two addresses in different order *boggle*.
Re: [Qemu-devel] [PATCH 0/3] target/mips: Convert to do_transaction_failed hook
02.08.2019. 18.05, "Peter Maydell" је написао/ла: > > This patchset converts the MIPS target away from the > old broken do_unassigned_access hook to the new (added in > 2017...) do_transaction_failed hook. > Herve, bonjour. As far as I can see these changes are fine. May I ask you for your opinion? Can you run your Jazz tests without regressions with this change? Mille mercis, Aleksandar > The motivation here is: > * do_unassigned_access is broken because: > + it will be called for any kind of access to physical addresses > where there is no assigned device, whether that access is by the > CPU or by something else (like a DMA controller!), so it can > result in spurious guest CPU exceptions. > + It will also get called even when using KVM, when there's nothing > useful it can do. > + It isn't passed in the return-address within the TCG generated > code, so it isn't able to correctly restore the CPU state > before generating the exception, and so the exception will > often be generated with the wrong faulting guest PC value > * there are now only a few targets still using the old hook, >so if we can convert them we can delete all the old code >and complete this API transation. (Patches for SPARC are on >the list; the other user is RISCV, which accidentally >implemented the old hook rather than the new one recently.) > > The general approach to the conversion is to check the target for > load/store-by-physical-address operations which were previously > implicitly causing exceptions, to see if they now need to explicitly > check for and handle memory access failures. (The 'git grep' regexes > in docs/devel/loads-stores.rst are useful here: the API families to > look for are ld*_phys/st*_phys, address_space_ld/st*, and > cpu_physical_memory*.) > > For MIPS, there are none of these (the usual place where targets do > this is hardware page table walks where the page table entries are > loaded by physical address, and MIPS doesn't seem to have those). > > Code audit out of the way, the actual hook changeover is pretty > simple. > > The complication here is the MIPS Jazz board, which has some rather > dubious code that intercepts the do_unassigned_access hook to suppress > generation of exceptions for invalid accesses due to data accesses, > while leaving exceptions for invalid instruction fetches in place. I'm > a bit dubious about whether the behaviour we have implemented here is > really what the hardware does -- it seems pretty odd to me to not > generate exceptions for d-side accesses but to generate them for > i-side accesses, and looking back through git and mailing list history > this code is here mainly as "put back the behaviour we had before a > previous commit broke it", and that older behaviour in turn I think is > more historical-accident than because anybody deliberately checked the > hardware behaviour and made QEMU work that way. However, I don't have > any real hardware to do comparative tests on, so this series retains > the same behaviour we had before on this board, by making it intercept > the new hook in the same way it did the old one. I've beefed up the > comment somewhat to indicate what we're doing, why, and why it might > not be right. > > The patch series is structured in three parts: > * make the Jazz board code support CPUs regardless of which >of the two hooks they implement > * switch the MIPS CPUs over to implementing the new hook > * remove the no-longer-needed Jazz board code for the old >hook > (This seemed cleaner to me than squashing the whole thing into > a single patch that touched core mips code and the jazz board > at the same time.) > > I have tested this with: > * the ARC Multiboot BIOS linked to from the bug >https://bugs.launchpad.net/qemu/+bug/1245924 (and which >was the test case for needing the hook intercept) > * a Linux kernel for the 'mips' mips r4k machine > * 'make check' > Obviously more extensive testing would be useful, but I > don't have any other test images. I also don't have > a KVM MIPS host, which would be worth testing to confirm > that it also still works. > > If anybody happens by some chance to still have a working > real-hardware Magnum or PICA61 board, we could perhaps test > how it handles accesses to invalid memory, but I suspect that > nobody does any more :-) > > thanks > -- PMM > > > Peter Maydell (3): > hw/mips/mips_jazz: Override do_transaction_failed hook > target/mips: Switch to do_transaction_failed() hook > hw/mips/mips_jazz: Remove no-longer-necessary override of > do_unassigned_access > > target/mips/internal.h | 8 --- > hw/mips/mips_jazz.c | 47 + > target/mips/cpu.c | 2 +- > target/mips/op_helper.c | 24 +++-- > 4 files changed, 47 insertions(+), 34 deletions(-) > > -- > 2.20.1 > >
Re: [Qemu-devel] [PATCH 0/3] pc: Fix die-id validation and compatibility with libvirt
On Thu, Aug 15, 2019 at 03:38:00PM -0300, Eduardo Habkost wrote: > Currently, if die-id is omitted on -device for CPUs, we get a > very confusing error message: > > $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ > -device qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0 > qemu-system-x86_64: -device > qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0: \ > Invalid CPU die-id: 4294967295 must be in range 0:5 > > This has 3 problems > > 1) The actual range for die-id is 0:0. >This is fixed by patch 1/3. > 2) The user didn't specify die-id=4294967295. >This is fixed by patch 2/3. > 3) It breaks compatibility with libvirt because die-id was not >mandatory before. >This is addressed by patch 3/3. > > Issues #1 and #2 were reported at: > https://bugzilla.redhat.com/show_bug.cgi?id=1741151 > > Issue #3 was reported at: > https://bugzilla.redhat.com/show_bug.cgi?id=1741451 > > Cc: Like Xu > Cc: Peter Krempa > Cc: Igor Mammedov Looks good Reviewed-by: Michael S. Tsirkin I'm traveling and can't test this properly. Anyone else can merge this? Eduardo? > Eduardo Habkost (3): > pc: Fix error message on die-id validation > pc: Improve error message when die-id is omitted > pc: Don't make CPU properties mandatory unless necessary > > hw/i386/pc.c | 23 - > tests/acceptance/pc_cpu_hotplug_props.py | 59 > 2 files changed, 81 insertions(+), 1 deletion(-) > create mode 100644 tests/acceptance/pc_cpu_hotplug_props.py > > -- > 2.21.0
Re: [Qemu-devel] [PATCH 0/3] block: Make various formats' block_status recurse again
On 25.07.19 17:55, Max Reitz wrote: > Hi, > > 69f47505ee66afaa513305de0c1895a224e52c45 changed block_status so that it > would only go down to the protocol layer if the format layer returned > BDRV_BLOCK_RECURSE, thus indicating that it has no sufficient > information whether a given range in the image is zero or not. > Generally, this is because the image is preallocated and thus all ranges > appear as zeroes. > > However, it only implemented this preallocation detection for qcow2. > There are more formats that support preallocation, though: vdi, vhdx, > vmdk, vpc. (Funny how they all start with “v”.) > > For vdi, vmdk, and vpc, the fix is rather simple, because they really > have different subformats depending on whether an image is preallocated > or not. This makes the check very simple. > > vhdx is more like qcow2, where after the image has been created, it > isn’t clear whether it’s been preallocated or everything is allocated > because everything was already written to. 69f47505ee added a heuristic > to qcow2 to get around this, but I think that’s too much for vhdx. I > just left it unfixed, because I don’t care that much, honestly (and I > don’t think anyone else does). > > > Max Reitz (3): > vdi: Make block_status recurse for fixed images > vmdk: Make block_status recurse for flat extents > vpc: Do not return RAW from block_status > > block/vdi.c | 3 ++- > block/vmdk.c | 3 +++ > block/vpc.c | 2 +- > 3 files changed, 6 insertions(+), 2 deletions(-) Thanks for the reviews, applied to my block-next branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block-next Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/3] colo: Add support for continious replication
Hi Lukas, Please fix this issue and add more comments in the commit log. Thanks Zhang Chen > -Original Message- > From: no-re...@patchew.org [mailto:no-re...@patchew.org] > Sent: Thursday, August 15, 2019 11:20 AM > To: lukasstra...@web.de > Cc: Zhang, Chen ; qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH 0/3] colo: Add support for continious > replication > > Patchew URL: > https://patchew.org/QEMU/cover.1565814686.git.lukasstra...@web.de/ > > > > Hi, > > This series failed build test on s390x host. Please find the details below. > > === TEST SCRIPT BEGIN === > #!/bin/bash > # Testing script will be invoked under the git checkout with # HEAD pointing > to > a commit that has the patches applied on top of "base" > # branch > set -e > > echo > echo "=== ENV ===" > env > > echo > echo "=== PACKAGES ===" > rpm -qa > > echo > echo "=== UNAME ===" > uname -a > > CC=$HOME/bin/cc > INSTALL=$PWD/install > BUILD=$PWD/build > mkdir -p $BUILD $INSTALL > SRC=$PWD > cd $BUILD > $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable > clean up # make check -j4 V=1 make install === TEST SCRIPT END === > > from /var/tmp/patchew-tester-tmp- > 6ji6qfi2/src/include/net/filter.h:13, > from > /var/tmp/patchew-tester-tmp-6ji6qfi2/src/net/filter.c:14: > /var/tmp/patchew-tester-tmp-6ji6qfi2/src/net/filter.c: In function > ‘netfilter_complete’: > /var/tmp/patchew-tester-tmp-6ji6qfi2/src/include/qemu/queue.h:412:44: error: > ‘position’ may be used uninitialized in this function [-Werror=maybe- > uninitialized] > 412 | (listelm)->field.tqe_circ.tql_prev = &(elm)->field.tqe_circ; >\ > |^ > /var/tmp/patchew-tester-tmp-6ji6qfi2/src/net/filter.c:237:21: note: ‘position’ > was declared here > > > The full log is available at > http://patchew.org/logs/cover.1565814686.git.lukasstra...@web.de/testing.s3 > 90x/?type=message. > --- > Email generated automatically by Patchew [https://patchew.org/]. > Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] colo: Add support for continious replication
Patchew URL: https://patchew.org/QEMU/cover.1565814686.git.lukasstra...@web.de/ Hi, This series failed build test on s390x host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo echo "=== ENV ===" env echo echo "=== PACKAGES ===" rpm -qa echo echo "=== UNAME ===" uname -a CC=$HOME/bin/cc INSTALL=$PWD/install BUILD=$PWD/build mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable clean up # make check -j4 V=1 make install === TEST SCRIPT END === from /var/tmp/patchew-tester-tmp-6ji6qfi2/src/include/net/filter.h:13, from /var/tmp/patchew-tester-tmp-6ji6qfi2/src/net/filter.c:14: /var/tmp/patchew-tester-tmp-6ji6qfi2/src/net/filter.c: In function ‘netfilter_complete’: /var/tmp/patchew-tester-tmp-6ji6qfi2/src/include/qemu/queue.h:412:44: error: ‘position’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 412 | (listelm)->field.tqe_circ.tql_prev = &(elm)->field.tqe_circ; \ |^ /var/tmp/patchew-tester-tmp-6ji6qfi2/src/net/filter.c:237:21: note: ‘position’ was declared here The full log is available at http://patchew.org/logs/cover.1565814686.git.lukasstra...@web.de/testing.s390x/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] Reduce the number of Valgrind reports in unit tests.
On 13/08/2019 15:05, Paolo Bonzini wrote: > On 13/08/19 14:02, Andrey Shinkevich wrote: >> PINGING... > > I thought I had already said I queued the series? > > Paolo > Thank you Paolo. I am clear now. Andrey >> On 30/07/2019 19:01, Andrey Shinkevich wrote: >>> Running unit tests under the Valgrind may help to detect QEMU memory issues >>> (suggested by Denis V. Lunev). Some of the Valgrind reports relate to the >>> unit test code itself. Let's eliminate the detected memory issues to ease >>> locating critical ones. >>> >>> Andrey Shinkevich (3): >>> test-throttle: Fix uninitialized use of burst_length >>> tests: Fix uninitialized byte in test_visitor_in_fuzz >>> i386/kvm: initialize struct at full before ioctl call >>> >>>target/i386/kvm.c | 3 +++ >>>tests/test-string-input-visitor.c | 8 +++- >>>tests/test-throttle.c | 2 ++ >>>3 files changed, 8 insertions(+), 5 deletions(-) >>> >> > -- With the best regards, Andrey Shinkevich
Re: [Qemu-devel] [PATCH 0/3] Reduce the number of Valgrind reports in unit tests.
On 13/08/19 14:02, Andrey Shinkevich wrote: > PINGING... I thought I had already said I queued the series? Paolo > On 30/07/2019 19:01, Andrey Shinkevich wrote: >> Running unit tests under the Valgrind may help to detect QEMU memory issues >> (suggested by Denis V. Lunev). Some of the Valgrind reports relate to the >> unit test code itself. Let's eliminate the detected memory issues to ease >> locating critical ones. >> >> Andrey Shinkevich (3): >>test-throttle: Fix uninitialized use of burst_length >>tests: Fix uninitialized byte in test_visitor_in_fuzz >>i386/kvm: initialize struct at full before ioctl call >> >> target/i386/kvm.c | 3 +++ >> tests/test-string-input-visitor.c | 8 +++- >> tests/test-throttle.c | 2 ++ >> 3 files changed, 8 insertions(+), 5 deletions(-) >> >
Re: [Qemu-devel] [PATCH 0/3] Reduce the number of Valgrind reports in unit tests.
PINGING... On 30/07/2019 19:01, Andrey Shinkevich wrote: > Running unit tests under the Valgrind may help to detect QEMU memory issues > (suggested by Denis V. Lunev). Some of the Valgrind reports relate to the > unit test code itself. Let's eliminate the detected memory issues to ease > locating critical ones. > > Andrey Shinkevich (3): >test-throttle: Fix uninitialized use of burst_length >tests: Fix uninitialized byte in test_visitor_in_fuzz >i386/kvm: initialize struct at full before ioctl call > > target/i386/kvm.c | 3 +++ > tests/test-string-input-visitor.c | 8 +++- > tests/test-throttle.c | 2 ++ > 3 files changed, 8 insertions(+), 5 deletions(-) > -- With the best regards, Andrey Shinkevich
Re: [Qemu-devel] [PATCH 0/3] backup fixes for 4.1?
On 7/31/19 6:29 AM, Vladimir Sementsov-Ogievskiy wrote: > 30.07.2019 21:41, John Snow wrote: >> >> >> On 7/30/19 12:32 PM, Vladimir Sementsov-Ogievskiy wrote: >>> Hi all! >>> >>> Here are two small fixes. >>> >>> 01 is not a degradation at all, so it's OK for 4.2 >>> 02 is degradation of 3.0, so it's possibly OK for 4.2 too, >>> but it seems to be real bug and fix is very simple, so, >>> may be 4.1 is better >>> >>> Or you may take the whole series to 4.1 if you want. >>> >> >> I think (1) and (2) can go in for stable after review, but they're not >> crucial for 4.1 especially at this late of a stage. Should be cataclysms >> only right now. You found a cataclysm :( >> >> --js >> > > I can rebase it than on your bitmaps branch. Or, if we want it for stable, > maybe, > I shouldn't? > I rebased these two patches (1 and 3) on top of bitmaps, on top of rc4. Thanks, applied to my bitmaps tree: https://github.com/jnsnow/qemu/commits/bitmaps https://github.com/jnsnow/qemu.git --js
Re: [Qemu-devel] [PATCH 0/3] migration/savevm: move non SaveStateEntry condition check out of iteration
* Wei Yang (richardw.y...@linux.intel.com) wrote: > qemu_savevm_state_complete_precopy() iterates SaveStateEntry and does proper > tasks for migration. > > For each iteration, in_postcopy and iterable_only would be checked to see > whether it should skip. Since these two conditions are not SaveStateEntry > specific, it is proper to move the check out of iteration. > > These three patches prepare the code and move the condition check out of the > iteration. > Queued > > Wei Yang (3): > migration/savevm: flush file for iterable_only case > migration/savevm: split qemu_savevm_state_complete_precopy() into two > parts > migration/savevm: move non SaveStateEntry condition check out of > iteration > > migration/savevm.c | 68 ++ > 1 file changed, 50 insertions(+), 18 deletions(-) > > -- > 2.17.1 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] migration/postcopy: cleanup function postcopy_send_discard_bm_ram
* Wei Yang (richardw.y...@linux.intel.com) wrote: > Some cleanup of function postcopy_send_discard_bm_ram: > > * use a more restrict check for discard page > * break the loop when no more page to discard > * it is for sure discard_length is not 0 > > No functional change. > Queued > Wei Yang (3): > migration/postcopy: the valid condition is one less then end > migration/postcopy: break the loop when there is no more page to > discard > migration/postcopy: discard_length must not be 0 > > migration/ram.c | 24 +++- > 1 file changed, 11 insertions(+), 13 deletions(-) > > -- > 2.19.1 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
On 07/08/19 18:38, Alex Bennée wrote: >>> Rather, we use '#!/usr/bin/env bash' to find bash anywhere. >> Nevermind - this script is pure Bourne shell. The only fix needed >> should be > I thought we wanted to use pure POSIX shell and not rely on bash-ism's > creeping in? > There are uses of bash as long as it's marked as such (and not /bin/sh). But Bourne shell = /bin/sh, bash is the "Bourne-again" shell. :) Paolo
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
Paolo Bonzini writes:
> On 07/08/19 15:33, Eric Blake wrote:
>> On 8/7/19 8:06 AM, Paolo Bonzini wrote:
>>> On 07/08/19 14:40, Alex Bennée wrote:
Paolo Bonzini writes:
> The tests/tcg rely a lot on per-target informations from
> the QEMU makefiles, but most of the definitions in there
> aren't really relevant to TCG tests.
>
> This series is just a cleanup, but it could also be
> a useful start in making it possible to compile tests/tcg
> out of QEMU's tree, and/or making it a submodule, and/or
> unifying the system emulation tests with kvm-unit-tests.
Hmm something is throwing off configure and making it use my login shell
instead of /bin/sh:
libpmem support no
libudev yes
default devices yes
~/lsrc/qemu.git/tests/tcg/configure.sh (line 63): 'case' builtin not
inside of switch block
case $arch in
^
fish: Error while reading file
/home/alex/lsrc/qemu.git/tests/tcg/configure.sh
>>>
>>> It's the ${SHELL} you found in patch 3. The disadvantage of relying on
>>> #! is that some people have bash in /usr/bin/bash rather than /bin/bash.
>>> But we already assume /bin/bash elsewhere so I can drop it.
>>
>> Rather, we use '#!/usr/bin/env bash' to find bash anywhere.
>
> Nevermind - this script is pure Bourne shell. The only fix needed
> should be
I thought we wanted to use pure POSIX shell and not rely on bash-ism's
creeping in?
>
> diff --git a/configure b/configure
> index eeeda8760a..f216f3f9d9 100755
> --- a/configure
> +++ b/configure
> @@ -6468,12 +6468,6 @@ if ! $python -c 'import sys; sys.exit(sys.version_info
> < (3,0))'; then
>echo "warning: Python 3 will be required for building future versions of
> QEMU" >&2
> fi
>
> -(for i in $cross_cc_vars; do
> - export $i
> -done
> -export target_list source_path
> -${SHELL-/bin/sh} $source_path/tests/tcg/configure.sh)
> -
> config_host_mak="config-host.mak"
>
> echo "# Automatically generated by configure - do not modify"
> >config-all-disas.mak
> @@ -7844,6 +7838,12 @@ for f in $LINKS ; do
> fi
> done
>
> +(for i in $cross_cc_vars; do
> + export $i
> +done
> +export target_list source_path
> +$source_path/tests/tcg/configure.sh)
> +
I also had chmod +x the script.
> # temporary config to build submodules
> for rom in seabios vgabios ; do
> config_mak=roms/$rom/config.mak
--
Alex Bennée
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
On 07/08/19 15:33, Eric Blake wrote:
> On 8/7/19 8:06 AM, Paolo Bonzini wrote:
>> On 07/08/19 14:40, Alex Bennée wrote:
>>>
>>> Paolo Bonzini writes:
>>>
The tests/tcg rely a lot on per-target informations from
the QEMU makefiles, but most of the definitions in there
aren't really relevant to TCG tests.
This series is just a cleanup, but it could also be
a useful start in making it possible to compile tests/tcg
out of QEMU's tree, and/or making it a submodule, and/or
unifying the system emulation tests with kvm-unit-tests.
>>>
>>> Hmm something is throwing off configure and making it use my login shell
>>> instead of /bin/sh:
>>>
>>> libpmem support no
>>> libudev yes
>>> default devices yes
>>> ~/lsrc/qemu.git/tests/tcg/configure.sh (line 63): 'case' builtin not
>>> inside of switch block
>>> case $arch in
>>> ^
>>>fish: Error while reading file
>>> /home/alex/lsrc/qemu.git/tests/tcg/configure.sh
>>
>> It's the ${SHELL} you found in patch 3. The disadvantage of relying on
>> #! is that some people have bash in /usr/bin/bash rather than /bin/bash.
>> But we already assume /bin/bash elsewhere so I can drop it.
>
> Rather, we use '#!/usr/bin/env bash' to find bash anywhere.
Nevermind - this script is pure Bourne shell. The only fix needed should be
diff --git a/configure b/configure
index eeeda8760a..f216f3f9d9 100755
--- a/configure
+++ b/configure
@@ -6468,12 +6468,6 @@ if ! $python -c 'import sys; sys.exit(sys.version_info <
(3,0))'; then
echo "warning: Python 3 will be required for building future versions of
QEMU" >&2
fi
-(for i in $cross_cc_vars; do
- export $i
-done
-export target_list source_path
-${SHELL-/bin/sh} $source_path/tests/tcg/configure.sh)
-
config_host_mak="config-host.mak"
echo "# Automatically generated by configure - do not modify"
>config-all-disas.mak
@@ -7844,6 +7838,12 @@ for f in $LINKS ; do
fi
done
+(for i in $cross_cc_vars; do
+ export $i
+done
+export target_list source_path
+$source_path/tests/tcg/configure.sh)
+
# temporary config to build submodules
for rom in seabios vgabios ; do
config_mak=roms/$rom/config.mak
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
On 8/7/19 8:06 AM, Paolo Bonzini wrote:
> On 07/08/19 14:40, Alex Bennée wrote:
>>
>> Paolo Bonzini writes:
>>
>>> The tests/tcg rely a lot on per-target informations from
>>> the QEMU makefiles, but most of the definitions in there
>>> aren't really relevant to TCG tests.
>>>
>>> This series is just a cleanup, but it could also be
>>> a useful start in making it possible to compile tests/tcg
>>> out of QEMU's tree, and/or making it a submodule, and/or
>>> unifying the system emulation tests with kvm-unit-tests.
>>
>> Hmm something is throwing off configure and making it use my login shell
>> instead of /bin/sh:
>>
>> libpmem support no
>> libudev yes
>> default devices yes
>> ~/lsrc/qemu.git/tests/tcg/configure.sh (line 63): 'case' builtin not
>> inside of switch block
>> case $arch in
>> ^
>>fish: Error while reading file
>> /home/alex/lsrc/qemu.git/tests/tcg/configure.sh
>
> It's the ${SHELL} you found in patch 3. The disadvantage of relying on
> #! is that some people have bash in /usr/bin/bash rather than /bin/bash.
> But we already assume /bin/bash elsewhere so I can drop it.
Rather, we use '#!/usr/bin/env bash' to find bash anywhere.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
On 07/08/19 14:40, Alex Bennée wrote:
>
> Paolo Bonzini writes:
>
>> The tests/tcg rely a lot on per-target informations from
>> the QEMU makefiles, but most of the definitions in there
>> aren't really relevant to TCG tests.
>>
>> This series is just a cleanup, but it could also be
>> a useful start in making it possible to compile tests/tcg
>> out of QEMU's tree, and/or making it a submodule, and/or
>> unifying the system emulation tests with kvm-unit-tests.
>
> Hmm something is throwing off configure and making it use my login shell
> instead of /bin/sh:
>
> libpmem support no
> libudev yes
> default devices yes
> ~/lsrc/qemu.git/tests/tcg/configure.sh (line 63): 'case' builtin not inside
> of switch block
> case $arch in
> ^
>fish: Error while reading file
> /home/alex/lsrc/qemu.git/tests/tcg/configure.sh
It's the ${SHELL} you found in patch 3. The disadvantage of relying on
#! is that some people have bash in /usr/bin/bash rather than /bin/bash.
But we already assume /bin/bash elsewhere so I can drop it.
Paolo
>>
>> Paolo
>>
>> Paolo Bonzini (3):
>> tests/tcg: use EXTRA_CFLAGS everywhere
>> tests/tcg: cleanup Makefile inclusions
>> tests/tcg: move configuration to a sub-shell script
>>
>> Makefile | 1 +
>> Makefile.target | 3 -
>> configure | 155 ++--
>> tests/Makefile.include| 25 ++--
>> tests/tcg/Makefile.include| 88
>> tests/tcg/Makefile.prereqs| 18 +++
>> tests/tcg/Makefile.probe | 31
>> tests/tcg/Makefile.qemu | 95 +
>> tests/tcg/{Makefile => Makefile.target} | 15 +-
>> tests/tcg/aarch64/Makefile.include| 8 --
>> tests/tcg/aarch64/Makefile.softmmu-target | 4 +-
>> tests/tcg/aarch64/Makefile.target | 12 +-
>> tests/tcg/alpha/Makefile.include | 2 -
>> tests/tcg/alpha/Makefile.softmmu-target | 4 +-
>> tests/tcg/arm/Makefile.include| 8 --
>> tests/tcg/arm/Makefile.softmmu-target | 6 +-
>> tests/tcg/configure.sh| 228
>> ++
>> tests/tcg/cris/Makefile.include | 6 -
>> tests/tcg/hppa/Makefile.include | 2 -
>> tests/tcg/i386/Makefile.include | 9 --
>> tests/tcg/i386/Makefile.softmmu-target| 12 +-
>> tests/tcg/i386/Makefile.target| 13 +-
>> tests/tcg/m68k/Makefile.include | 2 -
>> tests/tcg/minilib/Makefile.target | 2 +-
>> tests/tcg/mips/Makefile.include | 20 ---
>> tests/tcg/ppc/Makefile.include| 10 --
>> tests/tcg/riscv/Makefile.include | 10 --
>> tests/tcg/s390x/Makefile.include | 2 -
>> tests/tcg/sh4/Makefile.include| 4 -
>> tests/tcg/sparc64/Makefile.include| 2 -
>> tests/tcg/x86_64/Makefile.softmmu-target | 36 +
>> tests/tcg/x86_64/Makefile.target | 7 +-
>> tests/tcg/xtensa/Makefile.include | 11 --
>> tests/tcg/xtensa/Makefile.softmmu-target | 4 +-
>> 34 files changed, 435 insertions(+), 420 deletions(-)
>> delete mode 100644 tests/tcg/Makefile.include
>> create mode 100644 tests/tcg/Makefile.prereqs
>> delete mode 100644 tests/tcg/Makefile.probe
>> create mode 100644 tests/tcg/Makefile.qemu
>> rename tests/tcg/{Makefile => Makefile.target} (90%)
>> delete mode 100644 tests/tcg/aarch64/Makefile.include
>> delete mode 100644 tests/tcg/alpha/Makefile.include
>> delete mode 100644 tests/tcg/arm/Makefile.include
>> create mode 100644 tests/tcg/configure.sh
>> delete mode 100644 tests/tcg/cris/Makefile.include
>> delete mode 100644 tests/tcg/hppa/Makefile.include
>> delete mode 100644 tests/tcg/i386/Makefile.include
>> delete mode 100644 tests/tcg/m68k/Makefile.include
>> delete mode 100644 tests/tcg/mips/Makefile.include
>> delete mode 100644 tests/tcg/ppc/Makefile.include
>> delete mode 100644 tests/tcg/riscv/Makefile.include
>> delete mode 100644 tests/tcg/s390x/Makefile.include
>> delete mode 100644 tests/tcg/sh4/Makefile.include
>> delete mode 100644 tests/tcg/sparc64/Makefile.include
>> create mode 100644 tests/tcg/x86_64/Makefile.softmmu-target
>> delete mode 100644 tests/tcg/xtensa/Makefile.include
>
>
> --
> Alex Bennée
>
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
Paolo Bonzini writes:
> The tests/tcg rely a lot on per-target informations from
> the QEMU makefiles, but most of the definitions in there
> aren't really relevant to TCG tests.
>
> This series is just a cleanup, but it could also be
> a useful start in making it possible to compile tests/tcg
> out of QEMU's tree, and/or making it a submodule, and/or
> unifying the system emulation tests with kvm-unit-tests.
Hmm something is throwing off configure and making it use my login shell
instead of /bin/sh:
libpmem support no
libudev yes
default devices yes
~/lsrc/qemu.git/tests/tcg/configure.sh (line 63): 'case' builtin not inside
of switch block
case $arch in
^
fish: Error while reading file
/home/alex/lsrc/qemu.git/tests/tcg/configure.sh
>
> Paolo
>
> Paolo Bonzini (3):
> tests/tcg: use EXTRA_CFLAGS everywhere
> tests/tcg: cleanup Makefile inclusions
> tests/tcg: move configuration to a sub-shell script
>
> Makefile | 1 +
> Makefile.target | 3 -
> configure | 155 ++--
> tests/Makefile.include| 25 ++--
> tests/tcg/Makefile.include| 88
> tests/tcg/Makefile.prereqs| 18 +++
> tests/tcg/Makefile.probe | 31
> tests/tcg/Makefile.qemu | 95 +
> tests/tcg/{Makefile => Makefile.target} | 15 +-
> tests/tcg/aarch64/Makefile.include| 8 --
> tests/tcg/aarch64/Makefile.softmmu-target | 4 +-
> tests/tcg/aarch64/Makefile.target | 12 +-
> tests/tcg/alpha/Makefile.include | 2 -
> tests/tcg/alpha/Makefile.softmmu-target | 4 +-
> tests/tcg/arm/Makefile.include| 8 --
> tests/tcg/arm/Makefile.softmmu-target | 6 +-
> tests/tcg/configure.sh| 228
> ++
> tests/tcg/cris/Makefile.include | 6 -
> tests/tcg/hppa/Makefile.include | 2 -
> tests/tcg/i386/Makefile.include | 9 --
> tests/tcg/i386/Makefile.softmmu-target| 12 +-
> tests/tcg/i386/Makefile.target| 13 +-
> tests/tcg/m68k/Makefile.include | 2 -
> tests/tcg/minilib/Makefile.target | 2 +-
> tests/tcg/mips/Makefile.include | 20 ---
> tests/tcg/ppc/Makefile.include| 10 --
> tests/tcg/riscv/Makefile.include | 10 --
> tests/tcg/s390x/Makefile.include | 2 -
> tests/tcg/sh4/Makefile.include| 4 -
> tests/tcg/sparc64/Makefile.include| 2 -
> tests/tcg/x86_64/Makefile.softmmu-target | 36 +
> tests/tcg/x86_64/Makefile.target | 7 +-
> tests/tcg/xtensa/Makefile.include | 11 --
> tests/tcg/xtensa/Makefile.softmmu-target | 4 +-
> 34 files changed, 435 insertions(+), 420 deletions(-)
> delete mode 100644 tests/tcg/Makefile.include
> create mode 100644 tests/tcg/Makefile.prereqs
> delete mode 100644 tests/tcg/Makefile.probe
> create mode 100644 tests/tcg/Makefile.qemu
> rename tests/tcg/{Makefile => Makefile.target} (90%)
> delete mode 100644 tests/tcg/aarch64/Makefile.include
> delete mode 100644 tests/tcg/alpha/Makefile.include
> delete mode 100644 tests/tcg/arm/Makefile.include
> create mode 100644 tests/tcg/configure.sh
> delete mode 100644 tests/tcg/cris/Makefile.include
> delete mode 100644 tests/tcg/hppa/Makefile.include
> delete mode 100644 tests/tcg/i386/Makefile.include
> delete mode 100644 tests/tcg/m68k/Makefile.include
> delete mode 100644 tests/tcg/mips/Makefile.include
> delete mode 100644 tests/tcg/ppc/Makefile.include
> delete mode 100644 tests/tcg/riscv/Makefile.include
> delete mode 100644 tests/tcg/s390x/Makefile.include
> delete mode 100644 tests/tcg/sh4/Makefile.include
> delete mode 100644 tests/tcg/sparc64/Makefile.include
> create mode 100644 tests/tcg/x86_64/Makefile.softmmu-target
> delete mode 100644 tests/tcg/xtensa/Makefile.include
--
Alex Bennée
Re: [Qemu-devel] [PATCH 0/3] target/mips: Convert to do_transaction_failed hook
Cc'ing broader MIPS audience. On 8/2/19 6:04 PM, Peter Maydell wrote: > This patchset converts the MIPS target away from the > old broken do_unassigned_access hook to the new (added in > 2017...) do_transaction_failed hook. > > The motivation here is: > * do_unassigned_access is broken because: > + it will be called for any kind of access to physical addresses > where there is no assigned device, whether that access is by the > CPU or by something else (like a DMA controller!), so it can > result in spurious guest CPU exceptions. > + It will also get called even when using KVM, when there's nothing > useful it can do. > + It isn't passed in the return-address within the TCG generated > code, so it isn't able to correctly restore the CPU state > before generating the exception, and so the exception will > often be generated with the wrong faulting guest PC value > * there are now only a few targets still using the old hook, >so if we can convert them we can delete all the old code >and complete this API transation. (Patches for SPARC are on >the list; the other user is RISCV, which accidentally >implemented the old hook rather than the new one recently.) > > The general approach to the conversion is to check the target for > load/store-by-physical-address operations which were previously > implicitly causing exceptions, to see if they now need to explicitly > check for and handle memory access failures. (The 'git grep' regexes > in docs/devel/loads-stores.rst are useful here: the API families to > look for are ld*_phys/st*_phys, address_space_ld/st*, and > cpu_physical_memory*.) > > For MIPS, there are none of these (the usual place where targets do > this is hardware page table walks where the page table entries are > loaded by physical address, and MIPS doesn't seem to have those). > > Code audit out of the way, the actual hook changeover is pretty > simple. > > The complication here is the MIPS Jazz board, which has some rather > dubious code that intercepts the do_unassigned_access hook to suppress > generation of exceptions for invalid accesses due to data accesses, > while leaving exceptions for invalid instruction fetches in place. I'm > a bit dubious about whether the behaviour we have implemented here is > really what the hardware does -- it seems pretty odd to me to not > generate exceptions for d-side accesses but to generate them for > i-side accesses, and looking back through git and mailing list history > this code is here mainly as "put back the behaviour we had before a > previous commit broke it", and that older behaviour in turn I think is > more historical-accident than because anybody deliberately checked the > hardware behaviour and made QEMU work that way. However, I don't have > any real hardware to do comparative tests on, so this series retains > the same behaviour we had before on this board, by making it intercept > the new hook in the same way it did the old one. I've beefed up the > comment somewhat to indicate what we're doing, why, and why it might > not be right. > > The patch series is structured in three parts: > * make the Jazz board code support CPUs regardless of which >of the two hooks they implement > * switch the MIPS CPUs over to implementing the new hook > * remove the no-longer-needed Jazz board code for the old >hook > (This seemed cleaner to me than squashing the whole thing into > a single patch that touched core mips code and the jazz board > at the same time.) > > I have tested this with: > * the ARC Multiboot BIOS linked to from the bug >https://bugs.launchpad.net/qemu/+bug/1245924 (and which >was the test case for needing the hook intercept) > * a Linux kernel for the 'mips' mips r4k machine > * 'make check' > Obviously more extensive testing would be useful, but I > don't have any other test images. I also don't have > a KVM MIPS host, which would be worth testing to confirm > that it also still works. > > If anybody happens by some chance to still have a working > real-hardware Magnum or PICA61 board, we could perhaps test > how it handles accesses to invalid memory, but I suspect that > nobody does any more :-) > > thanks > -- PMM > > > Peter Maydell (3): > hw/mips/mips_jazz: Override do_transaction_failed hook > target/mips: Switch to do_transaction_failed() hook > hw/mips/mips_jazz: Remove no-longer-necessary override of > do_unassigned_access > > target/mips/internal.h | 8 --- > hw/mips/mips_jazz.c | 47 + > target/mips/cpu.c | 2 +- > target/mips/op_helper.c | 24 +++-- > 4 files changed, 47 insertions(+), 34 deletions(-) >
Re: [Qemu-devel] [PATCH 0/3] backup fixes for 4.1?
On 7/31/19 6:29 AM, Vladimir Sementsov-Ogievskiy wrote: > 30.07.2019 21:41, John Snow wrote: >> >> >> On 7/30/19 12:32 PM, Vladimir Sementsov-Ogievskiy wrote: >>> Hi all! >>> >>> Here are two small fixes. >>> >>> 01 is not a degradation at all, so it's OK for 4.2 >>> 02 is degradation of 3.0, so it's possibly OK for 4.2 too, >>> but it seems to be real bug and fix is very simple, so, >>> may be 4.1 is better >>> >>> Or you may take the whole series to 4.1 if you want. >>> >> >> I think (1) and (2) can go in for stable after review, but they're not >> crucial for 4.1 especially at this late of a stage. Should be cataclysms >> only right now. >> >> --js >> > > I can rebase it than on your bitmaps branch. Or, if we want it for stable, > maybe, > I shouldn't? > Good point. Keep it based on main and I'll slip it in at the beginning of the staging queue. --js
Re: [Qemu-devel] [PATCH 0/3] backup fixes for 4.1?
30.07.2019 21:41, John Snow wrote: > > > On 7/30/19 12:32 PM, Vladimir Sementsov-Ogievskiy wrote: >> Hi all! >> >> Here are two small fixes. >> >> 01 is not a degradation at all, so it's OK for 4.2 >> 02 is degradation of 3.0, so it's possibly OK for 4.2 too, >> but it seems to be real bug and fix is very simple, so, >> may be 4.1 is better >> >> Or you may take the whole series to 4.1 if you want. >> > > I think (1) and (2) can go in for stable after review, but they're not > crucial for 4.1 especially at this late of a stage. Should be cataclysms > only right now. > > --js > I can rebase it than on your bitmaps branch. Or, if we want it for stable, maybe, I shouldn't? -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH 0/3] backup fixes for 4.1?
On 7/30/19 12:32 PM, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > Here are two small fixes. > > 01 is not a degradation at all, so it's OK for 4.2 > 02 is degradation of 3.0, so it's possibly OK for 4.2 too, >but it seems to be real bug and fix is very simple, so, >may be 4.1 is better > > Or you may take the whole series to 4.1 if you want. > I think (1) and (2) can go in for stable after review, but they're not crucial for 4.1 especially at this late of a stage. Should be cataclysms only right now. --js
Re: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles
Patchew URL: https://patchew.org/QEMU/20190730123759.21723-1-pbonz...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Subject: [Qemu-devel] [PATCH 0/3] tests/tcg: disentangle makefiles Message-id: 20190730123759.21723-1-pbonz...@redhat.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu ee9545e..6e9a6cb master -> master * [new tag] patchew/20190730123759.21723-1-pbonz...@redhat.com -> patchew/20190730123759.21723-1-pbonz...@redhat.com Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone' Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc' Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers' Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF' Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 'roms/edk2' Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe' Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios' Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware' Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for path 'roms/opensbi' Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode' Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) registered for path 'roms/seabios-hppa' Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios' Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot' Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot' Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex' Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp' Submodule 'tests/fp/berkeley-softfloat-3' (https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 'tests/fp/berkeley-softfloat-3' Submodule 'tests/fp/berkeley-testfloat-3' (https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 'tests/fp/berkeley-testfloat-3' Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into 'capstone'... Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf' Cloning into 'dtc'... Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536' Cloning into 'roms/QemuMacDrivers'... Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266' Cloning into 'roms/SLOF'... Submodule path 'roms/SLOF': checked out 'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3' Cloning into 'roms/edk2'... Submodule path 'roms/edk2': checked out '20d2e5a125e34fc8501026613a71549b2a1a3e54' Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3' Submodule 'CryptoPkg/Library/OpensslLib/openssl' (https://github.com/openssl/openssl) registered for path 'CryptoPkg/Library/OpensslLib/openssl' Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'... Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037' Cloning into 'CryptoPkg/Library/OpensslLib/openssl'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out '50eaac9f3337667259de725451f201e784599687' Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered for path 'boringssl' Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5' Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) registered for path 'pyca-cryptography' Cloning into 'boringssl'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f' Cloning into 'krb5'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked out 'b9ad6c49505c96a088326b62a52568e3484f2168' Cloning into 'pyca-cryptography'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out '09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Re: [Qemu-devel] [PATCH 0/3] virtiofsd: add FUSE_INIT map_alignment field
* Stefan Hajnoczi (stefa...@redhat.com) wrote: > The client must know the server's alignment constraints for FUSE_SETUPMAPPING > and FUSE_REMOVEMAPPING. This is necessary because mmap(2)/munmap(2) have > alignment constraints and the guest may have a different page size from the > host. The new FUSE_INIT map_alignment field communicates this information to > the client. OK, merged into my local world. DAve > Stefan Hajnoczi (3): > virtiofsd: sync up fuse.h Linux 5.1 header > virtiofsd: add map_alignment to fuse_kernel.h > virtiofsd: implement FUSE_INIT map_alignment field > > contrib/virtiofsd/fuse_kernel.h | 56 +-- > contrib/virtiofsd/fuse_lowlevel.c | 8 + > 2 files changed, 47 insertions(+), 17 deletions(-) > > -- > 2.21.0 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] spapr/pci: Improve MSI tracking
On Fri, Jul 26, 2019 at 04:44:33PM +0200, Greg Kurz wrote: > Some recent tests with AIX guests showed that we don't tear down > MSIs that were allocated with the "change-msi" RTAS call, when > the guest is rebooted. This series teach PHBs to do the cleanup > at reset time. > > This bug has always been there. Not sure it is worth the pain to > have this fixed in 4.1. Since we've only recently started looking at AIX as a supported guest, and these bugs don't appear to break Linux guests, no, I don't think it is. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 0/3] Series to implement suspend for ppc/spapr
Paolo Bonzini's on July 18, 2019 9:08 pm: > On 18/07/19 12:39, Nicholas Piggin wrote: >> Any comments on this series would be welcome. Hopefully someone who >> knows i386 can give some feedback on the possible bug fix, and >> whether the new wakeup method will suit i386. > > Looks good, though only i386 supports wakeup so perhaps it's better to > DTRT and move the reset to the PC machine's wakeup method. Then pseries > need not implement mc->wakeup at all. Yeah that probably makes more sense because the i386 patch should be quite trivial. I will try that and re-send. Thanks, Nick
Re: [Qemu-devel] [PATCH 0/3] Series to implement suspend for ppc/spapr
On 18/07/19 12:39, Nicholas Piggin wrote: > Any comments on this series would be welcome. Hopefully someone who > knows i386 can give some feedback on the possible bug fix, and > whether the new wakeup method will suit i386. Looks good, though only i386 supports wakeup so perhaps it's better to DTRT and move the reset to the PC machine's wakeup method. Then pseries need not implement mc->wakeup at all. Paolo > Thanks, > Nick > > Nicholas Piggin (3): > qmp: don't emit the RESET event on wakeup > machine: Add wakeup method to MachineClass > spapr: Implement ibm,suspend-me > > hw/ppc/spapr.c | 11 +++ > hw/ppc/spapr_rtas.c| 32 > include/hw/boards.h| 1 + > include/hw/ppc/spapr.h | 3 ++- > vl.c | 31 +-- > 5 files changed, 75 insertions(+), 3 deletions(-) >
Re: [Qemu-devel] [PATCH 0/3] s390x/cpumodel fixes for 4.1
Patchew URL: https://patchew.org/QEMU/20190715142304.215018-1-borntrae...@de.ibm.com/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20190715142304.215018-1-borntrae...@de.ibm.com Type: series Subject: [Qemu-devel] [PATCH 0/3] s390x/cpumodel fixes for 4.1 === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === From https://github.com/patchew-project/qemu t [tag update] patchew/20190715142304.215018-1-borntrae...@de.ibm.com -> patchew/20190715142304.215018-1-borntrae...@de.ibm.com Switched to a new branch 'test' 7bc436d3ed s390x/cpumodel: change internal name of vxp to make description 6b0474bbb3 s390x/cpumodel: also change name of vxbeh 2a344a s390x/cpumodel: remove esort from the default model === OUTPUT BEGIN === 1/3 Checking commit 2a344a55 (s390x/cpumodel: remove esort from the default model) 2/3 Checking commit 6b0474bbb3ec (s390x/cpumodel: also change name of vxbeh) WARNING: line over 80 characters #29: FILE: target/s390x/cpu_features_def.inc.h:107: +DEF_FEAT(VECTOR_BCD_ENH, "vxp", STFL, 152, "Vector-Packed-Decimal-Enhancement Facility") total: 0 errors, 1 warnings, 8 lines checked Patch 2/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/3 Checking commit 7bc436d3eda4 (s390x/cpumodel: change internal name of vxp to make description) ERROR: line over 90 characters #22: FILE: target/s390x/cpu_features_def.inc.h:107: +DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH, "vxp", STFL, 152, "Vector-Packed-Decimal-Enhancement Facility") total: 1 errors, 0 warnings, 24 lines checked Patch 3/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190715142304.215018-1-borntrae...@de.ibm.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] s390x/cpumodel fixes for 4.1
On Mon, 15 Jul 2019 16:23:01 +0200 Christian Borntraeger wrote: > Some fallout of the gen15 cpu model. As this is new in 4.1 > it is still time to fixup some aspects. > > Christian Borntraeger (3): > s390x/cpumodel: remove esort from the default model > s390x/cpumodel: also change name of vxbeh > s390x/cpumodel: change internal name of vxp to make description > > target/s390x/cpu_features_def.inc.h | 2 +- > target/s390x/gen-features.c | 5 ++--- > 2 files changed, 3 insertions(+), 4 deletions(-) > Thanks, queued to s390-fixes (with s/vxp/vxpdeh/).
Re: [Qemu-devel] [PATCH 0/3] hw/Kconfig: PCI & USB fixes
On 15/07/19 11:55, Philippe Mathieu-Daudé wrote: > I actually wanted to clean the USB devices (i.e. to not have > USB3 devices appear on OHCI buses) but this is too late for > the next release, so let's post the patches accumulated. > > Philippe Mathieu-Daudé (3): > hw/Kconfig: PCI bus implies PCI_DEVICES > hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI > hw/usb/Kconfig: USB_XHCI_NEC requires USB_XHCI > > docs/devel/kconfig.rst | 1 - > hw/alpha/Kconfig | 1 - > hw/arm/Kconfig | 4 > hw/hppa/Kconfig| 1 - > hw/i386/Kconfig| 1 - > hw/pci/Kconfig | 1 + > hw/ppc/Kconfig | 6 -- > hw/riscv/Kconfig | 1 - > hw/sh4/Kconfig | 1 - > hw/sparc64/Kconfig | 1 - > hw/usb/Kconfig | 11 +++ > hw/usb/Makefile.objs | 5 +++-- > 12 files changed, 11 insertions(+), 23 deletions(-) > ACK for 2 and 3. Gerd is on vacation so I'm picking them up. Paolo
Re: [Qemu-devel] [PATCH 0/3] migration: fix migrate_cancel problems of multifd
The problem still exists in mainline, Ping for review On Tue, Jun 25, 2019 at 9:18 PM Ivan Ren wrote: > The patches fix the problems encountered in multifd migration when try > to cancel the migration by migrate_cancel. > > Ivan Ren (3): > migration: fix migrate_cancel leads live_migration thread endless loop > migration: fix migrate_cancel leads live_migration thread hung forever > migration: fix migrate_cancel multifd migration leads destination hung > forever > > migration/ram.c | 62 - > 1 file changed, 51 insertions(+), 11 deletions(-) > > -- > 2.17.2 (Apple Git-113) > >
Re: [Qemu-devel] [PATCH 0/3] virtio pmem: coverity fixes
> > On Fri, 12 Jul 2019 13:05:51 +0530 > Pankaj Gupta wrote: > > > This patch series two fixes for coverity and a > > transactional info removal patch. > > > > Pankaj Gupta (3): > > virtio pmem: fix wrong mem region condition > > virtio pmem: remove memdev null check > > virtio pmem: remove transational device info > > > > hw/virtio/virtio-pmem-pci.c | 4 +--- > > hw/virtio/virtio-pmem.c | 4 ++-- > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > I think all of this is 4.1 material. Yes, forgot to add in the subject. Thank you for the review. Best regards, Pankaj > >
Re: [Qemu-devel] [PATCH 0/3] virtio pmem: coverity fixes
On Fri, 12 Jul 2019 13:05:51 +0530 Pankaj Gupta wrote: > This patch series two fixes for coverity and a > transactional info removal patch. > > Pankaj Gupta (3): > virtio pmem: fix wrong mem region condition > virtio pmem: remove memdev null check > virtio pmem: remove transational device info > > hw/virtio/virtio-pmem-pci.c | 4 +--- > hw/virtio/virtio-pmem.c | 4 ++-- > 2 files changed, 3 insertions(+), 5 deletions(-) > I think all of this is 4.1 material.
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On 10/07/19 11:03, Daniel P. Berrangé wrote: >> Would it be possible to make QEMU the broker? That is, how hard would it >> be to embed a minimal DBus broker (which only takes care of connecting >> servers >> and clients---stuff like launching servers would be completely out of scope)? > What would be the benefit of embedding it in QEMU ? If in the future we want to keep only the multiprocess case then QEMU would be able to launch subprocesses itself. In this case you'd keep the old command line working but QEMU would set up the bus and the services that work together on it (for example the basic QOM operations such as unparent and property get/set could be mapped to a DBus interface, and QOM classes and interfaces could also become DBus interfaces). The broker itself could be a separate subprocess. Paolo
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On Tue, Jul 09, 2019 at 02:47:32PM +0400, Marc-André Lureau wrote: > Hi > > On Tue, Jul 9, 2019 at 1:02 PM Daniel P. Berrangé wrote: > > > > On Tue, Jul 09, 2019 at 12:26:38PM +0400, Marc-André Lureau wrote: > > > Hi > > > > > > On Mon, Jul 8, 2019 at 8:04 PM Daniel P. Berrangé > > > wrote: > > > > QEMU already has a direct UNIX socket connection to the helper > > > > processes in question. I'd much rather we just had another direct > > > > UNIX socket connection to that helper, using D-Bus peer-to-peer. > > > > The benefit of debugging doesn't feel compelling enough to justify > > > > running an extra daemon for each VM. > > > > > > I wouldn't minor the need for easier debugging. Debugging multiple > > > processes talking to each other is really hard. Having a bus is > > > awesome (if not required) in this case. > > > > > > There are other advantages of using a bus, those come to my mind: > > > > > > - less connections (bus topology) > > > > That applies to general use of DBus, but doesn't really apply to > > the proposed QEMU usage, as every single helper is talking to the > > same QEMU endpoint. So if we have 10 helpers, in p2p mode, we > > get 10 sockets open between the helper & QEMU. In bus mode, we > > get 10 sockets open between the helper & dbus and another socket > > open between dbus & QEMU. The bus is only a win in connections > > if you have a mesh-like connection topology not hub & spoke. > > The mesh already exist, as it's not just QEMU that want to talk to the > helpers, but the management layer, and 3rd parties (debug tools, > audit, other management tools etc). There are also cases where helpers > may want to talk to each other. Taking networking as an example, 2 > slirp interfaces may want to share the same DHCP, bootp/TFTP, > filter/service provider. Redirection/forwarding may be provided on > demand (chardev-like services). The same is probably true for block > layers, security, GPU/display etc. In this case, the bus topology > makes more sense than hiding it under. These are alot of scenarios / use cases not described in the cover letter for this series. I'm reviewing this series from the POV of the need to transfer vmstate from a helper back to QEMU, which was the scenario in the cover letter. From this I see no need for a bus. If you think there's a more general use cases involving QEMU backends that will need the bus, then I think the bigger picture needs to be described when proposing the use of the bus, instead of only describing the very simple vmstate use case as the motivation. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On Wed, Jul 10, 2019 at 10:53:10AM +0200, Paolo Bonzini wrote: > On 08/07/19 18:04, Daniel P. Berrangé wrote: > > The downside of using the bus daemon is that we have to spawn a new > > instance of dbus-daemon for every QEMU VM that's running on the host, > > which is yet more memory overhead for each VM & another process to > > manage, and yet another thing to go wrong. > > > > QEMU already has a direct UNIX socket connection to the helper > > processes in question. I'd much rather we just had another direct > > UNIX socket connection to that helper, using D-Bus peer-to-peer. > > The benefit of debugging doesn't feel compelling enough to justify > > running an extra daemon for each VM. > > Would it be possible to make QEMU the broker? That is, how hard would it > be to embed a minimal DBus broker (which only takes care of connecting servers > and clients---stuff like launching servers would be completely out of scope)? What would be the benefit of embedding it in QEMU ? I see significant security downside to that which would mean its not something I'd want to support. If we accept the need for a bus then this implies there's a need for service <-> service messages, where neither service is QEMU. This in turn requires enforcement of security policies for the separation of services. It is highly desirable, if not mandatory, to have such security enforcement outside the QEMU address space, given that QEMU is an untrustworthy component. > Would it for example make sense to split the bus handling part of dbus-broker > into a library that QEMU could reuse? (And if we plan to do this, should QEMU > use sd-bus instead of gdbus?) > > In QOM that would be something like > > -object dbus-connection,id=client1,chardev=...,addr=foo # p2p > -object dbus-vmstate,connection=client1 # the > interface > > -object dbus-connection,id=client1,addr=foo # via > external daemon > -object dbus-vmstate,client=client1 # the > interface > > -object dbus-session,id=session1,chardev=... > -object dbus-connection,id=client1,session=session1,addr=foo # via > internal daemon > -object dbus-vmstate,client=client1 # the > interface >From my POV I only see two viable options. Either p2p with no bus & QEMU being one endpoint so there's no requirement for security policies, or bus based mesh with an external process to enforce security policy. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On 08/07/19 18:04, Daniel P. Berrangé wrote: > The downside of using the bus daemon is that we have to spawn a new > instance of dbus-daemon for every QEMU VM that's running on the host, > which is yet more memory overhead for each VM & another process to > manage, and yet another thing to go wrong. > > QEMU already has a direct UNIX socket connection to the helper > processes in question. I'd much rather we just had another direct > UNIX socket connection to that helper, using D-Bus peer-to-peer. > The benefit of debugging doesn't feel compelling enough to justify > running an extra daemon for each VM. Would it be possible to make QEMU the broker? That is, how hard would it be to embed a minimal DBus broker (which only takes care of connecting servers and clients---stuff like launching servers would be completely out of scope)? Would it for example make sense to split the bus handling part of dbus-broker into a library that QEMU could reuse? (And if we plan to do this, should QEMU use sd-bus instead of gdbus?) In QOM that would be something like -object dbus-connection,id=client1,chardev=...,addr=foo # p2p -object dbus-vmstate,connection=client1 # the interface -object dbus-connection,id=client1,addr=foo # via external daemon -object dbus-vmstate,client=client1 # the interface -object dbus-session,id=session1,chardev=... -object dbus-connection,id=client1,session=session1,addr=foo # via internal daemon -object dbus-vmstate,client=client1 # the interface Paolo
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
Hi On Tue, Jul 9, 2019 at 1:02 PM Daniel P. Berrangé wrote: > > On Tue, Jul 09, 2019 at 12:26:38PM +0400, Marc-André Lureau wrote: > > Hi > > > > On Mon, Jul 8, 2019 at 8:04 PM Daniel P. Berrangé > > wrote: > > > > The D-Bus protocol can be made to work peer-to-peer, but the most > > > > common and practical way is through a bus daemon. This also has the > > > > advantage of increased debuggability (you can eavesdrop on the bus and > > > > introspect it). > > > > > > The downside of using the bus daemon is that we have to spawn a new > > > instance of dbus-daemon for every QEMU VM that's running on the host, > > > which is yet more memory overhead for each VM & another process to > > > manage, and yet another thing to go wrong. > > > > dbus-daemon (or dbus-broker) has been optimized to fit on many devices > > and use cases, it doesn't take much memory (3mb for my session dbus > > right now). > > > > More processes to manage is inevitable. In a near future, we may have > > 5-10 processes running around qemu. I think dbus-daemon will be one of > > the easiest to deal with. (as can be seen in the dbus-vmstate test, it > > is very simple to start a private dbus-daemon) > > The increase in processes per-QEMU is a significant concern I have > around complexity & manageability in general, hence a desire to avoid > requiring processes unless they have a compelling reason to exist. Fair enough, although when the job a bus is done by some other process (libvirt, qemu or other external process), then I would much rather have dbus-daemon doing it. > > > > QEMU already has a direct UNIX socket connection to the helper > > > processes in question. I'd much rather we just had another direct > > > UNIX socket connection to that helper, using D-Bus peer-to-peer. > > > The benefit of debugging doesn't feel compelling enough to justify > > > running an extra daemon for each VM. > > > > I wouldn't minor the need for easier debugging. Debugging multiple > > processes talking to each other is really hard. Having a bus is > > awesome (if not required) in this case. > > > > There are other advantages of using a bus, those come to my mind: > > > > - less connections (bus topology) > > That applies to general use of DBus, but doesn't really apply to > the proposed QEMU usage, as every single helper is talking to the > same QEMU endpoint. So if we have 10 helpers, in p2p mode, we > get 10 sockets open between the helper & QEMU. In bus mode, we > get 10 sockets open between the helper & dbus and another socket > open between dbus & QEMU. The bus is only a win in connections > if you have a mesh-like connection topology not hub & spoke. The mesh already exist, as it's not just QEMU that want to talk to the helpers, but the management layer, and 3rd parties (debug tools, audit, other management tools etc). There are also cases where helpers may want to talk to each other. Taking networking as an example, 2 slirp interfaces may want to share the same DHCP, bootp/TFTP, filter/service provider. Redirection/forwarding may be provided on demand (chardev-like services). The same is probably true for block layers, security, GPU/display etc. In this case, the bus topology makes more sense than hiding it under. > > > - configuring/enforcing policies & limits > > I don't see that as an advantage. Rather it is addressing the > decreased security that the bus model exposes. In peer2peer > mode, the helpers can only talk to QEMU, so can't directly > interact with each other. In bus mode, the helpers have a > direct communications path to attack each other over, so we > absolutely need policy to mitigate this increased risk. It > would be better to remove that risk at any architectural > level by not having a bus at all. You can enforce security/policy at the bus level, in a single place (including with selinux/apparmor context - although I am not sure how much that gives you). If each helper process implements its own protocol, you will probably never have that kind of central enforcement. And if they exist, libvirt/management layer, qemu & helpers will have to implement it for each case... > > > - on-demand service activation & discoverability > > Again useful for dbus in general, but I don't see any clear scenario > in which this is relevant to QEMU's usage. Perhaps not to QEMU itself, but helpers could benefit it, see examples I listed above. > > > I also think D-Bus is the IPC of choice for multi-process. It's easier > > to use than many other IPC due to the various tools and language > > bindings available. Having a common bus is a good incentive to use a > > common IPC, instead of a dozen of half-baked protocols. > > As I said, I don't have any objection to DBus as a protocol. I think it > would serve our needs well, most especially because GIO has decent API > bindings to using it, so we avoid having to depend on another 3rd party > library for something else. > > I think from QEMU's POV, the only real alternative to DBus would
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On Tue, Jul 09, 2019 at 12:26:38PM +0400, Marc-André Lureau wrote: > Hi > > On Mon, Jul 8, 2019 at 8:04 PM Daniel P. Berrangé wrote: > > > The D-Bus protocol can be made to work peer-to-peer, but the most > > > common and practical way is through a bus daemon. This also has the > > > advantage of increased debuggability (you can eavesdrop on the bus and > > > introspect it). > > > > The downside of using the bus daemon is that we have to spawn a new > > instance of dbus-daemon for every QEMU VM that's running on the host, > > which is yet more memory overhead for each VM & another process to > > manage, and yet another thing to go wrong. > > dbus-daemon (or dbus-broker) has been optimized to fit on many devices > and use cases, it doesn't take much memory (3mb for my session dbus > right now). > > More processes to manage is inevitable. In a near future, we may have > 5-10 processes running around qemu. I think dbus-daemon will be one of > the easiest to deal with. (as can be seen in the dbus-vmstate test, it > is very simple to start a private dbus-daemon) The increase in processes per-QEMU is a significant concern I have around complexity & manageability in general, hence a desire to avoid requiring processes unless they have a compelling reason to exist. > > QEMU already has a direct UNIX socket connection to the helper > > processes in question. I'd much rather we just had another direct > > UNIX socket connection to that helper, using D-Bus peer-to-peer. > > The benefit of debugging doesn't feel compelling enough to justify > > running an extra daemon for each VM. > > I wouldn't minor the need for easier debugging. Debugging multiple > processes talking to each other is really hard. Having a bus is > awesome (if not required) in this case. > > There are other advantages of using a bus, those come to my mind: > > - less connections (bus topology) That applies to general use of DBus, but doesn't really apply to the proposed QEMU usage, as every single helper is talking to the same QEMU endpoint. So if we have 10 helpers, in p2p mode, we get 10 sockets open between the helper & QEMU. In bus mode, we get 10 sockets open between the helper & dbus and another socket open between dbus & QEMU. The bus is only a win in connections if you have a mesh-like connection topology not hub & spoke. > - configuring/enforcing policies & limits I don't see that as an advantage. Rather it is addressing the decreased security that the bus model exposes. In peer2peer mode, the helpers can only talk to QEMU, so can't directly interact with each other. In bus mode, the helpers have a direct communications path to attack each other over, so we absolutely need policy to mitigate this increased risk. It would be better to remove that risk at any architectural level by not having a bus at all. > - on-demand service activation & discoverability Again useful for dbus in general, but I don't see any clear scenario in which this is relevant to QEMU's usage. > I also think D-Bus is the IPC of choice for multi-process. It's easier > to use than many other IPC due to the various tools and language > bindings available. Having a common bus is a good incentive to use a > common IPC, instead of a dozen of half-baked protocols. As I said, I don't have any objection to DBus as a protocol. I think it would serve our needs well, most especially because GIO has decent API bindings to using it, so we avoid having to depend on another 3rd party library for something else. I think from QEMU's POV, the only real alternative to DBus would be to build something on QMP. I prefer DBus, because JSON is a disaster for integer type handling, and DBus is more accessible for the helper apps which can easily use a DBus API of their choice. > Nevertheless, I also think we could use D-Bus in peer-to-peer mode, > and I did some investigation. The slirp-helper supports it. We could > teach dbus-vmstate to eastablish peer-to-peer connections. Instead of > receiving a bus address and list of Ids, it could have a list of dbus > peer socket path. Both approaches are not incompatible, but I think > the bus benefits outweigh the downside of running an extra process. As above I'm not seeing the compelling benefits of using a bus, so think we shoud stick to dbus in p2p mode. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
Hi On Mon, Jul 8, 2019 at 8:04 PM Daniel P. Berrangé wrote: > > The D-Bus protocol can be made to work peer-to-peer, but the most > > common and practical way is through a bus daemon. This also has the > > advantage of increased debuggability (you can eavesdrop on the bus and > > introspect it). > > The downside of using the bus daemon is that we have to spawn a new > instance of dbus-daemon for every QEMU VM that's running on the host, > which is yet more memory overhead for each VM & another process to > manage, and yet another thing to go wrong. dbus-daemon (or dbus-broker) has been optimized to fit on many devices and use cases, it doesn't take much memory (3mb for my session dbus right now). More processes to manage is inevitable. In a near future, we may have 5-10 processes running around qemu. I think dbus-daemon will be one of the easiest to deal with. (as can be seen in the dbus-vmstate test, it is very simple to start a private dbus-daemon) > > QEMU already has a direct UNIX socket connection to the helper > processes in question. I'd much rather we just had another direct > UNIX socket connection to that helper, using D-Bus peer-to-peer. > The benefit of debugging doesn't feel compelling enough to justify > running an extra daemon for each VM. I wouldn't minor the need for easier debugging. Debugging multiple processes talking to each other is really hard. Having a bus is awesome (if not required) in this case. There are other advantages of using a bus, those come to my mind: - less connections (bus topology) - configuring/enforcing policies & limits - on-demand service activation & discoverability I also think D-Bus is the IPC of choice for multi-process. It's easier to use than many other IPC due to the various tools and language bindings available. Having a common bus is a good incentive to use a common IPC, instead of a dozen of half-baked protocols. Nevertheless, I also think we could use D-Bus in peer-to-peer mode, and I did some investigation. The slirp-helper supports it. We could teach dbus-vmstate to eastablish peer-to-peer connections. Instead of receiving a bus address and list of Ids, it could have a list of dbus peer socket path. Both approaches are not incompatible, but I think the bus benefits outweigh the downside of running an extra process.
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On Mon, Jul 08, 2019 at 04:44:06PM +0100, Dr. David Alan Gilbert wrote: > * Marc-André Lureau (marcandre.lur...@redhat.com) wrote: > > Hi, > > > > With external processes or helpers participating to the VM support, it > > becomes necessary to handle their migration. Various options exist to > > transfer their state: > > 1) as the VM memory, RAM or devices (we could say that's how > >vhost-user devices can be handled today, they are expected to > >restore from ring state) > > 2) other "vmstate" (as with TPM emulator state blobs) > > 3) left to be handled by management layer > > > > 1) is not practical, since an external processes may legitimatelly > > need arbitrary state date to back a device or a service, or may not > > even have an associated device. > > > > 2) needs ad-hoc code for each helper, but is simple and working > > > > 3) is complicated for management layer, QEMU has the migration timing > > > > The proposed "dbus-vmstate" object will connect to a given D-Bus > > address, and save/load from org.qemu.VMState1 owners on migration. > > Some very high level questions: > a) If I've got two QEMU's running, how do the right devices >end up migrating to the right qemu? This isn't using the normal DBus instance. It needs a new isntance of dbus-daemon to be spawned for each VM IIUC. > b) Why use dbus for the comms? Don't all of the daemons have some >protocol'd socket between QEMU and the daemon? If so they could >send up a separate FD for migration data There's two distinct aspects here - Whether to use a bus vs peer-to-peer - What protocol to run over the wire DBus defines a low level wire protocol. It just happens that it is commonly used in bus topology, but it is fine being used peer-to-peer instead. IOW, we could use Dbus as the wire encoding, and still have a direct FD betwwen QEMU & the helper program, without needign dbus-daemon present. > c) Your 1MB limit is pretty aribtary - it's nice to have a limit > but it's hard to justify why it's that one. IIRC, that's the default DBus message size limit. You can choose to raise that in the client & server impl if desired, or alternatively just pass back a memfd() handle with the DBus relpy, over which to access the bulk data out of band. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
On Mon, Jul 08, 2019 at 11:24:34AM +0400, Marc-André Lureau wrote: > Hi, > > With external processes or helpers participating to the VM support, it > becomes necessary to handle their migration. Various options exist to > transfer their state: > 1) as the VM memory, RAM or devices (we could say that's how >vhost-user devices can be handled today, they are expected to >restore from ring state) > 2) other "vmstate" (as with TPM emulator state blobs) > 3) left to be handled by management layer > > 1) is not practical, since an external processes may legitimatelly > need arbitrary state date to back a device or a service, or may not > even have an associated device. > > 2) needs ad-hoc code for each helper, but is simple and working > > 3) is complicated for management layer, QEMU has the migration timing > > The proposed "dbus-vmstate" object will connect to a given D-Bus > address, and save/load from org.qemu.VMState1 owners on migration. > > Thus helpers can easily have their state migrated with QEMU, without > implementing ad-hoc support (such as done for TPM emulation) > > I chose D-Bus as it is ubiquitous on Linux (it is systemd IPC), and > can be made to work on various other OSes. There are several > implementations and good bindings for various languages. > (the tests/dbus-vmstate-test.c is a good example of how simple > the implementation of services can be, even in C) > > The D-Bus protocol can be made to work peer-to-peer, but the most > common and practical way is through a bus daemon. This also has the > advantage of increased debuggability (you can eavesdrop on the bus and > introspect it). The downside of using the bus daemon is that we have to spawn a new instance of dbus-daemon for every QEMU VM that's running on the host, which is yet more memory overhead for each VM & another process to manage, and yet another thing to go wrong. QEMU already has a direct UNIX socket connection to the helper processes in question. I'd much rather we just had another direct UNIX socket connection to that helper, using D-Bus peer-to-peer. The benefit of debugging doesn't feel compelling enough to justify running an extra daemon for each VM. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
* Marc-André Lureau (marcandre.lur...@redhat.com) wrote:
> Hi,
>
> With external processes or helpers participating to the VM support, it
> becomes necessary to handle their migration. Various options exist to
> transfer their state:
> 1) as the VM memory, RAM or devices (we could say that's how
>vhost-user devices can be handled today, they are expected to
>restore from ring state)
> 2) other "vmstate" (as with TPM emulator state blobs)
> 3) left to be handled by management layer
>
> 1) is not practical, since an external processes may legitimatelly
> need arbitrary state date to back a device or a service, or may not
> even have an associated device.
>
> 2) needs ad-hoc code for each helper, but is simple and working
>
> 3) is complicated for management layer, QEMU has the migration timing
>
> The proposed "dbus-vmstate" object will connect to a given D-Bus
> address, and save/load from org.qemu.VMState1 owners on migration.
Some very high level questions:
a) If I've got two QEMU's running, how do the right devices
end up migrating to the right qemu?
b) Why use dbus for the comms? Don't all of the daemons have some
protocol'd socket between QEMU and the daemon? If so they could
send up a separate FD for migration data
c) Your 1MB limit is pretty aribtary - it's nice to have a limit
but it's hard to justify why it's that one.
Dave
> Thus helpers can easily have their state migrated with QEMU, without
> implementing ad-hoc support (such as done for TPM emulation)
>
> I chose D-Bus as it is ubiquitous on Linux (it is systemd IPC), and
> can be made to work on various other OSes. There are several
> implementations and good bindings for various languages.
> (the tests/dbus-vmstate-test.c is a good example of how simple
> the implementation of services can be, even in C)
>
> The D-Bus protocol can be made to work peer-to-peer, but the most
> common and practical way is through a bus daemon. This also has the
> advantage of increased debuggability (you can eavesdrop on the bus and
> introspect it).
>
> dbus-vmstate is put into use by the libvirt series "[PATCH 00/23] Use
> a slirp helper process".
>
> Marc-André Lureau (3):
> qemu-file: move qemu_{get,put}_counted_string() declarations
> tests: add qtest_set_exit_status()
> Add dbus-vmstate object
>
> MAINTAINERS | 6 +
> backends/Makefile.objs | 4 +
> backends/dbus-vmstate.c | 497
> configure | 7 +
> docs/interop/dbus-vmstate.rst | 64
> docs/interop/index.rst | 1 +
> include/migration/qemu-file-types.h | 4 +
> migration/qemu-file.h | 4 -
> tests/Makefile.include | 18 +-
> tests/dbus-vmstate-test.c | 387 ++
> tests/dbus-vmstate1.xml | 12 +
> tests/libqtest.c| 41 +--
> tests/libqtest.h| 9 +
> 13 files changed, 1030 insertions(+), 24 deletions(-)
> create mode 100644 backends/dbus-vmstate.c
> create mode 100644 docs/interop/dbus-vmstate.rst
> create mode 100644 tests/dbus-vmstate-test.c
> create mode 100644 tests/dbus-vmstate1.xml
>
> --
> 2.22.0.214.g8dca754b1e
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
Patchew URL:
https://patchew.org/QEMU/20190708072437.3339-1-marcandre.lur...@redhat.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
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 2 fdc-test /x86_64/fdc/no_media_on_start
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-qobject-input-visitor -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-qobject-input-visitor"
PASS 3 fdc-test /x86_64/fdc/read_without_media
==10077==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 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"
==10121==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
==10121==WARNING: ASan is ignoring requested __asan_handle_no_return: stack
top: 0x7ffe12e7d000; bottom 0x7f68670f8000; size: 0x0095abd85000 (642833207296)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 3 test-coroutine /basic/yield
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==10136==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
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"
PASS 1 test-aio-multithread /aio/multi/lifecycle
==10142==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
PASS 2 test-aio-multithread /aio/multi/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ide-test"
==10165==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 1 ide-test /x86_64/ide/identify
==10176==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==10182==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 4 test-aio-multithread /aio/multi/mutex/handoff
==10188==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==10199==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/short_prdt
==10210==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
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"
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
---
PASS 5 test-throttle /throttle/have_timer
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
==10218==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 8 test-throttle /throttle/accounting
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
---
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
Re: [Qemu-devel] [PATCH 0/3] Add dbus-vmstate
Patchew URL:
https://patchew.org/QEMU/20190708072437.3339-1-marcandre.lur...@redhat.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
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 2 fdc-test /x86_64/fdc/no_media_on_start
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-string-output-visitor -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-string-output-visitor"
PASS 3 fdc-test /x86_64/fdc/read_without_media
==10016==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 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"
==10047==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
==10047==WARNING: ASan is ignoring requested __asan_handle_no_return: stack
top: 0x7fff648aa000; bottom 0x7fdcf7af8000; size: 0x00226cdb2000 (147855187968)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 3 test-coroutine /basic/yield
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
==10062==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
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))}
tests/test-aio-multithread -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-aio-multithread"
==10069==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
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ide-test"
PASS 2 test-aio-multithread /aio/multi/schedule
==10086==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
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==10097==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==10108==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 4 test-aio-multithread /aio/multi/mutex/handoff
==10114==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==10125==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/short_prdt
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
==10136==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
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"
==10143==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 15 test-throttle /throttle/config/iops_size
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
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"
==10151==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
Re: [Qemu-devel] [PATCH 0/3] Misc ati-vga fixes
Patchew URL: https://patchew.org/QEMU/cover.1562151410.git.bala...@eik.bme.hu/
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
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 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
==7804==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 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"
==7859==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==7859==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top:
0x7ffef6a42000; bottom 0x7fa151ff8000; size: 0x005da4a4a000 (402194210816)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
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/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ide-test"
==7874==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 25 test-aio /aio-gsource/event/wait
PASS 26 test-aio /aio-gsource/event/flush
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
==7883==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
==7889==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
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"
PASS 2 ide-test /x86_64/ide/flush
==7897==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
==7899==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 2 test-aio-multithread /aio/multi/schedule
==7916==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==7927==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/short_prdt
==7938==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
==7944==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 7 ide-test /x86_64/ide/bmdma/long_prdt
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
==7950==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==7950==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top:
0x7ffe31687000; bottom 0x7f238c3dc000; size: 0x00daa52ab000 (939073908736)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
---
PASS 9 ide-test /x86_64/ide/flush/nodev
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"
==7974==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 13 test-throttle /throttle/config/ranges
Re: [Qemu-devel] [PATCH 0/3] tests/acceptance: Add tests for the Leon3 board
On 6/27/19 2:28 PM, KONRAD Frederic wrote: > Hi Philippe, > > Thanks for that! > > I'm not aware at all of the tests/acceptance/* stuff.. How can we launch > those > tests? $ make subdir-sparc-softmmu $ make check-venv $ tests/venv/bin/python -m avocado --show=app run tests/acceptance/machine_sparc_leon3.py JOB ID : 12900968820fcd9ba2a03b9cfe2d060508c1d91c JOB LOG: /home/phil/avocado/job-results/job-2019-06-27T14.38-1290096/job.log (1/2) tests/acceptance/machine_sparc_leon3.py:Leon3Machine.test_leon3_helenos_uimage: PASS (1.10 s) (2/2) tests/acceptance/machine_sparc_leon3.py:Leon3Machine.test_leon3_linux_kernel_4_9_busybox: PASS (3.72 s) RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 JOB TIME : 5.32 s Due to a pending issue with the chardev console [*] , the Avocado framework sometime hangs, so meanwhile I run tests in a loop and they eventually succeed :S [*] https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01879.html To get the console, and filter uimage tests: $ tests/venv/bin/python -m avocado --show=app,console run -t binfmt:uimage tests/acceptance/machine_sparc_leon3.py JOB ID : b1fb8e8a101c6a45d2f15e57a9faafee94cdf2b5 JOB LOG: /home/phil/avocado/job-results/job-2019-06-27T14.41-b1fb8e8/job.log (1/1) tests/acceptance/machine_sparc_leon3.py:Leon3Machine.test_leon3_helenos_uimage: console: HelenOS bootloader, release 0.6.0 (Elastic Horse) console: Built on 2014-12-21 20:17:42 for sparc32 console: Copyright (c) 2001-2014 HelenOS project console: 0x4000bf20|0x4000bf20: kernel image (496640/128466 bytes) console: 0x4002b4f2|0x4002b4f2: ns image (154195/66444 bytes) console: 0x4003b87e|0x4003b87e: loader image (153182/66437 bytes) console: 0x4004bc03|0x4004bc03: init image (155339/66834 bytes) console: 0x4005c115|0x4005c115: locsrv image (162063/70267 bytes) console: 0x4006d390|0x4006d390: rd image (152678/65889 bytes) console: 0x4007d4f1|0x4007d4f1: vfs image (168480/73394 bytes) console: 0x4008f3a3|0x4008f3a3: logger image (158034/68368 bytes) console: 0x4009feb3|0x4009feb3: ext4fs image (234510/100301 bytes) console: 0x400b8680|0x400b8680: initrd image (8388608/1668901 bytes) console: ABMA devices: console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: <0:3000> at 0x irq 0 console: Memory size: 0 MB console: Inflating components ... initrd ext4fs logger vfs rd locsrv init loader ns kernel Booting the kernel ... PASS (1.11 s) RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 JOB TIME : 1.48 s The other test often timeouts due to [*]: $ tests/venv/bin/python -m avocado --show=app,console
Re: [Qemu-devel] [PATCH 0/3] tests/acceptance: Add tests for the Leon3 board
Hi Philippe, Thanks for that! I'm not aware at all of the tests/acceptance/* stuff.. How can we launch those tests? Appart of that it looks good to me :). Regards, Fred Le 6/27/19 à 1:53 PM, Philippe Mathieu-Daudé a écrit : Quick tests worth to avoid regressions, idea from https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04177.html "Maintainers, please tell us how to boot your machines" Regards, Phil. Philippe Mathieu-Daudé (3): tests/acceptance: Add test that boots the HelenOS microkernel on Leon3 tests/acceptance: Add test that boots Linux up to BusyBox on Leon3 .travis.yml: Let the avocado job run the Leon3 test .travis.yml | 2 +- MAINTAINERS | 1 + tests/acceptance/machine_sparc_leon3.py | 89 + 3 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/acceptance/machine_sparc_leon3.py
Re: [Qemu-devel] [PATCH 0/3] add ati vgabios
On Thu, 20 Jun 2019, Gerd Hoffmann wrote: Gerd Hoffmann (3): seabios: add config for ati vgabios seabios: add ati vgabios binary ati-vga: switch to vgabios-ati.bin hw/display/ati.c| 2 +- pc-bios/vgabios-ati.bin | Bin 0 -> 38912 bytes roms/config.vga-ati | 4 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 pc-bios/vgabios-ati.bin create mode 100644 roms/config.vga-ati Tested-by: BALATON Zoltan
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
Am 18.06.2019 um 09:38 hat Vladimir Sementsov-Ogievskiy geschrieben: > 17.06.2019 16:20, Kevin Wolf wrote: > > Am 17.06.2019 um 15:09 hat Eric Blake geschrieben: > >> On 6/17/19 7:09 AM, Kevin Wolf wrote: > >> > > > > Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called > > on nbd driver? > > I didn't implement it. But may be I should.. > > > > May aim was only to avoid extra allocation and unnecessary reads. But > > if we implement > > full-featured io request, what should it do? > > > > On qcow2 with backing it should pull data from backing to top, like in > > copy-on-read. > > And for nbd it will send NBD_CMD_CACHE? > > These semantics seems different, but why not? > > > > Any opinions here? Should I resend or could we use it as a first step, > not touching external API but improving things a little bit? > >>> > >>> I'm not opposed to making only a first step now. The interface seems to > >>> make sense to me; the only thing that I don't really like is the naming > >>> both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE) > >>> because to me, "cache" doesn't mean "read, but ignore the result". > >>> > >>> The operation only results in something being cached if the block graph > >>> is configured to cache things. And indeed, the most importatn use case > >>> for the flag is not populating a cache, but triggering copy-on-read. So > >>> I think calling this operation "cache" is misleading. > >>> > >>> Now, I don't have very good ideas for better names either. I guess > >>> BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if > >>> a blk_co_preadv_no_read wrapper is even needed when you can just call > >>> blk_co_preadv with the right flag.) > >>> > >>> I'm open for good naming ideas. > >> > >> Would 'prefetch' be a more useful name? The name NBD_CMD_CACHE was > >> chosen in the NBD project, but we are not stuck to that name if we think > >> something better makes more sense. > > > > Whether 'prefetch' is entirely accurate really depends on the graph > > configuration, too. But at least it gives me the right idea of "read > > something, but don't return it yet", so yes, I think that would work for > > me. > > Do you mean BDRV_REQ_PREFETCH + blk_co_pprefetch, or only the flag? > Hmm, doubled 'p' in blk_co_pprefetch looks strange, bit it should be > here for consistency with other requests.. I think I would only do the flag because the wrapper is so trivial, but this is a matter of taste. The kind of thing that is decided by whoever writes the patch. Kevin
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
17.06.2019 16:20, Kevin Wolf wrote: > Am 17.06.2019 um 15:09 hat Eric Blake geschrieben: >> On 6/17/19 7:09 AM, Kevin Wolf wrote: >> > > Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on > nbd driver? > I didn't implement it. But may be I should.. > > May aim was only to avoid extra allocation and unnecessary reads. But if > we implement > full-featured io request, what should it do? > > On qcow2 with backing it should pull data from backing to top, like in > copy-on-read. > And for nbd it will send NBD_CMD_CACHE? > These semantics seems different, but why not? > Any opinions here? Should I resend or could we use it as a first step, not touching external API but improving things a little bit? >>> >>> I'm not opposed to making only a first step now. The interface seems to >>> make sense to me; the only thing that I don't really like is the naming >>> both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE) >>> because to me, "cache" doesn't mean "read, but ignore the result". >>> >>> The operation only results in something being cached if the block graph >>> is configured to cache things. And indeed, the most importatn use case >>> for the flag is not populating a cache, but triggering copy-on-read. So >>> I think calling this operation "cache" is misleading. >>> >>> Now, I don't have very good ideas for better names either. I guess >>> BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if >>> a blk_co_preadv_no_read wrapper is even needed when you can just call >>> blk_co_preadv with the right flag.) >>> >>> I'm open for good naming ideas. >> >> Would 'prefetch' be a more useful name? The name NBD_CMD_CACHE was >> chosen in the NBD project, but we are not stuck to that name if we think >> something better makes more sense. > > Whether 'prefetch' is entirely accurate really depends on the graph > configuration, too. But at least it gives me the right idea of "read > something, but don't return it yet", so yes, I think that would work for > me. > Do you mean BDRV_REQ_PREFETCH + blk_co_pprefetch, or only the flag? Hmm, doubled 'p' in blk_co_pprefetch looks strange, bit it should be here for consistency with other requests.. -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH 0/3] Some qemu-bridge-helper work
On 2019/6/4 下午7:52, Markus Armbruster wrote: Markus Armbruster (3): MAINTAINERS: Add qemu-bridge-helper.c to "Network device backends" net: Deprecate tap backend's parameter "helper" qemu-bridge-helper: Document known shortcomings MAINTAINERS | 1 + qapi/net.json| 3 ++- qemu-bridge-helper.c | 12 +++- qemu-deprecated.texi | 4 qemu-options.hx | 18 ++ 5 files changed, 20 insertions(+), 18 deletions(-) I've queued patch 1 and 3. For patch 2, it still require more thought since tap is not tied to bridge in fact, it could be used independently. Thanks
Re: [Qemu-devel] [PATCH 0/3] tricore: Convert to translate_loop
Patchew URL: https://patchew.org/QEMU/20190617143533.15013-1-kbast...@mail.uni-paderborn.de/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20190617143533.15013-1-kbast...@mail.uni-paderborn.de Type: series Subject: [Qemu-devel] [PATCH 0/3] tricore: Convert to translate_loop === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === From https://github.com/patchew-project/qemu * [new tag] patchew/20190617143533.15013-1-kbast...@mail.uni-paderborn.de -> patchew/20190617143533.15013-1-kbast...@mail.uni-paderborn.de Switched to a new branch 'test' 3795cef target/tricore: Use translate_loop d50dce9 target-tricore: Make env a member of DisasContext 6d30fd1 target/tricore: Use DisasContextBase API === OUTPUT BEGIN === 1/3 Checking commit 6d30fd14d6c8 (target/tricore: Use DisasContextBase API) 2/3 Checking commit d50dce928c26 (target-tricore: Make env a member of DisasContext) ERROR: spaces required around that '+' (ctx:VxV) #660: FILE: target/tricore/translate.c:6586: +gen_dvinit_b(ctx, cpu_gpr_d[r3], cpu_gpr_d[r3+1], cpu_gpr_d[r1], ^ ERROR: spaces required around that '+' (ctx:VxV) #678: FILE: target/tricore/translate.c:6619: +gen_dvinit_h(ctx, cpu_gpr_d[r3], cpu_gpr_d[r3+1], cpu_gpr_d[r1], ^ total: 2 errors, 0 warnings, 1154 lines checked Patch 2/3 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/3 Checking commit 3795cefb9ed0 (target/tricore: Use translate_loop) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190617143533.15013-1-kbast...@mail.uni-paderborn.de/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
Am 17.06.2019 um 15:09 hat Eric Blake geschrieben: > On 6/17/19 7:09 AM, Kevin Wolf wrote: > > >>> > >>> Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on > >>> nbd driver? > >>> I didn't implement it. But may be I should.. > >>> > >>> May aim was only to avoid extra allocation and unnecessary reads. But if > >>> we implement > >>> full-featured io request, what should it do? > >>> > >>> On qcow2 with backing it should pull data from backing to top, like in > >>> copy-on-read. > >>> And for nbd it will send NBD_CMD_CACHE? > >>> These semantics seems different, but why not? > >>> > >> > >> Any opinions here? Should I resend or could we use it as a first step, > >> not touching external API but improving things a little bit? > > > > I'm not opposed to making only a first step now. The interface seems to > > make sense to me; the only thing that I don't really like is the naming > > both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE) > > because to me, "cache" doesn't mean "read, but ignore the result". > > > > The operation only results in something being cached if the block graph > > is configured to cache things. And indeed, the most importatn use case > > for the flag is not populating a cache, but triggering copy-on-read. So > > I think calling this operation "cache" is misleading. > > > > Now, I don't have very good ideas for better names either. I guess > > BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if > > a blk_co_preadv_no_read wrapper is even needed when you can just call > > blk_co_preadv with the right flag.) > > > > I'm open for good naming ideas. > > Would 'prefetch' be a more useful name? The name NBD_CMD_CACHE was > chosen in the NBD project, but we are not stuck to that name if we think > something better makes more sense. Whether 'prefetch' is entirely accurate really depends on the graph configuration, too. But at least it gives me the right idea of "read something, but don't return it yet", so yes, I think that would work for me. Kevin signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
vandersonmr writes: > This is the first series of patches related to the TCGCodeQuality GSoC project > More at https://wiki.qemu.org/Features/TCGCodeQuality > > It adds an option to instrument TBs and collects their execution frequency. > The execution frequency is then store/accumulated in an auxiliary structure > every time a tb_flush or a read happens. > > [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter. > [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events. > [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user. One more thing: https://app.shippable.com/github/stsquad/qemu/runs/866/summary/console The use of: uint64_t exec_freq; breaks 32 bit builds as we violate ATOMIC_REG_SIZE. Maybe we can get away with uint32_t? I guess we need more of an idea of the range of these counters are likely to hit (and maybe detect overflow in the helper?). -- Alex Bennée
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
On 6/17/19 7:09 AM, Kevin Wolf wrote: >>> >>> Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on >>> nbd driver? >>> I didn't implement it. But may be I should.. >>> >>> May aim was only to avoid extra allocation and unnecessary reads. But if we >>> implement >>> full-featured io request, what should it do? >>> >>> On qcow2 with backing it should pull data from backing to top, like in >>> copy-on-read. >>> And for nbd it will send NBD_CMD_CACHE? >>> These semantics seems different, but why not? >>> >> >> Any opinions here? Should I resend or could we use it as a first step, >> not touching external API but improving things a little bit? > > I'm not opposed to making only a first step now. The interface seems to > make sense to me; the only thing that I don't really like is the naming > both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE) > because to me, "cache" doesn't mean "read, but ignore the result". > > The operation only results in something being cached if the block graph > is configured to cache things. And indeed, the most importatn use case > for the flag is not populating a cache, but triggering copy-on-read. So > I think calling this operation "cache" is misleading. > > Now, I don't have very good ideas for better names either. I guess > BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if > a blk_co_preadv_no_read wrapper is even needed when you can just call > blk_co_preadv with the right flag.) > > I'm open for good naming ideas. Would 'prefetch' be a more useful name? The name NBD_CMD_CACHE was chosen in the NBD project, but we are not stuck to that name if we think something better makes more sense. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
17.06.2019 15:09, Kevin Wolf wrote: > Am 17.06.2019 um 13:20 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 06.06.2019 17:07, Vladimir Sementsov-Ogievskiy wrote: >>> 06.06.2019 16:55, Eric Blake wrote: On 6/6/19 8:48 AM, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > Here is small new io API: blk_co_pcache, which does copy-on-read without > extra buffer for read data. This means that only parts that needs COR > will be actually read and only corresponding buffers allocated, no more. > > This allows to improve a bit block-stream and NBD_CMD_CACHE I'd really like to see qemu-io gain access to calling this command, so that we can add iotests coverage of this new feature. Note that the in-development libnbd (https://github.com/libguestfs/libnbd/commits/master) is also usable as an NBD client that can drive NBD_CMD_CACHE, although it's still new enough that we probably don't want to rely on it being available yet. >>> >>> Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on >>> nbd driver? >>> I didn't implement it. But may be I should.. >>> >>> May aim was only to avoid extra allocation and unnecessary reads. But if we >>> implement >>> full-featured io request, what should it do? >>> >>> On qcow2 with backing it should pull data from backing to top, like in >>> copy-on-read. >>> And for nbd it will send NBD_CMD_CACHE? >>> These semantics seems different, but why not? >>> >> >> Any opinions here? Should I resend or could we use it as a first step, >> not touching external API but improving things a little bit? > > I'm not opposed to making only a first step now. The interface seems to > make sense to me; the only thing that I don't really like is the naming > both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE) > because to me, "cache" doesn't mean "read, but ignore the result". > > The operation only results in something being cached if the block graph > is configured to cache things. And indeed, the most importatn use case > for the flag is not populating a cache, but triggering copy-on-read. So > I think calling this operation "cache" is misleading. > > Now, I don't have very good ideas for better names either. I guess > BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if > a blk_co_preadv_no_read wrapper is even needed when you can just call > blk_co_preadv with the right flag.) > > I'm open for good naming ideas. > My first try (not published) was BDRV_REQ_FAKE_READ, passed as flag to blk_co_preadv, without separate io request function. I decided to make it to be Cache request inspired by NBD_CMD_CACHE, which was created to do exactly copy-on-read operation. So if we call it cache it will correspond to NBD protocol. _NO_DATA also works for me, not a problem to resend with this flag and without additional wrapper, as a first step. -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
Am 17.06.2019 um 13:20 hat Vladimir Sementsov-Ogievskiy geschrieben: > 06.06.2019 17:07, Vladimir Sementsov-Ogievskiy wrote: > > 06.06.2019 16:55, Eric Blake wrote: > >> On 6/6/19 8:48 AM, Vladimir Sementsov-Ogievskiy wrote: > >>> Hi all! > >>> > >>> Here is small new io API: blk_co_pcache, which does copy-on-read without > >>> extra buffer for read data. This means that only parts that needs COR > >>> will be actually read and only corresponding buffers allocated, no more. > >>> > >>> This allows to improve a bit block-stream and NBD_CMD_CACHE > >> > >> I'd really like to see qemu-io gain access to calling this command, so > >> that we can add iotests coverage of this new feature. Note that the > >> in-development libnbd > >> (https://github.com/libguestfs/libnbd/commits/master) is also usable as > >> an NBD client that can drive NBD_CMD_CACHE, although it's still new > >> enough that we probably don't want to rely on it being available yet. > >> > > > > Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on > > nbd driver? > > I didn't implement it. But may be I should.. > > > > May aim was only to avoid extra allocation and unnecessary reads. But if we > > implement > > full-featured io request, what should it do? > > > > On qcow2 with backing it should pull data from backing to top, like in > > copy-on-read. > > And for nbd it will send NBD_CMD_CACHE? > > These semantics seems different, but why not? > > > > Any opinions here? Should I resend or could we use it as a first step, > not touching external API but improving things a little bit? I'm not opposed to making only a first step now. The interface seems to make sense to me; the only thing that I don't really like is the naming both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE) because to me, "cache" doesn't mean "read, but ignore the result". The operation only results in something being cached if the block graph is configured to cache things. And indeed, the most importatn use case for the flag is not populating a cache, but triggering copy-on-read. So I think calling this operation "cache" is misleading. Now, I don't have very good ideas for better names either. I guess BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if a blk_co_preadv_no_read wrapper is even needed when you can just call blk_co_preadv with the right flag.) I'm open for good naming ideas. Kevin
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
06.06.2019 17:07, Vladimir Sementsov-Ogievskiy wrote: > 06.06.2019 16:55, Eric Blake wrote: >> On 6/6/19 8:48 AM, Vladimir Sementsov-Ogievskiy wrote: >>> Hi all! >>> >>> Here is small new io API: blk_co_pcache, which does copy-on-read without >>> extra buffer for read data. This means that only parts that needs COR >>> will be actually read and only corresponding buffers allocated, no more. >>> >>> This allows to improve a bit block-stream and NBD_CMD_CACHE >> >> I'd really like to see qemu-io gain access to calling this command, so >> that we can add iotests coverage of this new feature. Note that the >> in-development libnbd >> (https://github.com/libguestfs/libnbd/commits/master) is also usable as >> an NBD client that can drive NBD_CMD_CACHE, although it's still new >> enough that we probably don't want to rely on it being available yet. >> > > Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on nbd > driver? > I didn't implement it. But may be I should.. > > May aim was only to avoid extra allocation and unnecessary reads. But if we > implement > full-featured io request, what should it do? > > On qcow2 with backing it should pull data from backing to top, like in > copy-on-read. > And for nbd it will send NBD_CMD_CACHE? > These semantics seems different, but why not? > Any opinions here? Should I resend or could we use it as a first step, not touching external API but improving things a little bit? -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
vandersonmr writes: > This is the first series of patches related to the TCGCodeQuality GSoC project > More at https://wiki.qemu.org/Features/TCGCodeQuality > > It adds an option to instrument TBs and collects their execution frequency. > The execution frequency is then store/accumulated in an auxiliary structure > every time a tb_flush or a read happens. > > [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter. > [Qemu-Devel][PATCH 2/3] Saving counters between tb_flush events. > [Qemu-Devel][PATCH 3/3] Adding command line option to linux-user. Did you explicitly set the patch prefix to qemu-devel? You don't need to, they are added by the mailing list software. -- Alex Bennée
Re: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
Patchew URL:
https://patchew.org/QEMU/20190614135332.12777-1-vanderson...@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 0/3] Collecting TB Execution Frequency
Type: series
Message-id: 20190614135332.12777-1-vanderson...@gmail.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update]
patchew/155912548463.2019004.3515830305299809902.st...@bahia.lan ->
patchew/155912548463.2019004.3515830305299809902.st...@bahia.lan
- [tag update]
patchew/156051774276.244890.8660277280145466396.st...@bahia.lan ->
patchew/156051774276.244890.8660277280145466396.st...@bahia.lan
Switched to a new branch 'test'
6512387 Adding command line option to linux-user.
af52efd Saving counters between tb_flush events.
906c8ef Adding an optional tb execution counter.
=== OUTPUT BEGIN ===
1/3 Checking commit 906c8ef92aac (Adding an optional tb execution counter.)
ERROR: "(foo*)" should be "(foo *)"
#24: FILE: accel/tcg/tcg-runtime.c:173:
+TranslationBlock* tb = (TranslationBlock*) ptr;
ERROR: do not initialise globals to 0 or NULL
#83: FILE: linux-user/main.c:61:
+bool enable_freq_count = false;
ERROR: do not initialise globals to 0 or NULL
#95: FILE: vl.c:193:
+bool enable_freq_count = false;
total: 3 errors, 0 warnings, 56 lines checked
Patch 1/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit af52efd7842d (Saving counters between tb_flush events.)
ERROR: "foo* bar" should be "foo *bar"
#22: FILE: accel/tcg/translate-all.c:1121:
+static bool statistics_cmp(const void* ap, const void *bp) {
ERROR: open brace '{' following function declarations go on the next line
#22: FILE: accel/tcg/translate-all.c:1121:
+static bool statistics_cmp(const void* ap, const void *bp) {
ERROR: line over 90 characters
#35: FILE: accel/tcg/translate-all.c:1146:
+qht_init(_ctx.tb_statistics, statistics_cmp, CODE_GEN_HTABLE_SIZE,
QHT_MODE_AUTO_RESIZE);
ERROR: "(foo*)" should be "(foo *)"
#70: FILE: accel/tcg/translate-all.c:1265:
+uint64_t exec_freq = tb_get_and_reset_exec_freq((TranslationBlock*) p);
ERROR: "(foo*)" should be "(foo *)"
#190: FILE: include/qom/cpu.h:477:
+uint64_t tb_get_and_reset_exec_freq(struct TranslationBlock*);
total: 5 errors, 0 warnings, 146 lines checked
Patch 2/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/3 Checking commit 651238793473 (Adding command line option to linux-user.)
ERROR: externs should be avoided in .c files
#22: FILE: linux-user/exit.c:29:
+extern bool enable_freq_count;
total: 1 errors, 0 warnings, 32 lines checked
Patch 3/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190614135332.12777-1-vanderson...@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
06.06.2019 16:55, Eric Blake wrote: > On 6/6/19 8:48 AM, Vladimir Sementsov-Ogievskiy wrote: >> Hi all! >> >> Here is small new io API: blk_co_pcache, which does copy-on-read without >> extra buffer for read data. This means that only parts that needs COR >> will be actually read and only corresponding buffers allocated, no more. >> >> This allows to improve a bit block-stream and NBD_CMD_CACHE > > I'd really like to see qemu-io gain access to calling this command, so > that we can add iotests coverage of this new feature. Note that the > in-development libnbd > (https://github.com/libguestfs/libnbd/commits/master) is also usable as > an NBD client that can drive NBD_CMD_CACHE, although it's still new > enough that we probably don't want to rely on it being available yet. > Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on nbd driver? I didn't implement it. But may be I should.. May aim was only to avoid extra allocation and unnecessary reads. But if we implement full-featured io request, what should it do? On qcow2 with backing it should pull data from backing to top, like in copy-on-read. And for nbd it will send NBD_CMD_CACHE? These semantics seems different, but why not? -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache
On 6/6/19 8:48 AM, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > Here is small new io API: blk_co_pcache, which does copy-on-read without > extra buffer for read data. This means that only parts that needs COR > will be actually read and only corresponding buffers allocated, no more. > > This allows to improve a bit block-stream and NBD_CMD_CACHE I'd really like to see qemu-io gain access to calling this command, so that we can add iotests coverage of this new feature. Note that the in-development libnbd (https://github.com/libguestfs/libnbd/commits/master) is also usable as an NBD client that can drive NBD_CMD_CACHE, although it's still new enough that we probably don't want to rely on it being available yet. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/3] hw/microblaze: Kconfig cleanup
On 27/05/19 18:10, Philippe Mathieu-Daudé wrote: >>> >> Queued, thanks. > I don't see these patches in your queue merged on "Fri 17 May", are > you planning to include them in your next batch? Yes, thanks. Paolo
Re: [Qemu-devel] [PATCH 0/3] hw/microblaze: Kconfig cleanup
Hi Paolo, On Tue, Apr 30, 2019 at 9:55 PM Paolo Bonzini wrote: > > On 27/04/19 16:14, Philippe Mathieu-Daudé wrote: > > Hi Edgar, Peter, > > > > Few fixes while cleaning Kconfig, trying to optimize builds. > > > > Regards, > > > > Phil. > > > > Philippe Mathieu-Daudé (3): > > hw/Kconfig: Move the generic XLNX_ZYNQMP to the root hw/Kconfig > > hw/intc: Only build the xlnx-iomod-intc device for the MicroBlaze PMU > > hw/dma: Do not build the xlnx_dpdma device for the MicroBlaze machines > > > > hw/Kconfig| 3 +++ > > hw/dma/Makefile.objs | 1 - > > hw/intc/Makefile.objs | 2 +- > > hw/timer/Kconfig | 3 --- > > 4 files changed, 4 insertions(+), 5 deletions(-) > > > > Queued, thanks. I don't see these patches in your queue merged on "Fri 17 May", are you planning to include them in your next batch? Thanks, Phil.
Re: [Qemu-devel] [PATCH 0/3] arm: Clean up and rename hw/arm/arm.h to hw/arm/boot.h
On 5/16/19 6:38 PM, Peter Maydell wrote: > The header hw/arm/arm.h used to be a general bucket for > putting all kinds of arm-related declarations in. It now > has mostly kernel-boot related declarations, with one > exception: the declaration of the system_clock_scale global. > This patchset: > * moves system_clock_scale to armv7m_systick.h (since that >is the only device that uses it) > * deletes some unnecessary #includes of hw/arm/arm.h > * renames it to hw/arm/boot.h, since it now only has >declarations relating to hw/arm/boot.c functionality Yay \o/ > Since system_clock_scale is a weird thing, I have included > in the first patch an expansion of the comment describing > it to be clearer about what it does, and also a TODO note > sketching out how we could go about eradicating this global. > > thanks > -- PMM > > Peter Maydell (3): > arm: Move system_clock_scale to armv7m_systick.h > arm: Remove unnecessary includes of hw/arm/arm.h > arm: Rename hw/arm/arm.h to hw/arm/boot.h Series: Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé
Re: [Qemu-devel] [PATCH 0/3] Optimize COLO related codes and description
* Zhang Chen (chen.zh...@intel.com) wrote: > From: Zhang Chen > > In this series we optimize codes and fix some tiny issues. Queued > > Zhang Chen (3): > migration/colo.c: Remove redundant input parameter > migration/colo.h: Remove obsolete codes > qemu-option.hx: Update missed parameter for colo-compare > > include/migration/colo.h | 4 +--- > migration/colo-failover.c | 2 +- > migration/colo.c | 2 +- > qemu-options.hx | 9 ++--- > 4 files changed, 9 insertions(+), 8 deletions(-) > > -- > 2.17.GIT > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] Optimize COLO related codes and description
* Zhang, Chen (chen.zh...@intel.com) wrote: > Hi Dave, > > I noticed that you have reviewed all the patches in this series, can you > queue it? Yes, I'm about to start a migration pull now. Dave > Thanks > Zhang Chen > > > > -Original Message- > > From: Zhang, Chen > > Sent: Friday, April 26, 2019 5:07 PM > > To: Laurent Vivier ; Dr. David Alan Gilbert > > ; Juan Quintela ; zhanghailiang > > ; Markus Armbruster > > ; qemu-dev > > Cc: Zhang Chen ; Zhang, Chen > > Subject: [PATCH 0/3] Optimize COLO related codes and description > > > > From: Zhang Chen > > > > In this series we optimize codes and fix some tiny issues. > > > > Zhang Chen (3): > > migration/colo.c: Remove redundant input parameter > > migration/colo.h: Remove obsolete codes > > qemu-option.hx: Update missed parameter for colo-compare > > > > include/migration/colo.h | 4 +--- > > migration/colo-failover.c | 2 +- > > migration/colo.c | 2 +- > > qemu-options.hx | 9 ++--- > > 4 files changed, 9 insertions(+), 8 deletions(-) > > > > -- > > 2.17.GIT > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
Eduardo Habkost writes: > On Fri, May 10, 2019 at 11:29:53AM +0200, Markus Armbruster wrote: > [...] >> >> >> > We could extend QAPI introspection to return that if necessary, >> >> >> > right? >> >> >> >> >> >> I'm confident we can come up with *something*. It might kill the neat >> >> >> and simple "use QAPI features to communicate deprecation" idea, though. >> >> > >> >> > If something is important enough to be communicated through >> >> > stderr, it's important enough to be communicated through QMP. >> >> >> >> Mostly. Differences are due to the different consumers. >> >> >> >> stderr is primarily for human users. We print stuff useful to human >> >> users. >> > >> > We have users that don't have access to stderr. They might have >> > access to log files, but log files are pretty bad user >> > interfaces. If it's important for some set of human users, apps >> > using libvirt or QMP need access to that information so they can >> > show it to their human users too. >> >> Command line means stderr. > > I disagree. > >> I'm afraid our command line is awkward both for machines and for humans, >> albeit for different reasons. >> >> For humans doing simple things, the command line is okay. But beyond >> that, it gets forbiddingly unwieldy[2]. >> >> Machines are fine with that kind of unwieldy, but would prefer something >> with more structure, both on input (talking to QEMU) and even more so on >> output (QEMU talking back). >> >> Ideally, we'd support machines do their work in (structured) QMP, >> resorting to the command line only to set up a QMP monitor. We're not >> anywhere close to this. >> >> As long as management applications use the command line in not-trivial >> ways, they have to deal with configuration errors reported via stderr. > > That's only true if we want to. > > Command line is an interface usable by machines. Not the ideal, > but it works. > > Messages on stderr are not an interface for machines. We must > provide something better, and I don't think "wait until we > convert everything to QMP" is a reasonable answer. Where else do you propose to report command line errors? In what format? >> >> QMP is primarily for machines, secondarily for the humans building these >> >> machines. We send stuff useful to the machines themselves, and stuff >> >> the machines can use to be more useful for their users (which may be >> >> machines or humans). We can also send stuff to help the humans building >> >> the machines. >> >> >> >> In any case, the information we provide is limited by the cost to >> >> provide it. >> > >> > Absolutely. >> > >> >> >> >> > Is that enough reason to provide something more complex? >> >> >> >> We need to consider cost / benefit. >> >> >> >> On benefit, I'd like to know what libvirt would do with the additional >> >> information beyond logging it. >> > >> > I'd say it should provide it to apps, otherwise this won't be >> > more useful than the existing log files. >> >> A management application simply showing its user whatever error QEMU >> reports or hint it provides is bound to be confusing: since QEMU talks >> in QEMU terms, its errors and hints generally need translation to make >> sense at higher layers. Translation involves recognizing specific >> messages, which means it's limited to special cases (and painfully >> brittle). >> >> The farther you propagate QEMU's messages up the stack, the less sense >> they'll likely make. >> >> Management applications logging QEMU's messages is useful anyway, mainly >> because it's better than nothing. >> >> I doubt logging them some more further up the stack would be all that >> useful, but I might be wrong. >> >> Discussed further elsewhere in this thread. >> >> >> Is the additional information you propose to provide static or dynamic? >> >> >> >> By "static", I mean each occurence of a feature in the QAPI schema is >> >> tied to one fixed instance of "additional information". >> > >> > I don't think I understand this description of "static". I >> > expect the data to be fixed at build time, but I expect it to be >> > different in downstream distributions of QEMU. >> >> Let me try differently. >> >> QAPI features as currently envisaged convey one bit of information: >> there / not there. The information is fixed at build time. It is tied >> to a specific QAPI entity (command, object type, enumeration value, >> ...). >> >> My question is about the difference between this and what you have in >> mind. Specifically, is the difference only the amount of information >> (one bit vs. a pair of string literals), or is there more? > > Right now, it's only in the amount of information. > >> >> "More" includes string values that can vary at run time or between >> different uses of the QAPI entity in the schema. > > Right now, it includes string values that are fixed at build > time, but collected dynamically at run time (because we are > describing QOM types, which are collected dynamically).
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
On Fri, May 10, 2019 at 02:17:11PM -0300, Eduardo Habkost wrote: > On Fri, May 10, 2019 at 11:29:53AM +0200, Markus Armbruster wrote: > [...] > > I'm afraid our command line is awkward both for machines and for humans, > > albeit for different reasons. > > > > For humans doing simple things, the command line is okay. But beyond > > that, it gets forbiddingly unwieldy[2]. > > > > Machines are fine with that kind of unwieldy, but would prefer something > > with more structure, both on input (talking to QEMU) and even more so on > > output (QEMU talking back). > > > > Ideally, we'd support machines do their work in (structured) QMP, > > resorting to the command line only to set up a QMP monitor. We're not > > anywhere close to this. > > > > As long as management applications use the command line in not-trivial > > ways, they have to deal with configuration errors reported via stderr. > > That's only true if we want to. > > Command line is an interface usable by machines. Not the ideal, > but it works. > > Messages on stderr are not an interface for machines. We must > provide something better, and I don't think "wait until we > convert everything to QMP" is a reasonable answer. If QEMU successfully starts then libvirt essentially ignores stderr just letting it go to the logfile. If we see any deprecated features used during startup the natural thing would be to queue up a list of warnings, and then once the client (libvirt) connects to QMP emit them as events. QEMU still ought to emit them on stderr anyway so they do end up in the logs regardless of whether anyone actually procsses the deprecation QMP events. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
On Fri, May 10, 2019 at 11:29:53AM +0200, Markus Armbruster wrote: [...] > >> >> > We could extend QAPI introspection to return that if necessary, > >> >> > right? > >> >> > >> >> I'm confident we can come up with *something*. It might kill the neat > >> >> and simple "use QAPI features to communicate deprecation" idea, though. > >> > > >> > If something is important enough to be communicated through > >> > stderr, it's important enough to be communicated through QMP. > >> > >> Mostly. Differences are due to the different consumers. > >> > >> stderr is primarily for human users. We print stuff useful to human > >> users. > > > > We have users that don't have access to stderr. They might have > > access to log files, but log files are pretty bad user > > interfaces. If it's important for some set of human users, apps > > using libvirt or QMP need access to that information so they can > > show it to their human users too. > > Command line means stderr. I disagree. > > I'm afraid our command line is awkward both for machines and for humans, > albeit for different reasons. > > For humans doing simple things, the command line is okay. But beyond > that, it gets forbiddingly unwieldy[2]. > > Machines are fine with that kind of unwieldy, but would prefer something > with more structure, both on input (talking to QEMU) and even more so on > output (QEMU talking back). > > Ideally, we'd support machines do their work in (structured) QMP, > resorting to the command line only to set up a QMP monitor. We're not > anywhere close to this. > > As long as management applications use the command line in not-trivial > ways, they have to deal with configuration errors reported via stderr. That's only true if we want to. Command line is an interface usable by machines. Not the ideal, but it works. Messages on stderr are not an interface for machines. We must provide something better, and I don't think "wait until we convert everything to QMP" is a reasonable answer. > > >> QMP is primarily for machines, secondarily for the humans building these > >> machines. We send stuff useful to the machines themselves, and stuff > >> the machines can use to be more useful for their users (which may be > >> machines or humans). We can also send stuff to help the humans building > >> the machines. > >> > >> In any case, the information we provide is limited by the cost to > >> provide it. > > > > Absolutely. > > > >> > >> > Is that enough reason to provide something more complex? > >> > >> We need to consider cost / benefit. > >> > >> On benefit, I'd like to know what libvirt would do with the additional > >> information beyond logging it. > > > > I'd say it should provide it to apps, otherwise this won't be > > more useful than the existing log files. > > A management application simply showing its user whatever error QEMU > reports or hint it provides is bound to be confusing: since QEMU talks > in QEMU terms, its errors and hints generally need translation to make > sense at higher layers. Translation involves recognizing specific > messages, which means it's limited to special cases (and painfully > brittle). > > The farther you propagate QEMU's messages up the stack, the less sense > they'll likely make. > > Management applications logging QEMU's messages is useful anyway, mainly > because it's better than nothing. > > I doubt logging them some more further up the stack would be all that > useful, but I might be wrong. > > Discussed further elsewhere in this thread. > > >> Is the additional information you propose to provide static or dynamic? > >> > >> By "static", I mean each occurence of a feature in the QAPI schema is > >> tied to one fixed instance of "additional information". > > > > I don't think I understand this description of "static". I > > expect the data to be fixed at build time, but I expect it to be > > different in downstream distributions of QEMU. > > Let me try differently. > > QAPI features as currently envisaged convey one bit of information: > there / not there. The information is fixed at build time. It is tied > to a specific QAPI entity (command, object type, enumeration value, > ...). > > My question is about the difference between this and what you have in > mind. Specifically, is the difference only the amount of information > (one bit vs. a pair of string literals), or is there more? Right now, it's only in the amount of information. > > "More" includes string values that can vary at run time or between > different uses of the QAPI entity in the schema. Right now, it includes string values that are fixed at build time, but collected dynamically at run time (because we are describing QOM types, which are collected dynamically). > > >> > Do we need QAPI features to be just strings? Can't they be a > >> > more complex type, like a QAPI alternate? > >> > >> Adds complexity. > >> > >> We currently imagine QAPI features enum-like, i.e. a list of strings, >
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
On Fri, May 10, 2019 at 08:19:52AM +0200, Markus Armbruster wrote: > Eduardo Habkost writes: > > > On Thu, May 09, 2019 at 10:14:52AM +0100, Daniel P. Berrangé wrote: > >> On Thu, May 09, 2019 at 10:31:46AM +0200, Markus Armbruster wrote: > >> > We've wandered into the QAPI vs. QOM swamp. Cc: Paolo. > >> > > >> > Eduardo Habkost writes: > >> > > >> > > On Wed, May 08, 2019 at 11:16:50AM +0200, Markus Armbruster wrote: > [...] > >> > >> I agree we should point to a preferred replacement whenever we > >> > >> deprecate > >> > >> something. > >> > >> > >> > >> We have to do it in documentation. And we generally do, in > >> > >> qemu-deprecated.texi. > >> > >> > >> > >> How useful would doing it in QMP as well be? Depends on what > >> > >> management > >> > >> applications can do with the additional information. > >> > > > >> > > I expect it to be useful for things that have obvious > >> > > replacements, like old machine type or CPU model versions. > >> > > >> > I doubt a management application should apply suggested replacements > >> > automatically, and I doubt libvirt would. Not even when QEMU developers > >> > deem them "obvious". > >> > >> We certainly won't apply the suggested replacement as in many cases > >> it is not going to be a functionally equivalent drop-in. > > > > Who's "we"? > > > >> > >> If QEMU logs it to stderr, it will end up in the per-VM log file > >> libvirt has under /var/log/libvirt/qemu/$GUESTNAME.log. If QEMU > >> doesn't log it to stderr, then libvirt would just write it to > >> that same log file itself. > >> > >> If libvirt gains some API or event for notifying apps of deprecation > >> we might bubble it up to the mgmt app that way. > >> > >> I still feel it is useful to have the suggested replacement in the > >> logs, rather than only leaving it in qemu-deprecated.texi. This > >> way the info is immediately visible to both app developers and any > >> support person dealing with bugs. > >> > >> If the app dev see the suggested replacement upfront they're more > >> likely to make an immediate decision to update their code if the > >> suggestion is trivial. If they need to go find the QEMU docs to > >> lookup what action is required I feel they'll more likely just > >> put the item on their long todo list where it will languish. > > > > Agreed. However, note that the audience for deprecation > > information is not just developers and support people. End users > > need to know when they are relying on a deprecated feature, and > > applications should make it as easy as possible for them to > > update their configurations. > > > > I'm not suggesting the alternative would be applied > > automatically. But having the alternative available in a > > machine-friendly way may be the difference between a unhelpful UI > > that just tells the user there's some problem but can't give a > > solution, and one that can really assist the user to fix the > > problem. > > I'm skeptical. > > For the management application to assist its users, it has to translate > both the deprecated QEMU interface and its replacement into its own > interfaces (because those are the ones the users actually use). > Management applications routinely translate in the other direction. I > doubt anyone would build reverse translation capabilities just for > helping users update deprecated configurations. So unless such > capabilities get built for other purposes, machine-friendliness will > remain unused. > > If the management application's user is another machine, another > translation is needed. And so forth until we reach the guy who's > supposed to update configuration. > > Such a game of telephone is unlikely to produce anything but confusion, > except for specific cases we test across the whole stack. I don't see how that applies to machine type and CPU model names. Machine type and CPU model names are often exposed directly to the end user. -- Eduardo
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
On Fri, May 10, 2019 at 08:28:04AM +0200, Markus Armbruster wrote: > Eduardo Habkost writes: > > > On Thu, May 09, 2019 at 05:08:11PM +0100, Daniel P. Berrangé wrote: > >> On Thu, May 09, 2019 at 12:52:47PM -0300, Eduardo Habkost wrote: > >> > On Thu, May 09, 2019 at 10:14:52AM +0100, Daniel P. Berrangé wrote: > >> > > On Thu, May 09, 2019 at 10:31:46AM +0200, Markus Armbruster wrote: > >> > > > We've wandered into the QAPI vs. QOM swamp. Cc: Paolo. > >> > > > > >> > > > Eduardo Habkost writes: > >> > > > > >> > > > > On Wed, May 08, 2019 at 11:16:50AM +0200, Markus Armbruster wrote: > [...] > >> > > > >> I agree we should point to a preferred replacement whenever we > >> > > > >> deprecate > >> > > > >> something. > >> > > > >> > >> > > > >> We have to do it in documentation. And we generally do, in > >> > > > >> qemu-deprecated.texi. > >> > > > >> > >> > > > >> How useful would doing it in QMP as well be? Depends on what > >> > > > >> management > >> > > > >> applications can do with the additional information. > >> > > > > > >> > > > > I expect it to be useful for things that have obvious > >> > > > > replacements, like old machine type or CPU model versions. > >> > > > > >> > > > I doubt a management application should apply suggested replacements > >> > > > automatically, and I doubt libvirt would. Not even when QEMU > >> > > > developers > >> > > > deem them "obvious". > >> > > > >> > > We certainly won't apply the suggested replacement as in many cases > >> > > it is not going to be a functionally equivalent drop-in. > >> > > >> > Who's "we"? > >> > >> I was refering to libvirt here. > >> > >> > > If QEMU logs it to stderr, it will end up in the per-VM log file > >> > > libvirt has under /var/log/libvirt/qemu/$GUESTNAME.log. If QEMU > >> > > doesn't log it to stderr, then libvirt would just write it to > >> > > that same log file itself. > >> > > > >> > > If libvirt gains some API or event for notifying apps of deprecation > >> > > we might bubble it up to the mgmt app that way. > >> > > > >> > > I still feel it is useful to have the suggested replacement in the > >> > > logs, rather than only leaving it in qemu-deprecated.texi. This > >> > > way the info is immediately visible to both app developers and any > >> > > support person dealing with bugs. > >> > > > >> > > If the app dev see the suggested replacement upfront they're more > >> > > likely to make an immediate decision to update their code if the > >> > > suggestion is trivial. If they need to go find the QEMU docs to > >> > > lookup what action is required I feel they'll more likely just > >> > > put the item on their long todo list where it will languish. > >> > > >> > Agreed. However, note that the audience for deprecation > >> > information is not just developers and support people. End users > >> > need to know when they are relying on a deprecated feature, and > >> > applications should make it as easy as possible for them to > >> > update their configurations. > >> > > >> > I'm not suggesting the alternative would be applied > >> > automatically. But having the alternative available in a > >> > machine-friendly way may be the difference between a unhelpful UI > >> > that just tells the user there's some problem but can't give a > >> > solution, and one that can really assist the user to fix the > >> > problem. > >> > >> For some aspects of QEMU it might be possible, but considering the > >> broader set of things which can be deprecated, I don't think it is > >> possible to expose a machine consumable "suggestion". > >> > >> Consider the deprecation of the ACL feature. We deprecated monitor > >> commands "acl_add", "acl_policy", "acl_reset", etc. The suggested > >> replacement is to use one of the many possible QAuthZ types combined > >> with the -object arg. Even if we invented some way to express this > >> in the schema, I don't think any app would usefully consume it. > > > > No problem, we don't need to suggest a machine consumable > > alternative for everything. > > Sure, but we need to get enough value out of it to justify its cost. > > > I'm thinking about features that are visible to the end user and > > require user action to fix their configuration, like machine type > > versions or CPU model versions. > > Even those may need translation as we cross layers of the stack. > > The fewer cases we have where the machinery for machine-readable > deprecation advice is actually useful, the worse its cost / benefit > ratio is going to be. Are you arguing the cost is unreasonably large? I still don't see what the problem is, here. -- Eduardo
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
Eduardo Habkost writes: > On Thu, May 09, 2019 at 10:31:46AM +0200, Markus Armbruster wrote: >> We've wandered into the QAPI vs. QOM swamp. Cc: Paolo. >> >> Eduardo Habkost writes: >> >> > On Wed, May 08, 2019 at 11:16:50AM +0200, Markus Armbruster wrote: >> >> Eduardo Habkost writes: >> >> >> >> > On Tue, May 07, 2019 at 07:07:04AM +0200, Markus Armbruster wrote: >> >> >> Eduardo Habkost writes: >> >> >> >> >> >> > This series adds machine type deprecation information to the >> >> >> > output of the `query-machines` QMP command. With this, libvirt >> >> >> > and management software will be able to show this information to >> >> >> > users and/or suggest changes to VM configuration to avoid >> >> >> > deprecated machine types. >> >> >> >> >> >> This overlaps with something I want to try, namely using Kevin's >> >> >> proposed QAPI feature flags for deprecation markings. Let's compare >> >> >> the >> >> >> two. >> >> >> >> >> >> To mark something as deprecated with your patches, you add a >> >> >> @support-status member somewhere, where "somewhere" is related to >> >> >> "something" by "provides information on". >> >> >> >> >> >> Example: MachineInfo (returned by query-machines) provides information >> >> >> on possible values of -machine parameter type. If -machine was >> >> >> QAPIfied, it would provide information on possible values of a QAPI >> >> >> object type's member. The type might be anonymous. The member should >> >> >> be an enum (we currently use 'str' in MachineInfo). >> >> > >> >> > QAPIfying -machine, -cpu, and -device would be wonderful. >> >> > >> >> >> >> >> >> Example: say we want to deprecate block driver "vfat", >> >> >> i.e. BlockdevDriver member @vfat. Type BlockdevDriver is used in >> >> >> multiple places; let's ignore all but BlockdevOptions. We need to add >> >> >> @support-status to something that provides information on >> >> >> BlockdevDriver, or maybe on BlockdevOptions. There is no ad hoc query >> >> >> providing information on either of the two, because QAPI/QMP >> >> >> introspection has been sufficient. What now? >> >> >> >> >> >> Can we add deprecation information to (general) QAPI/QMP introspection >> >> > >> >> > Yes, we can. I think it's a good idea. But: >> >> > >> >> >> instead of ad hoc queries? >> >> > >> >> > I'm not sure about the "instead of" part. I don't want perfect >> >> > to be the enemy of done, and I don't want QAPIfication of >> >> > -machine to be a requirement to start reporting machine type >> >> > deprecation information. >> >> >> >> Valid point. Still, I believe we should at least try to predict how the >> >> pieces we create now would fit with the pieces we plan to create later >> >> on. >> > >> > Sure. >> > >> >> >> >> Note that full QAPIfication of -machine isn't necessary to make QAPI >> >> feature "deprecated" work for machine types. Turning MachineInfo member >> >> @name into an enum, so we can tack "deprecated" onto its values, would >> >> suffice. >> >> >> >> Such a QAPIfication of machine types is still hard: QOM types are >> >> defined at compile time just like the QAPI schema, but their definition >> >> is distributed, and collected into one place only at run time. I >> >> discussed this on slide 39 of my "QEMU interface introspection: From >> >> hacks to solutions" talk (KVM Form 2015). Just for device_add, but it's >> >> just a special case of QOM. Choices listed there: >> >> >> >> * Collect drivers at compile time? Hard... >> >> * Make QAPI schema dynamic? Hard... >> >> * Forgo driver-specific arguments in schema? >> >> Defeats introspection... >> >> >> >> I'd like to add to the last item: >> >> >> >> Provide QOM introspection on par with QAPI schema introspection >> >> >> >> The QOM introspection we have (qom-list-types etc. is not on par. >> > >> > Agreed, but do we really want to do it? We have been avoiding >> > exposing QOM internals to the outside on purpose. I believe >> > there are at least two reasons for that: >> > >> > 1) Not every QOM type/property is supposed to be visible to the >> >outside >> >> True. >> >> However, the parts of QOM exposed via device_add and object-add are >> definitely part of the stable external interface (unless explicitly >> marked unstable). >> >> >(and nobody really knows what's the full set of >> >supported external QOM interfaces); >> >> Also true. And terrible. >> >> > 2) QAPI is our preferred interface interface specification/introspection >> >mechanism. >> >> When preferences and requirements collide, preferences tend to get run >> over. >> >> The QAPI schema is *declarative*: the schema declares QAPI objects and >> properties. We generate C from the schema, which we then compile and >> link into QEMU. >> >> QOM is by design *imperative*: we execute compiled C at QEMU run-time to >> define QOM objects and properties. Maximizes flexibility. See also >> Turing tarpit. >> >> No matter how much we'd prefer to use
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
Eduardo Habkost writes: > On Thu, May 09, 2019 at 05:08:11PM +0100, Daniel P. Berrangé wrote: >> On Thu, May 09, 2019 at 12:52:47PM -0300, Eduardo Habkost wrote: >> > On Thu, May 09, 2019 at 10:14:52AM +0100, Daniel P. Berrangé wrote: >> > > On Thu, May 09, 2019 at 10:31:46AM +0200, Markus Armbruster wrote: >> > > > We've wandered into the QAPI vs. QOM swamp. Cc: Paolo. >> > > > >> > > > Eduardo Habkost writes: >> > > > >> > > > > On Wed, May 08, 2019 at 11:16:50AM +0200, Markus Armbruster wrote: [...] >> > > > >> I agree we should point to a preferred replacement whenever we >> > > > >> deprecate >> > > > >> something. >> > > > >> >> > > > >> We have to do it in documentation. And we generally do, in >> > > > >> qemu-deprecated.texi. >> > > > >> >> > > > >> How useful would doing it in QMP as well be? Depends on what >> > > > >> management >> > > > >> applications can do with the additional information. >> > > > > >> > > > > I expect it to be useful for things that have obvious >> > > > > replacements, like old machine type or CPU model versions. >> > > > >> > > > I doubt a management application should apply suggested replacements >> > > > automatically, and I doubt libvirt would. Not even when QEMU >> > > > developers >> > > > deem them "obvious". >> > > >> > > We certainly won't apply the suggested replacement as in many cases >> > > it is not going to be a functionally equivalent drop-in. >> > >> > Who's "we"? >> >> I was refering to libvirt here. >> >> > > If QEMU logs it to stderr, it will end up in the per-VM log file >> > > libvirt has under /var/log/libvirt/qemu/$GUESTNAME.log. If QEMU >> > > doesn't log it to stderr, then libvirt would just write it to >> > > that same log file itself. >> > > >> > > If libvirt gains some API or event for notifying apps of deprecation >> > > we might bubble it up to the mgmt app that way. >> > > >> > > I still feel it is useful to have the suggested replacement in the >> > > logs, rather than only leaving it in qemu-deprecated.texi. This >> > > way the info is immediately visible to both app developers and any >> > > support person dealing with bugs. >> > > >> > > If the app dev see the suggested replacement upfront they're more >> > > likely to make an immediate decision to update their code if the >> > > suggestion is trivial. If they need to go find the QEMU docs to >> > > lookup what action is required I feel they'll more likely just >> > > put the item on their long todo list where it will languish. >> > >> > Agreed. However, note that the audience for deprecation >> > information is not just developers and support people. End users >> > need to know when they are relying on a deprecated feature, and >> > applications should make it as easy as possible for them to >> > update their configurations. >> > >> > I'm not suggesting the alternative would be applied >> > automatically. But having the alternative available in a >> > machine-friendly way may be the difference between a unhelpful UI >> > that just tells the user there's some problem but can't give a >> > solution, and one that can really assist the user to fix the >> > problem. >> >> For some aspects of QEMU it might be possible, but considering the >> broader set of things which can be deprecated, I don't think it is >> possible to expose a machine consumable "suggestion". >> >> Consider the deprecation of the ACL feature. We deprecated monitor >> commands "acl_add", "acl_policy", "acl_reset", etc. The suggested >> replacement is to use one of the many possible QAuthZ types combined >> with the -object arg. Even if we invented some way to express this >> in the schema, I don't think any app would usefully consume it. > > No problem, we don't need to suggest a machine consumable > alternative for everything. Sure, but we need to get enough value out of it to justify its cost. > I'm thinking about features that are visible to the end user and > require user action to fix their configuration, like machine type > versions or CPU model versions. Even those may need translation as we cross layers of the stack. The fewer cases we have where the machinery for machine-readable deprecation advice is actually useful, the worse its cost / benefit ratio is going to be.
Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP
Eduardo Habkost writes: > On Thu, May 09, 2019 at 10:14:52AM +0100, Daniel P. Berrangé wrote: >> On Thu, May 09, 2019 at 10:31:46AM +0200, Markus Armbruster wrote: >> > We've wandered into the QAPI vs. QOM swamp. Cc: Paolo. >> > >> > Eduardo Habkost writes: >> > >> > > On Wed, May 08, 2019 at 11:16:50AM +0200, Markus Armbruster wrote: [...] >> > >> I agree we should point to a preferred replacement whenever we deprecate >> > >> something. >> > >> >> > >> We have to do it in documentation. And we generally do, in >> > >> qemu-deprecated.texi. >> > >> >> > >> How useful would doing it in QMP as well be? Depends on what management >> > >> applications can do with the additional information. >> > > >> > > I expect it to be useful for things that have obvious >> > > replacements, like old machine type or CPU model versions. >> > >> > I doubt a management application should apply suggested replacements >> > automatically, and I doubt libvirt would. Not even when QEMU developers >> > deem them "obvious". >> >> We certainly won't apply the suggested replacement as in many cases >> it is not going to be a functionally equivalent drop-in. > > Who's "we"? > >> >> If QEMU logs it to stderr, it will end up in the per-VM log file >> libvirt has under /var/log/libvirt/qemu/$GUESTNAME.log. If QEMU >> doesn't log it to stderr, then libvirt would just write it to >> that same log file itself. >> >> If libvirt gains some API or event for notifying apps of deprecation >> we might bubble it up to the mgmt app that way. >> >> I still feel it is useful to have the suggested replacement in the >> logs, rather than only leaving it in qemu-deprecated.texi. This >> way the info is immediately visible to both app developers and any >> support person dealing with bugs. >> >> If the app dev see the suggested replacement upfront they're more >> likely to make an immediate decision to update their code if the >> suggestion is trivial. If they need to go find the QEMU docs to >> lookup what action is required I feel they'll more likely just >> put the item on their long todo list where it will languish. > > Agreed. However, note that the audience for deprecation > information is not just developers and support people. End users > need to know when they are relying on a deprecated feature, and > applications should make it as easy as possible for them to > update their configurations. > > I'm not suggesting the alternative would be applied > automatically. But having the alternative available in a > machine-friendly way may be the difference between a unhelpful UI > that just tells the user there's some problem but can't give a > solution, and one that can really assist the user to fix the > problem. I'm skeptical. For the management application to assist its users, it has to translate both the deprecated QEMU interface and its replacement into its own interfaces (because those are the ones the users actually use). Management applications routinely translate in the other direction. I doubt anyone would build reverse translation capabilities just for helping users update deprecated configurations. So unless such capabilities get built for other purposes, machine-friendliness will remain unused. If the management application's user is another machine, another translation is needed. And so forth until we reach the guy who's supposed to update configuration. Such a game of telephone is unlikely to produce anything but confusion, except for specific cases we test across the whole stack.
