Re: [Qemu-devel] [PATCH RFC v5 1/5] qemu-iotests: qemu machine type support
On 2015-02-10 at 02:59, Xiao Guang Chen wrote: This patch adds qemu machine type support to the io test suite. Based on the qemu default machine type and alias of the default machine type the reference output file can now vary from the default to a machine specific output file if necessary. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com Should be "Xiao Guang Chen ", but that can be fixed up by the applying maintainer. --- tests/qemu-iotests/check | 5 + tests/qemu-iotests/common.config | 9 + tests/qemu-iotests/iotests.py| 1 + 3 files changed, 15 insertions(+) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index baeae80..22b2e63 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -324,6 +324,11 @@ do fi reference="$source_iotests/$seq.out" +reference_machine="$source_iotests/$seq.$QEMU_DEFAULT_MACHINE.out" +if [ -f "$reference_machine" ]; then +reference="$reference_machine" +fi + if [ "$CACHEMODE" = "none" ]; then [ -f "$source_iotests/$seq.out.nocache" ] && reference="$source_iotests/$seq.out.nocache" fi diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index a1973ad..0288cb1 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -107,6 +107,15 @@ export QEMU=$QEMU_PROG export QEMU_IMG=$QEMU_IMG_PROG export QEMU_IO="$QEMU_IO_PROG $QEMU_IO_OPTIONS" export QEMU_NBD=$QEMU_NBD_PROG +default_machine=$($QEMU -machine \? | awk '/(default)/{print $1}') +default_alias_machine=$($QEMU -machine \? |\ +awk -v var_default_machine="$default_machine"\)\ +'{if ($(NF-2)=="(alias"&&$(NF-1)=="of"&&$(NF)==var_default_machine){print $1}}') +if [ ! -z "$default_alias_machine" ]; then +default_machine="$default_alias_machine" +fi + +export QEMU_DEFAULT_MACHINE="$default_machine" It'd be good if someone else (who knows more about awk than me, which means, who just knows anything) could review this, but apart from me not knowing how this might break: Reviewed-by: Max Reitz [ -f /etc/qemu-iotest.config ] && . /etc/qemu-iotest.config diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 241b5ee..1f520ed 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -39,6 +39,7 @@ imgproto = os.environ.get('IMGPROTO', 'file') test_dir = os.environ.get('TEST_DIR', '/var/tmp') output_dir = os.environ.get('OUTPUT_DIR', '.') cachemode = os.environ.get('CACHEMODE') +qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE') socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
Re: [Qemu-devel] [PATCH RFC v5 2/5] qemu-iotests: run qemu with -nodefaults and fix 067, 071, 087
On 2015-02-10 at 02:59, Xiao Guang Chen wrote: This patch fixes an io test suite issue that was introduced with the commit c88930a6866e74953e931ae749781e98e486e5c8 'qemu-char: Permit only a single "stdio" character device'. The option supresses the creation of default devices such as the floopy and cdrom. Output files for test case 067, 071 and 087 need to be updated to accommodate this change. Use virtio-blk instead of virtio-blk-pci as the device driver for test case 067. For virtio-blk-pci is the same with virtio-blk as device driver but other platform such as s390 may not recognize the virtio-blk-pci. Reviewed-by: Michael Mueller Signed-off-by: Xiao Guang Chen --- tests/qemu-iotests/067 | 8 +- tests/qemu-iotests/067.out | 266 +-- tests/qemu-iotests/071.out | 4 - tests/qemu-iotests/087.out | 12 -- tests/qemu-iotests/common| 1 + tests/qemu-iotests/common.config | 2 +- tests/qemu-iotests/common.qemu | 2 +- 7 files changed, 8 insertions(+), 287 deletions(-) I'm sorry I didn't notice earlier, but test 081 needs the same treatment. Apart from this, this patch looks good to me, though. Max
Re: [Qemu-devel] [PATCH RFC v5 4/5] qemu-iotests: s390x: fix test 055
On 2015-02-10 at 03:00, Xiao Guang Chen wrote: There is no 'ide-cd' device defined on s390 platform, so test_medium_not_found() test should be skipped. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com With this fixed as in the previous patches: Reviewed-by: Max Reitz --- tests/qemu-iotests/055 | 9 + 1 file changed, 9 insertions(+)
Re: [Qemu-devel] [PATCH RFC v5 3/5] qemu-iotests: s390x: fix test 041
On 2015-02-10 at 02:59, Xiao Guang Chen wrote: There is no 'ide-cd' device defined on s390 platform, so test_medium_not_found() test should be skipped. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com Again, should be "Xiao Guang Chen ". --- tests/qemu-iotests/041 | 6 ++ 1 file changed, 6 insertions(+) With that fixed: Reviewed-by: Max Reitz
Re: [Qemu-devel] Revert commit 5af35d7feccaa7d26b72c6c3d14116421d736b36 - "usb-host-libusb: Fix reset handling"
Hello Hans, thanks for taking care. 09-02-15 09:09, Hans de Goede wrote: Hi, On 09-02-15 22:09, Dennis Ostermann wrote: Hi there, please revert commit 5af35d7feccaa7d26b72c6c3d14116421d736b36 - "usb-host-libusb: Fix reset handling" This breaks usb pass through of FTDI based usb devices: On the host: lsusb | grep FT2232 Bus 003 Device 008: ID 0403:6010 Future Technology Devices International, Ltd FT2232C Dual USB-UART/FIFO IC ~/qemu-install/bin$ sudo ./qemu-system-x86_64 -monitor telnet:127.0.0.1:1234,server,nowait -hda /dev/sdd2 -redir tcp:20022::22 --enable-kvm -cpu host -smp 4 -vga vmware --vnc :0 -m 8192 -usb -device usb-ehci,id=ehci -device usb-host,bus=ehci.0,vendorid=0x0403,productid=0x6010 WARNING: Image format was not specified for '/dev/sdd2' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/005: No such file or directory libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/006: No such file or directory libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/007: No such device The device gets reset again and again and is re-enumerated every time and finally not passed through. This looks like the device drops of the bus when it is reset, that is not normal behavior, there seems to be something unique to your setup causing this. Have you tried this on multiple machines / different usb ports on your pc ? This may be something weird with the usb controller in your machine. The machine uses Intel H97 chipset, so not that unique. I tried every port, every USB BIOS, with and without hub, it doesn't make any difference. But you're right, I also tried it on a T61 with Intel 900 series chipset and it worked alright. But this is an 'old' USB 2.0 chipset. I'll try on another box with USB 3.0 chipset tomorrow, if I can get one. With the commit reverted, it even works behind a no-name china USB 3.0 hub. After reverting the commit: ~/qemu-patched-install/bin$ sudo ./qemu-system-x86_64 -monitor telnet:127.0.0.1:1234,server,nowait -hda /dev/sdd2 -redir tcp:20022::22 --enable-kvm -cpu host -smp 4 -vga vmware --vnc :0 -m 8192 -usb -device usb-ehci,id=ehci -device usb-host,bus=ehci.0,vendorid=0x0403,productid=0x6010 WARNING: Image format was not specified for '/dev/sdd2' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. The device appears in the guest OS and can be used. Tested with HEAD and several libusb versions. Affects at least FTDI FT2232H and FTDI FT232R. Which versions of libusb have you tested exactly ? Latest HEAD, 1.0.16. and 1.0.17. Should I try any other? Couldn't find anything in the commit logs that might have s.th. to do with my issue. I compiled 1.0.16 with debug enabled. Here is what it shows with the original qemu: ~/qemu-install/bin$ sudo LD_LIBRARY_PATH=~/Devel/libusb-1.0.16-install/lib ./qemu-system-x86_64 -monitor telnet:127.0.0.1:1234,server,nowait -hda /dev/sdd2 -redir tcp:20022::22 --enable-kvm -cpu host -smp 4 -vga vmware --vnc :0 -m 8192 -usb -device usb-ehci,id=ehci -device usb-host,bus=ehci.0,vendorid=0x0403,productid=0x6010 WARNING: Image format was not specified for '/dev/sdd2' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. [timestamp] [threadID] facility level [function call] [ 0.03] [081c] libusbx: debug [libusb_init] created default context [ 0.37] [081c] libusbx: debug [libusb_init] libusbx v1.0.16.10774 [ 0.57] [081c] libusbx: debug [find_usbfs_path] found usbfs at /dev/bus/usb [ 0.67] [081c] libusbx: debug [op_init] bulk continuation flag supported [ 0.70] [081c] libusbx: debug [op_init] zero length packet flag supported [ 0.80] [081c] libusbx: debug [op_init] sysfs can relate devices [ 0.85] [081c] libusbx: debug [op_init] sysfs has complete descriptors [ 0.000310] [0824] libusbx: debug [linux_udev_event_thread_main] udev event thread entering. [ 0.000469] [081c] libusbx: debug [linux_get_device_address] getting address for device: usb1 detached: 0 [ 0.000477] [081c] libusbx: debug [linux_get_device_address] scan usb1 [ 0.000509] [081c] libusbx: debug [linux_get_device_address] bus=1 dev=1 [ 0.000514] [081c] libusbx: debug [linux_enumerate_device] busnum 1 devaddr 1 session_id 257 [ 0.000518] [081c] libusbx:
Re: [Qemu-devel] [PATCH 2/9] monitor: Clean up around monitor_handle_fd_param()
On 02/10/2015 09:34 AM, Markus Armbruster wrote: > monitor_handle_fd_param() is a wrapper around > monitor_handle_fd_param2() that feeds errors to qerror_report_err() > instead of returning them. qerror_report_err() is inappropriate in > many contexts. monitor_handle_fd_param() looks simpler than > monitor_handle_fd_param2(), which tempts use. Remove the temptation: > drop the wrapper and open-code the (trivial) error handling instead. > > Replace the open-coded qerror_report_err() by error_report_err() in > places that already use error_report(). Turns out that's everywhere. > > While there, rename monitor_handle_fd_param2() to monitor_fd_param(). > > Signed-off-by: Markus Armbruster > --- > hw/i386/kvm/pci-assign.c | 5 ++--- > hw/scsi/vhost-scsi.c | 2 +- > include/monitor/monitor.h | 3 +-- > monitor.c | 15 +-- > net/socket.c | 4 +++- > net/tap.c | 11 --- > 6 files changed, 16 insertions(+), 24 deletions(-) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: vmdk - fixed sizeof() error
On 02/10/2015 11:22 AM, Jeff Cody wrote: > The size compared should be PATH_MAX, rather than sizeof(char *). > > Reported-by: Paolo Bonzini > Signed-off-by: Jeff Cody > --- > block/vmdk.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Eric Blake > > diff --git a/block/vmdk.c b/block/vmdk.c > index 7d079ad..8410a15 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -843,8 +843,7 @@ static int vmdk_parse_extents(const char *desc, > BlockDriverState *bs, > } > > extent_path = g_malloc0(PATH_MAX); > -path_combine(extent_path, sizeof(extent_path), > -desc_file_path, fname); > +path_combine(extent_path, PATH_MAX, desc_file_path, fname); > extent_file = NULL; > ret = bdrv_open(&extent_file, extent_path, NULL, NULL, > bs->open_flags | BDRV_O_PROTOCOL, NULL, errp); > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH RFC v5 5/5] qemu-iotests: s390x: fix test 051
On 2015-02-10 at 03:00, Xiao Guang Chen wrote: The tests for device type "ide_cd" should only be tested for the pc platform. The default device id of hard disk on the s390 platform differs to that of the x86 platform. A new variable device_id is defined and "virtio0" set for the s390 platform. A x86 platform specific output file is also needed. A new filter was added to filter s390 warnings. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com --- tests/qemu-iotests/051 | 79 +--- tests/qemu-iotests/051.out | 160 +-- tests/qemu-iotests/051.pc.out| 427 +++ tests/qemu-iotests/common.filter | 7 + 4 files changed, 532 insertions(+), 141 deletions(-) create mode 100644 tests/qemu-iotests/051.pc.out diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 11c858f..fedf2d4 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 [snip] @@ -241,28 +253,37 @@ echo echo === Snapshot mode === echo +case "$QEMU_DEFAULT_MACHINE" in +pc) +device_id="ide0-hd0" +;; +*) +device_id="virtio0" "virtio0" is available for s390-virtio by default, but probably not for other platforms. So I suggest you change the * into s390-virtio and skip these tests for non-s390-virtio and non-pc platforms. On second thought, this test is probably highly platform-specific (because the reference output is tied pretty tightly to the default device types of the platform in used). Maybe we need a _supported_platforms or something, and in this case only pc and s390-virtio would be supported. However, considering that the iotests only worked for x86 so far, I'd be fine with you just leaving this patch completely as-is (it does not break x86, and I trust you that it makes the test work on s390) and we care about making it work (or just making sure it gets skipped) for other platforms later on. Therefore, with the "Signed-off-by" line fixed: Reviewed-by: Max Reitz I have further comments below concerning the issue of other platforms than pc and s390-virtio, but they don't influence this R-b. +;; +esac + $QEMU_IO -c "write -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io # Opening a read-only file r/w with snapshot=on chmod u-w "$TEST_IMG" -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io chmod u+w "$TEST_IMG" $QEMU_IO -c "read -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=off | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG",snapshot=off | _filter_qemu_io $QEMU_IO -c "read -P 0x22 0 4k" "$TEST_IMG" | _filter_qemu_io -echo -e 'qemu-io ide0-hd0 "write -P 0x33 0 4k"\ncommit ide0-hd0' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io +echo -e "qemu-io $device_id \"write -P 0x33 0 4k\"\ncommit $device_id" | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io [snip] diff --git a/t
Re: [Qemu-devel] [PATCH RFC v2 1/8] qmp: print dirty bitmap
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Adds qmp and hmp commands to print dirty bitmap. This is needed only for testing. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 33 + blockdev.c| 13 + hmp-commands.hx | 15 +++ hmp.c | 8 hmp.h | 1 + include/block/block.h | 2 ++ qapi-schema.json | 3 ++- qapi/block-core.json | 3 +++ qmp-commands.hx | 5 + 9 files changed, 82 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 2466ba8..6d3f0b2 100644 --- a/block.c +++ b/block.c @@ -5374,6 +5374,39 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, } +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap) +{ +unsigned long a = 0, b = 0; + +printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name"); +printf("enabled: %s\n", bitmap->enabled ? "true" : "false"); +printf("size: %" PRId64 "\n", bitmap->size); +printf("granularity: %" PRId64 "\n", bitmap->granularity); +printf("dirty regions begin:\n"); + +while (true) { +for (a = b; a < bitmap->size && !hbitmap_get(bitmap->bitmap, a); ++a) { +; +} +if (a >= bitmap->size) { +break; +} + +for (b = a + 1; + b < bitmap->size && hbitmap_get(bitmap->bitmap, b); + ++b) { +; +} + +printf("%ld -> %ld\n", a, b - 1); +if (b >= bitmap->size) { +break; +} +} + +printf("dirty regions end\n"); +} + BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity, const char *name, diff --git a/blockdev.c b/blockdev.c index 209fedd..66f0437 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2074,6 +2074,19 @@ void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name, aio_context_release(aio_context); } +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name, + Error **errp) +{ +BdrvDirtyBitmap *bitmap; + +bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp); +if (!bitmap) { +return; +} + +bdrv_print_dirty_bitmap(bitmap); +} + void qmp_block_dirty_bitmap_remove(const char *node_ref, const char *name, Error **errp) { diff --git a/hmp-commands.hx b/hmp-commands.hx index e37bc8b..a9be506 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -58,6 +58,21 @@ Quit the emulator. ETEXI { +.name = "print_dirty_bitmap", +.args_type = "device:B,bitmap:s", +.params = "device bitmap", +.help = "print dirty bitmap", +.user_print = monitor_user_noop, +.mhandler.cmd = hmp_print_dirty_bitmap, +}, + +STEXI +@item print_dirty_bitmap device_id bitmap_name +@findex print_dirty_bitmap +Print dirty bitmap meta information and dirty regions. +ETEXI + +{ .name = "block_resize", .args_type = "device:B,size:o", .params = "device size", diff --git a/hmp.c b/hmp.c index 63b19c7..a269145 100644 --- a/hmp.c +++ b/hmp.c @@ -782,6 +782,14 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) qapi_free_TPMInfoList(info_list); } +void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict) +{ +const char *device = qdict_get_str(qdict, "device"); +const char *name = qdict_get_str(qdict, "bitmap"); + +qmp_block_dirty_bitmap_print(device, name, NULL); +} + void hmp_quit(Monitor *mon, const QDict *qdict) { monitor_suspend(mon); diff --git a/hmp.h b/hmp.h index 4bb5dca..6bbbc33 100644 --- a/hmp.h +++ b/hmp.h @@ -19,6 +19,7 @@ #include "qapi-types.h" #include "qapi/qmp/qdict.h" +void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict); void hmp_info_name(Monitor *mon, const QDict *qdict); void hmp_info_version(Monitor *mon, const QDict *qdict); void hmp_info_kvm(Monitor *mon, const QDict *qdict); diff --git a/include/block/block.h b/include/block/block.h index cb1f28d..6cf067f 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -459,6 +459,8 @@ void bdrv_dirty_iter_init(BlockDriverState *bs, void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset); int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap); + void bdrv_enable_copy_on_read(BlockDriverState *bs); void bdrv_disable_copy_on_read(BlockDriverState *bs); diff --git a/qapi-schema.json b/qapi-schema.json index 85f55d9..1475f69 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1263,7 +1263,8 @@ 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal', 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
Re: [Qemu-devel] [PATCH RFC v2 3/8] block: BdrvDirtyBitmap serialization interface
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Several functions to provide necessary access to BdrvDirtyBitmap for block-migration.c Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 36 include/block/block.h | 12 2 files changed, 48 insertions(+) diff --git a/block.c b/block.c index 6d3f0b2..9860fc1 100644 --- a/block.c +++ b/block.c @@ -5552,6 +5552,42 @@ void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) hbitmap_reset(bitmap->bitmap, 0, bitmap->size); } +const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap) +{ +return bitmap->name; +} + +uint64_t bdrv_dirty_bitmap_data_size(const BdrvDirtyBitmap *bitmap, + uint64_t count) +{ +return hbitmap_data_size(bitmap->bitmap, count); +} + +void bdrv_dirty_bitmap_serialize_part(const BdrvDirtyBitmap *bitmap, + uint8_t *buf, uint64_t start, + uint64_t count) +{ +hbitmap_serialize_part(bitmap->bitmap, buf, start, count); +} + +void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap *bitmap, +uint8_t *buf, uint64_t start, +uint64_t count) +{ +hbitmap_deserialize_part(bitmap->bitmap, buf, start, count); +} + +void bdrv_dirty_bitmap_deserialize_part0(BdrvDirtyBitmap *bitmap, + uint64_t start, uint64_t count) +{ +hbitmap_deserialize_part0(bitmap->bitmap, start, count); +} + +void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap) +{ +hbitmap_deserialize_finish(bitmap->bitmap); +} + static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors) { diff --git a/include/block/block.h b/include/block/block.h index 6cf067f..0890cd2 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -460,6 +460,18 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset); int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap); +const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap); +uint64_t bdrv_dirty_bitmap_data_size(const BdrvDirtyBitmap *bitmap, + uint64_t count); +void bdrv_dirty_bitmap_serialize_part(const BdrvDirtyBitmap *bitmap, + uint8_t *buf, uint64_t start, + uint64_t count); +void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap *bitmap, +uint8_t *buf, uint64_t start, +uint64_t count); +void bdrv_dirty_bitmap_deserialize_part0(BdrvDirtyBitmap *bitmap, + uint64_t start, uint64_t count); +void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap); void bdrv_enable_copy_on_read(BlockDriverState *bs); void bdrv_disable_copy_on_read(BlockDriverState *bs); Reviewed-by: John Snow
Re: [Qemu-devel] [PATCH RFC v2 2/8] hbitmap: serialization
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Functions to serialize / deserialize(restore) HBitmap. HBitmap should be saved to linear sequence of bits independently of endianness and bitmap array element (unsigned long) size. Therefore Little Endian is chosen. These functions are appropriate for dirty bitmap migration, restoring the bitmap in several steps is available. To save performance, every step writes only the last level of the bitmap. All other levels are restored by hbitmap_deserialize_finish() as a last step of restoring. So, HBitmap is inconsistent while restoring. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/qemu/hbitmap.h | 59 +++ util/hbitmap.c | 96 ++ 2 files changed, 155 insertions(+) diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index c48c50a..1d37582 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -137,6 +137,65 @@ void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t count); bool hbitmap_get(const HBitmap *hb, uint64_t item); /** + * hbitmap_data_size: + * @hb: HBitmap to operate on. + * @count: Number of bits + * + * Return amount of bytes hbitmap_serialize_part needs + */ +uint64_t hbitmap_data_size(const HBitmap *hb, uint64_t count); + +/** + * hbitmap_serialize_part + * @hb: HBitmap to oprate on. + * @buf: Buffer to store serialized bitmap. + * @start: First bit to store. + * @count: Number of bits to store. + * + * Stores HBitmap data corresponding to given region. The format of saved data + * is linear sequence of bits, so it can be used by hbitmap_deserialize_part + * independently of endianess and size of HBitmap level array elements + */ +void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf, +uint64_t start, uint64_t count); + +/** + * hbitmap_deserialize_part + * @hb: HBitmap to operate on. + * @buf: Buffer to restore bitmap data from. + * @start: First bit to restore. + * @count: Number of bits to restore. + * + * Retores HBitmap data corresponding to given region. The format is the same + * as for hbitmap_serialize_part. + * + * ! The bitmap becomes inconsistent after this operation. + * hbitmap_serialize_finish should be called before using the bitmap after + * data restoring. + */ +void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf, + uint64_t start, uint64_t count); + +/** + * hbitmap_deserialize_part0 + * @hb: HBitmap to operate on. + * @start: First bit to restore. + * @count: Number of bits to restore. + * + * Same as hbitmap_serialize_part, but fills the bitmap with zeroes. + */ +void hbitmap_deserialize_part0(HBitmap *hb, uint64_t start, uint64_t count); + Not important enough to warrant a respin on its own: maybe "hbitmap_deserialize_zeroes"? "part0" is a little odd. +/** + * hbitmap_deserialize_finish + * @hb: HBitmap to operate on. + * + * Repair HBitmap after calling hbitmap_deserialize_data. Actually, all HBitmap + * layers are restored here. + */ +void hbitmap_deserialize_finish(HBitmap *hb); + +/** * hbitmap_free: * @hb: HBitmap to operate on. * diff --git a/util/hbitmap.c b/util/hbitmap.c index f400dcb..55226a0 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -366,6 +366,102 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item) return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) != 0; } +uint64_t hbitmap_data_size(const HBitmap *hb, uint64_t count) +{ +uint64_t size, gran; + +if (count == 0) { +return 0; +} + +gran = 1ll << hb->granularity; +size = (((gran + count - 2) >> hb->granularity) >> BITS_PER_LEVEL) + 1; + +return size * sizeof(unsigned long); +} + +void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf, +uint64_t start, uint64_t count) +{ +uint64_t i; +uint64_t last = start + count - 1; +unsigned long *out = (unsigned long *)buf; + +if (count == 0) { +return; +} + +start = (start >> hb->granularity) >> BITS_PER_LEVEL; +last = (last >> hb->granularity) >> BITS_PER_LEVEL; +count = last - start + 1; + +for (i = start; i <= last; ++i) { +unsigned long el = hb->levels[HBITMAP_LEVELS - 1][i]; +out[i] = (BITS_PER_LONG == 32 ? cpu_to_le32(el) : cpu_to_le64(el)); +} +} + +void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf, + uint64_t start, uint64_t count) +{ +uint64_t i; +uint64_t last = start + count - 1; +unsigned long *in = (unsigned long *)buf; + +if (count == 0) { +return; +} + +start = (start >> hb->granularity) >> BITS_PER_LEVEL; +last = (last >> hb->granularity) >> BITS_PER_LEVEL; +count = last - start + 1; + +for (i = start; i <= last; ++i) { +hb->levels[HBITMAP_LEVELS - 1][i] = +(BITS_PER_LONG == 32 ? le32_to_cpu(in[i]) : le64_to_cpu(in[i])); +} +}
Re: [Qemu-devel] [PATCH RFC v2 4/8] block: add dirty-dirty bitmaps
I had hoped it wouldn't come to this :) On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: A dirty-dirty bitmap is a dirty bitmap for BdrvDirtyBitmap. It tracks set/unset changes of BdrvDirtyBitmap. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 44 include/block/block.h | 5 + 2 files changed, 49 insertions(+) diff --git a/block.c b/block.c index 9860fc1..8ab724b 100644 --- a/block.c +++ b/block.c @@ -53,6 +53,7 @@ struct BdrvDirtyBitmap { HBitmap *bitmap; +HBitmap *dirty_dirty_bitmap; Just opinions: Maybe we can call it the "meta_bitmap" to help keep the name less confusing, and accompany it with a good structure comment here to explain what the heck it's for. If you feel that's a good idea; s/dirty_dirty_/meta_/g below. Regardless, let's make sure this patch adds documentation for it. BdrvDirtyBitmap *originator; int64_t size; int64_t granularity; @@ -5373,6 +5374,30 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, return originator; } +HBitmap *bdrv_create_dirty_dirty_bitmap(BdrvDirtyBitmap *bitmap, +uint64_t granularity) +{ +uint64_t sector_granularity; + +assert((granularity & (granularity - 1)) == 0); + +granularity *= 8 * bitmap->granularity; +sector_granularity = granularity >> BDRV_SECTOR_BITS; +assert(sector_granularity); + +bitmap->dirty_dirty_bitmap = +hbitmap_alloc(bitmap->size, ffsll(sector_granularity) - 1); + +return bitmap->dirty_dirty_bitmap; +} + +void bdrv_release_dirty_dirty_bitmap(BdrvDirtyBitmap *bitmap) +{ +if (bitmap->dirty_dirty_bitmap) { +hbitmap_free(bitmap->dirty_dirty_bitmap); +bitmap->dirty_dirty_bitmap = NULL; +} +} void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap) { @@ -5447,6 +5472,9 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) if (bm == bitmap) { QLIST_REMOVE(bitmap, list); hbitmap_free(bitmap->bitmap); +if (bitmap->dirty_dirty_bitmap) { +hbitmap_free(bitmap->dirty_dirty_bitmap); +} g_free(bitmap->name); g_free(bitmap); return; @@ -5534,6 +5562,10 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, { if (bitmap->enabled) { hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors); + +if (bitmap->dirty_dirty_bitmap) { +hbitmap_set(bitmap->dirty_dirty_bitmap, cur_sector, nr_sectors); +} } } @@ -5541,6 +5573,9 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t cur_sector, int nr_sectors) { hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); +if (bitmap->dirty_dirty_bitmap) { +hbitmap_set(bitmap->dirty_dirty_bitmap, cur_sector, nr_sectors); +} } /** @@ -5550,6 +5585,9 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) { hbitmap_reset(bitmap->bitmap, 0, bitmap->size); +if (bitmap->dirty_dirty_bitmap) { +hbitmap_set(bitmap->dirty_dirty_bitmap, 0, bitmap->size); +} } const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap) @@ -5597,6 +5635,9 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, continue; } hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors); +if (bitmap->dirty_dirty_bitmap) { +hbitmap_set(bitmap->dirty_dirty_bitmap, cur_sector, nr_sectors); +} } } @@ -5606,6 +5647,9 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, BdrvDirtyBitmap *bitmap; QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); +if (bitmap->dirty_dirty_bitmap) { +hbitmap_set(bitmap->dirty_dirty_bitmap, cur_sector, nr_sectors); +} } } diff --git a/include/block/block.h b/include/block/block.h index 0890cd2..648b0a9 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -4,6 +4,7 @@ #include "block/aio.h" #include "qemu-common.h" #include "qemu/option.h" +#include "qemu/hbitmap.h" #include "block/coroutine.h" #include "block/accounting.h" #include "qapi/qmp/qobject.h" @@ -473,6 +474,10 @@ void bdrv_dirty_bitmap_deserialize_part0(BdrvDirtyBitmap *bitmap, uint64_t start, uint64_t count); void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap); +HBitmap *bdrv_create_dirty_dirty_bitmap(BdrvDirtyBitmap *bitmap, +uint64_t granularity); +void bdrv_release_dirty_dirty_bitmap(BdrvDirtyBitmap *bitmap); + void bdrv_enable_copy_on_rea
Re: [Qemu-devel] [PATCH RFC v2 6/8] block: add bdrv_next_dirty_bitmap()
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Like bdrv_next() - bdrv_next_dirty_bitmap() is a function to provide access to private dirty bitmaps list. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 10 ++ include/block/block.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/block.c b/block.c index 15fc621..9e59c2e 100644 --- a/block.c +++ b/block.c @@ -5514,6 +5514,16 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) return list; } +BdrvDirtyBitmap *bdrv_next_dirty_bitmap(BlockDriverState *bs, +BdrvDirtyBitmap *bitmap) +{ +if (bitmap == NULL) { +return QLIST_FIRST(&bs->dirty_bitmaps); +} + +return QLIST_NEXT(bitmap, list); +} + int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector) { if (bitmap) { diff --git a/include/block/block.h b/include/block/block.h index 7b49d98..34d0259 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -474,6 +474,8 @@ void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap *bitmap, void bdrv_dirty_bitmap_deserialize_part0(BdrvDirtyBitmap *bitmap, uint64_t start, uint64_t count); void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap); +BdrvDirtyBitmap *bdrv_next_dirty_bitmap(BlockDriverState *bs, +BdrvDirtyBitmap *bitmap); HBitmap *bdrv_create_dirty_dirty_bitmap(BdrvDirtyBitmap *bitmap, uint64_t granularity); Makes sense to me. Reviewed-by: John Snow
Re: [Qemu-devel] [PATCH RFC v2 5/8] block: add bdrv_dirty_bitmap_enabled()
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 5 + include/block/block.h | 1 + 2 files changed, 6 insertions(+) diff --git a/block.c b/block.c index 8ab724b..15fc621 100644 --- a/block.c +++ b/block.c @@ -5551,6 +5551,11 @@ uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs, return bitmap->granularity; } +bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap) +{ +return bitmap->enabled; +} + void bdrv_dirty_iter_init(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, HBitmapIter *hbi) { diff --git a/include/block/block.h b/include/block/block.h index 648b0a9..7b49d98 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -449,6 +449,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs); uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs); uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); +bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap); int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector); void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t cur_sector, int nr_sectors); I wrote something similar in my incremental backup series. I'm submitting a new incremental backup series (v12!) soon which might have /slightly/ different semantics for enabled/disabled bitmaps. I don't think you'll need this patch, but in case you do need it: Reviewed-by: John Snow
Re: [Qemu-devel] [PATCH RFC v2 7/8] migration: add dirty parameter
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Add dirty parameter to qmp-migrate command. If this parameter is true, migration/block.c will migrate dirty bitmaps. This parameter can be used without "blk" parameter to migrate only dirty bitmaps, skipping block migration. Signed-off-by: Vladimir Sementsov-Ogievskiy --- hmp-commands.hx | 12 +++- hmp.c | 4 +++- include/migration/migration.h | 1 + migration/migration.c | 4 +++- qapi-schema.json | 7 ++- qmp-commands.hx | 5 - savevm.c | 3 ++- 7 files changed, 26 insertions(+), 10 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index a9be506..c16f93c 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -902,23 +902,25 @@ ETEXI { .name = "migrate", -.args_type = "detach:-d,blk:-b,inc:-i,uri:s", -.params = "[-d] [-b] [-i] uri", +.args_type = "detach:-d,blk:-b,inc:-i,dirty:-D,uri:s", +.params = "[-d] [-b] [-i] [-D] uri", .help = "migrate to URI (using -d to not wait for completion)" "\n\t\t\t -b for migration without shared storage with" " full copy of disk\n\t\t\t -i for migration without " - "shared storage with incremental copy of disk " - "(base image shared between src and destination)", + "shared storage with incremental copy of disk\n\t\t\t" + " -D for migration of named dirty bitmaps as well\n\t\t\t" + " (base image shared between src and destination)", .mhandler.cmd = hmp_migrate, }, STEXI -@item migrate [-d] [-b] [-i] @var{uri} +@item migrate [-d] [-b] [-i] [-D] @var{uri} @findex migrate Migrate to @var{uri} (using -d to not wait for completion). -b for migration with full copy of disk -i for migration with incremental copy of disk (base image is shared) + -D for migration of named dirty bitmaps ETEXI { diff --git a/hmp.c b/hmp.c index a269145..0b89ee8 100644 --- a/hmp.c +++ b/hmp.c @@ -1347,10 +1347,12 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) int detach = qdict_get_try_bool(qdict, "detach", 0); int blk = qdict_get_try_bool(qdict, "blk", 0); int inc = qdict_get_try_bool(qdict, "inc", 0); +int dirty = qdict_get_try_bool(qdict, "dirty", 0); const char *uri = qdict_get_str(qdict, "uri"); Error *err = NULL; -qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err); +qmp_migrate(uri, !!blk, blk, !!inc, inc, !!dirty, dirty, +false, false, &err); if (err) { monitor_printf(mon, "migrate: %s\n", error_get_pretty(err)); error_free(err); diff --git a/include/migration/migration.h b/include/migration/migration.h index 3cb5ba8..48d71d3 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -37,6 +37,7 @@ struct MigrationParams { bool blk; bool shared; +bool dirty; }; typedef struct MigrationState MigrationState; diff --git a/migration/migration.c b/migration/migration.c index c49a05a..e7bb7f3 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -404,7 +404,8 @@ void migrate_del_blocker(Error *reason) } void qmp_migrate(const char *uri, bool has_blk, bool blk, - bool has_inc, bool inc, bool has_detach, bool detach, + bool has_inc, bool inc, bool has_dirty, bool dirty, + bool has_detach, bool detach, Error **errp) { Error *local_err = NULL; @@ -414,6 +415,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, params.blk = has_blk && blk; params.shared = has_inc && inc; +params.dirty = has_dirty && dirty; if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP || s->state == MIG_STATE_CANCELLING) { diff --git a/qapi-schema.json b/qapi-schema.json index 1475f69..1d10d6b 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1656,12 +1656,17 @@ # @detach: this argument exists only for compatibility reasons and # is ignored by QEMU # +# @dirty: #optional do dirty-bitmaps migration (can be used with or without +# @blk parameter) +# (since 2.3) +# # Returns: nothing on success # # Since: 0.14.0 ## { 'command': 'migrate', - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } } + 'data': { 'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*dirty': 'bool', +'*detach': 'bool' } } # @xen-save-devices-state: # diff --git a/qmp-commands.hx b/qmp-commands.hx index 8065715..f8e50ac 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -610,7 +610,7 @@ EQMP { .name = "migrate", -.args_type = "detach:-d,blk:-b,inc:-i,uri:s", +.args_type = "d
Re: [Qemu-devel] [PATCH RFC v2 8/8] migration: add migration/dirty-bitmap.c
Peter Maydell: What's the right way to license a file as copied from a previous version? See below, please; Max, Markus: ctrl+f "bdrv_get_device_name" and let me know what you think, if you would. Juan, Amit, David: Copying migration maintainers. On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote: Live migration of dirty bitmaps. Only named dirty bitmaps are migrated. If destination qemu is already containing a dirty bitmap with the same name as a migrated bitmap, then their granularities should be the same, otherwise the error will be generated. If destination qemu doesn't contain such bitmap it will be created. format: 1 byte: flags [ 1 byte: node name size ] \ flags & DEVICE_NAME [ n bytes: node name ] / [ 1 byte: bitmap name size ] \ [ n bytes: bitmap name ] | flags & BITMAP_NAME [ [ be64: granularity] ] flags & GRANULARITY [ 1 byte: bitmap enabled bit ] flags & ENABLED [ be64: start sector ] \ flags & (NORMAL_CHUNK | ZERO_CHUNK) [ be32: number of sectors ] / [ be64: buffer size ] \ flags & NORMAL_CHUNK [ n bytes: buffer ] / The last chunk should contain flags & EOS. The chunk may skip device and/or bitmap names, assuming them to be the same with the previous chunk. GRANULARITY is sent with the first chunk for the bitmap. ENABLED bit is sent in the end of "complete" stage of migration. So when destination gets ENABLED flag it should deserialize_finish the bitmap and set its enabled bit to corresponding value. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/migration/block.h | 1 + migration/Makefile.objs | 2 +- migration/dirty-bitmap.c | 606 ++ vl.c | 1 + 4 files changed, 609 insertions(+), 1 deletion(-) create mode 100644 migration/dirty-bitmap.c diff --git a/include/migration/block.h b/include/migration/block.h index ffa8ac0..566bb9f 100644 --- a/include/migration/block.h +++ b/include/migration/block.h @@ -14,6 +14,7 @@ #ifndef BLOCK_MIGRATION_H #define BLOCK_MIGRATION_H +void dirty_bitmap_mig_init(void); void blk_mig_init(void); int blk_mig_active(void); uint64_t blk_mig_bytes_transferred(void); OK. diff --git a/migration/Makefile.objs b/migration/Makefile.objs index d929e96..9adfda9 100644 --- a/migration/Makefile.objs +++ b/migration/Makefile.objs @@ -6,5 +6,5 @@ common-obj-y += xbzrle.o common-obj-$(CONFIG_RDMA) += rdma.o common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o -common-obj-y += block.o +common-obj-y += block.o dirty-bitmap.o OK. diff --git a/migration/dirty-bitmap.c b/migration/dirty-bitmap.c new file mode 100644 index 000..8621218 --- /dev/null +++ b/migration/dirty-bitmap.c @@ -0,0 +1,606 @@ +/* + * QEMU dirty bitmap migration + * + * derived from migration/block.c + * + * Author: + * Sementsov-Ogievskiy Vladimir + * + * original copyright message: + * = + * Copyright IBM, Corp. 2009 + * + * Authors: + * Liran Schour + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + * Contributions after 2012-01-13 are licensed under the terms of the + * GNU GPL, version 2 or (at your option) any later version. + * = + */ + Not super familiar with the right way to do licensing here; it's possible you may not need to copy the original here, but I'm not sure. You will want to make it clear what license applies to /your/ work, I think. Maybe Peter Maydell can clue us in. +#include "block/block.h" +#include "qemu/main-loop.h" +#include "qemu/error-report.h" +#include "migration/block.h" +#include "migration/migration.h" +#include "qemu/hbitmap.h" +#include + +#define CHUNK_SIZE (1 << 20) + +#define DIRTY_BITMAP_MIG_FLAG_EOS 0x01 +#define DIRTY_BITMAP_MIG_FLAG_NORMAL_CHUNK 0x02 +#define DIRTY_BITMAP_MIG_FLAG_ZERO_CHUNK0x04 +#define DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME 0x08 +#define DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME 0x10 +#define DIRTY_BITMAP_MIG_FLAG_GRANULARITY 0x20 +#define DIRTY_BITMAP_MIG_FLAG_ENABLED 0x40 +/* flags should be <= 0xff */ + We should give ourselves a little breathing room with the flags, since we've only got room for one more. +/* #define DEBUG_DIRTY_BITMAP_MIGRATION */ + +#ifdef DEBUG_DIRTY_BITMAP_MIGRATION +#define DPRINTF(fmt, ...) \ +do { printf("dirty_migration: " fmt, ## __VA_ARGS__); } while (0) +#else +#define DPRINTF(fmt, ...) \ +do { } while (0) +#endif + +typedef struct DirtyBitmapMigBitmapState { +/* Written during setup phase. */ +BlockDriverState *bs; +BdrvDirtyBitmap *bitmap; +HBitmap *dirty_bitmap; For my own sanity, I'd really prefer "bitmap" and "meta_bitmap" here; "dirty_bitmap" is often used as a synonym (outside of this file) to refer to the BdrvDirtyBitmap in general, so it's usag
Re: [Qemu-devel] [PATCH] block: vmdk - fixed sizeof() error
On 2015-02-10 at 13:22, Jeff Cody wrote: The size compared should be PATH_MAX, rather than sizeof(char *). Reported-by: Paolo Bonzini Signed-off-by: Jeff Cody --- block/vmdk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) I just noticed iotest 059 failing, bisected it and the changeset of the commit in question looked strangely related to a patch I seemed to recall having seen on qemu-devel today... :-) Reviewed-by: Max Reitz diff --git a/block/vmdk.c b/block/vmdk.c index 7d079ad..8410a15 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -843,8 +843,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, } extent_path = g_malloc0(PATH_MAX); -path_combine(extent_path, sizeof(extent_path), -desc_file_path, fname); +path_combine(extent_path, PATH_MAX, desc_file_path, fname); extent_file = NULL; ret = bdrv_open(&extent_file, extent_path, NULL, NULL, bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
Re: [Qemu-devel] [PATCH v12 02/17] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
On 2015-02-09 at 20:35, John Snow wrote: The new command pair is added to manage user created dirty bitmap. The dirty bitmap's name is mandatory and must be unique for the same device, but different devices can have bitmaps with the same names. The granularity is an optional field. If it is not specified, we will choose a default granularity based on the cluster size if available, clamped to between 4K and 64K to mirror how the 'mirror' code was already choosing granularity. If we do not have cluster size info available, we choose 64K. This code has been factored out into a helper shared with block/mirror. This patch also introduces the 'block_dirty_bitmap_lookup' helper, which takes a device name and a dirty bitmap name and validates the lookup, returning NULL and setting errp if there is a problem with either field. This helper will be re-used in future patches in this series. The types added to block-core.json will be re-used in future patches in this series, see: 'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}' Signed-off-by: Fam Zheng Signed-off-by: John Snow --- block.c | 20 ++ block/mirror.c| 10 + blockdev.c| 100 ++ include/block/block.h | 1 + qapi/block-core.json | 55 +++ qmp-commands.hx | 51 + 6 files changed, 228 insertions(+), 9 deletions(-) Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCH v12 03/17] block: Introduce bdrv_dirty_bitmap_granularity()
On 2015-02-09 at 20:35, John Snow wrote: This returns the granularity (in bytes) of dirty bitmap, which matches the QMP interface and the existing query interface. Small adjustments are made to ensure that granularity-- in bytes-- I guess these should be ASCII replacements of an em dash? But they kind of look like decrement operators to me... is handled consistently as a uint64_t throughout the code. Signed-off-by: John Snow --- block.c | 17 +++-- include/block/block.h | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 1661ff9..83f411f 100644 --- a/block.c +++ b/block.c @@ -5387,12 +5387,13 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) } BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, - int granularity, + uint64_t granularity, const char *name, Error **errp) { int64_t bitmap_size; BdrvDirtyBitmap *bitmap; +int sector_granularity; assert((granularity & (granularity - 1)) == 0); @@ -5400,8 +5401,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, error_setg(errp, "Bitmap already exists: %s", name); return NULL; } -granularity >>= BDRV_SECTOR_BITS; -assert(granularity); +sector_granularity = granularity >> BDRV_SECTOR_BITS; I can see Coverity's screams regarding a possible overflow already... (but maybe it doesn't even scream and I'm just overcautious) Whether you add an assert((granularity >> BDRV_SECTOR_BITS) <= INT_MAX) or not here (it does look pretty ugly and it is pretty unnecessary, I know) or not, and whether you do something about the decrement operators in the commit message or not: Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCH 1/9] error: New convenience function error_report_err()
On Tue, Feb 10, 2015 at 05:44:59PM +0100, Paolo Bonzini wrote: > > > On 10/02/2015 17:34, Markus Armbruster wrote: > > I've typed error_report("%s", error_get_pretty(ERR)) too many times > > already, and I've fixed too many instances of qerror_report_err(ERR) > > to error_report("%s", error_get_pretty(ERR)) as well. Capture the > > pattern in a convenience function. > > > > Since it's almost invariably followed by error_free(), stuff that into > > the convenience function as well. > > > > Put it to use with this Coccinelle semantic patch: > > For your "no good deed goes unpunished" record, can you prepare a Wiki > page on how to use Coccinelle? My attempts always saw it confused by > qemu/queue.h macros. I assume you are asking about how to use/apply a semantic patch, not about writing one[1]. I do this: $ cat /tmp/error_report.spatch @@ expression E; @@ -error_report("%s", error_get_pretty(E)); -error_free(E); +error_report_err(E); @@ expression E, S; @@ -error_report("%s", error_get_pretty(E)); +error_report_err(E); ( exit(S); | abort(); ) $ spatch --sp-file /tmp/error_report.spatch $(git grep -l error_report) > /tmp/error_report.patch init_defs_builtins: /usr/share/coccinelle/standard.h HANDLING: arch_init.c audio/spiceaudio.c audio/wavcapture.c balloon.c block.c block/archipelago.c block/dmg.c block/iscsi.c block/nfs.c block/qcow2-snapshot.c block/qcow2.c block/rbd.c block/sheepdog.c block/ssh.c block/vhdx-log.c block/vmdk.c blockdev.c bootdevice.c device_tree.c exec.c hw/9pfs/virtio-9p-proxy.c hw/9pfs/virtio-9p.c hw/arm/armv7m.c hw/arm/cubieboard.c hw/arm/digic_boards.c hw/arm/exynos4210.c hw/arm/highbank.c hw/arm/integratorcp.c hw/arm/realview.c hw/arm/strongarm.c hw/arm/versatilepb.c hw/arm/vexpress.c hw/arm/virt.c hw/arm/xilinx_zynq.c hw/audio/milkymist-ac97.c hw/block/onenand.c hw/block/tc58128.c hw/block/virtio-blk.c hw/char/lm32_uart.c hw/char/milkymist-uart.c hw/char/sclpconsole-lm.c hw/char/sclpconsole.c hw/char/serial-pci.c hw/char/serial.c hw/char/virtio-serial-bus.c hw/core/machine.c hw/core/qdev-properties-system.c hw/core/qdev-properties.c hw/core/qdev.c hw/display/cg3.c hw/display/cirrus_vga.c hw/display/g364fb.c hw/display/milkymist-tmu2.c hw/display/milkymist-vgafb.c hw/display/qxl.c hw/display/tcx.c hw/i386/acpi-build.c hw/i386/kvm/pci-assign.c hw/i386/pc.c hw/i386/pc_piix.c hw/i386/pc_q35.c hw/i386/pc_sysfw.c hw/i386/smbios.c hw/ide/ahci.c hw/ide/core.c hw/ide/pci.c hw/ide/qdev.c hw/input/milkymist-softusb.c hw/intc/s390_flic.c hw/intc/s390_flic_kvm.c hw/intc/xics.c hw/intc/xics_kvm.c hw/isa/pc87312.c hw/microblaze/boot.c hw/mips/mips_fulong2e.c hw/mips/mips_jazz.c hw/mips/mips_malta.c hw/mips/mips_mipssim.c hw/misc/ivshmem.c hw/misc/milkymist-hpdmc.c hw/misc/milkymist-pfpu.c hw/net/milkymist-minimac2.c hw/net/vhost_net.c hw/net/virtio-net.c hw/nvram/fw_cfg.c hw/pci/pci-hotplug-old.c hw/pci/pci.c hw/pci/slotid_cap.c hw/ppc/e500.c hw/ppc/mpc8544ds.c hw/ppc/ppc.c hw/ppc/ppc405_boards.c hw/ppc/spapr.c hw/ppc/spapr_pci.c hw/ppc/virtex_ml507.c hw/s390x/s390-pci-inst.c hw/s390x/virtio-ccw.c hw/scsi/scsi-bus.c hw/scsi/vhost-scsi.c hw/scsi/virtio-scsi.c hw/sd/milkymist-memcard.c hw/sh4/shix.c hw/sparc/sun4m.c hw/timer/lm32_timer.c hw/timer/milkymist-sysctl.c hw/tpm/tpm_passthrough.c hw/tricore/tricore_testboard.c hw/usb/bus.c hw/usb/ccid-card-passthru.c hw/usb/dev-bluetooth.c hw/usb/dev-network.c hw/usb/dev-serial.c hw/usb/dev-smartcard-reader.c hw/usb/dev-storage.c hw/usb/dev-uas.c hw/usb/hcd-ehci.c hw/usb/host-libusb.c hw/usb/redirect.c hw/vfio/common.c hw/vfio/pci.c hw/virtio/dataplane/vring.c hw/virtio/vhost-backend.c hw/virtio/vhost-user.c hw/virtio/virtio-pci.c hw/virtio/virtio.c hw/xtensa/sim.c hw/xtensa/xtfpga.c include/qapi/qmp/qerror.h include/qemu/error-report.h migration/block.c migration/migration.c migration/qemu-file-buf.c migration/rdma.c migration/tcp.c migration/unix.c migration/vmstate.c monitor.c net/dump.c net/l2tpv3.c net/net.c net/netmap.c net/slirp.c net/socket.c net/tap-bsd.c net/tap-linux.c net/tap-solaris.c net/tap-win32.c net/tap.c net/vhost-user.c numa.c qdev-monitor.c qemu-char.c qemu-img.c qemu-io-cmds.c qemu-io.c qemu-nbd.c qmp.c qobject/qerror.c qom/cpu.c savevm.c scripts/qapi-commands.py slirp/misc.c target-i386/cpu.c target-mips/kvm.c target-ppc/translate_init.c target-s390x/cpu.c target-s390x/kvm.c target-sparc/cpu.c tests/test-aio.c tests/test-thread-pool.c tests/test-throttle.c tpm.c trace/control.c ui/input.c ui/spice-core.c ui/vnc.c util/error.c util/osdep.c util/qemu-config.c util/qemu-error.c util/qemu-option.c vl.c [...] Note: processing took59.7s: arch_init.c audio/spiceaudio.c audio/wavcapture.c balloon.c block.c block/archipelago.c block/dmg.c block/iscsi.c block/nfs.c block/qcow2-snapshot.c block/qcow2.c block/rbd.c block/sheepdog.c block/ssh.c block/vhdx-log.c block/vmdk.c blockdev.c bootdevice.c device_tree.c exec.c hw/9pfs/virtio-9p-proxy.c hw/9pfs
Re: [Qemu-devel] [PATCH v12 03/17] block: Introduce bdrv_dirty_bitmap_granularity()
On 2015-02-09 at 20:35, John Snow wrote: This returns the granularity (in bytes) of dirty bitmap, which matches the QMP interface and the existing query interface. Small adjustments are made to ensure that granularity-- in bytes-- is handled consistently as a uint64_t throughout the code. Just asking whether you want to adjust the following, too: "int granularity" in mirror_free_init() (the granularity is stored as an 64 bit integer, but the value is limited by qmp_drive_mirror() to stay between 512 and 64M, and is defined to be a uint32_t by qapi/block-core.json). It's fine from my perspective, so my R-b stands, but it looked like it might conflict with "is handled consistently". :-) Max Signed-off-by: John Snow --- block.c | 17 +++-- include/block/block.h | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-)
Re: [Qemu-devel] [PATCH v12 03/17] block: Introduce bdrv_dirty_bitmap_granularity()
On 02/10/2015 05:13 PM, Max Reitz wrote: On 2015-02-09 at 20:35, John Snow wrote: This returns the granularity (in bytes) of dirty bitmap, which matches the QMP interface and the existing query interface. Small adjustments are made to ensure that granularity-- in bytes-- is handled consistently as a uint64_t throughout the code. Just asking whether you want to adjust the following, too: "int granularity" in mirror_free_init() (the granularity is stored as an 64 bit integer, but the value is limited by qmp_drive_mirror() to stay between 512 and 64M, and is defined to be a uint32_t by qapi/block-core.json). It's fine from my perspective, so my R-b stands, but it looked like it might conflict with "is handled consistently". :-) Max Signed-off-by: John Snow --- block.c | 17 +++-- include/block/block.h | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-) I'll take a look! Every time I change a type I have to change ten more. It is a ceaseless game of whackamole. When I inevitably need to change the QMP parameter name again, I'll fix this while I'm at it.
Re: [Qemu-devel] [PATCH v12 04/17] hbitmap: add hbitmap_merge
On 2015-02-09 at 20:35, John Snow wrote: We add a bitmap merge operation to assist in error cases where we wish to combine two bitmaps together. This is algorithmically O(bits) provided HBITMAP_LEVELS remains constant. For a full bitmap on a 64bit machine: sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits We may be able to improve running speed for particularly sparse bitmaps by using iterators, but the running time for dense maps will be worse. We present the simpler solution first, and we can refine it later if needed. Signed-off-by: John Snow --- include/qemu/hbitmap.h | 11 +++ util/hbitmap.c | 32 2 files changed, 43 insertions(+) Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCH 0/2] qcow2: Respect new_block in alloc_refcount_block()
On 02/10/2015 01:02 PM, Max Reitz wrote: > Under certain circumstances, making the refcount table grow can result > in leaking clusters. The first patch fixes at least some of those > circumstances (maybe there are more, but these are the ones I am aware > of), and the second patch adds a test case. > > > Max Reitz (2): > qcow2: Respect new_block in alloc_refcount_block() > iotests: Add tests for refcount table growth Series: Reviewed-by: Eric Blake How'd you find the leak? > > block/qcow2-refcount.c | 16 ++- > tests/qemu-iotests/121 | 102 > + > tests/qemu-iotests/121.out | 23 ++ > tests/qemu-iotests/group | 1 + > 4 files changed, 140 insertions(+), 2 deletions(-) > create mode 100755 tests/qemu-iotests/121 > create mode 100644 tests/qemu-iotests/121.out > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/2] qcow2: Respect new_block in alloc_refcount_block()
On 2015-02-10 at 17:19, Eric Blake wrote: On 02/10/2015 01:02 PM, Max Reitz wrote: Under certain circumstances, making the refcount table grow can result in leaking clusters. The first patch fixes at least some of those circumstances (maybe there are more, but these are the ones I am aware of), and the second patch adds a test case. Max Reitz (2): qcow2: Respect new_block in alloc_refcount_block() iotests: Add tests for refcount table growth Series: Reviewed-by: Eric Blake How'd you find the leak? Test 015 failed with refcount_bits=32 and refcount_bits=64 after my refcount_order series; but strangely only if the recent qcow2 cache patch (http://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg00786.html) was applied before. Max
[Qemu-devel] [PATCH] balloon: Fix typo
Commit 422e0501 introduced a typo (unless removing an 'o' from balloon is how you deflate it?) Signed-off-by: Eric Blake --- I'm usually good at spotting typos when giving my R-b. Oh well, Dave is doing well to make sure my ego doesn't get too inflated :) balloon.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/balloon.c b/balloon.c index dea19a4..70c00f5 100644 --- a/balloon.c +++ b/balloon.c @@ -36,7 +36,7 @@ static QEMUBalloonEvent *balloon_event_fn; static QEMUBalloonStatus *balloon_stat_fn; static void *balloon_opaque; -static bool have_ballon(Error **errp) +static bool have_balloon(Error **errp) { if (kvm_enabled() && !kvm_has_sync_mmu()) { error_set(errp, ERROR_CLASS_KVM_MISSING_CAP, @@ -81,7 +81,7 @@ BalloonInfo *qmp_query_balloon(Error **errp) { BalloonInfo *info; -if (!have_ballon(errp)) { +if (!have_balloon(errp)) { return NULL; } @@ -92,7 +92,7 @@ BalloonInfo *qmp_query_balloon(Error **errp) void qmp_balloon(int64_t target, Error **errp) { -if (!have_ballon(errp)) { +if (!have_balloon(errp)) { return; } -- 2.1.0
Re: [Qemu-devel] [PATCH 8/9] balloon: Factor out common "is balloon active" test
On 02/10/2015 11:39 AM, Dr. David Alan Gilbert wrote: > * Markus Armbruster (arm...@redhat.com) wrote: >> Signed-off-by: Markus Armbruster >> --- >> balloon.c | 29 +++-- >> 1 file changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/balloon.c b/balloon.c >> index 2884c2d..aa30617 100644 >> --- a/balloon.c >> +++ b/balloon.c >> @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn; >> static QEMUBalloonStatus *balloon_stat_fn; >> static void *balloon_opaque; >> >> +static int have_ballon(Error **errp) > >^ > > An 'o' escaped from your balloon. Oops - it also escaped from my R-b. I've sent the obvious patch, as penance for my "deflated ego" :) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs
On 2015/2/10 3:56, Michael S. Tsirkin wrote: It doesn't make sense to copy values manually: the only issue with getting headers from linux seems to be dealing with linux/types, we can easily fix that automatically while importing. Signed-off-by: Michael S. Tsirkin --- FYI this is what I propose instead of the recently suggested virtio: uniform virtio device IDs we can then rework existing code to include these headers. Will automatically bring in goodies as they arrive in linux. This doesn't yet import virtio ccw header, that won't be hard to add later. scripts/update-linux-headers.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index c8e026d..0bd8437 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -76,4 +76,14 @@ else cp "$linux/COPYING" "$output/linux-headers" fi +rm -rf "$output/standard-headers/linux" +mkdir -p "$output/standard-headers/linux" Shouldn't we add something in configure file to execute this automatically? Or instead of creating 'standard-headers/, why can't we go that existing linux-headers/? Thanks Tiejun +for f in $tmpdir/include/linux/virtio*h; do +header=$(expr "$f" : '.*/\(.*\)'); +sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \ +-e 's/linux\/types/inttypes/' \ +-e 's/__bitwise__//' \ +"$tmpdir/include/linux/$header" > \ +"$output/standard-headers/linux/$header"; +done rm -rf "$tmpdir"
Re: [Qemu-devel] [PATCH v1 1/2] vhost-user: support SET_MEM_TABLE waite the result of mmap
On 2015/2/10 20:04, Michael S. Tsirkin wrote: > So that's not good. We need a way to negotiate the capability, > we can't just deadlock with legacy slaves. Should we wait many seconds if slave not reply we just return error? -- Regards, Haifeng
Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs
On 11 February 2015 at 01:36, Chen, Tiejun wrote: > On 2015/2/10 3:56, Michael S. Tsirkin wrote: >> >> It doesn't make sense to copy values manually: >> the only issue with getting headers from linux >> seems to be dealing with linux/types, we >> can easily fix that automatically while importing. >> >> Signed-off-by: Michael S. Tsirkin >> --- >> >> FYI this is what I propose instead of the recently >> suggested >> virtio: uniform virtio device IDs >> we can then rework existing code to include these headers. >> >> Will automatically bring in goodies as they arrive in linux. >> >> This doesn't yet import virtio ccw header, >> that won't be hard to add later. >> >> scripts/update-linux-headers.sh | 10 ++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/scripts/update-linux-headers.sh >> b/scripts/update-linux-headers.sh >> index c8e026d..0bd8437 100755 >> --- a/scripts/update-linux-headers.sh >> +++ b/scripts/update-linux-headers.sh >> @@ -76,4 +76,14 @@ else >> cp "$linux/COPYING" "$output/linux-headers" >> fi >> >> +rm -rf "$output/standard-headers/linux" >> +mkdir -p "$output/standard-headers/linux" > > > Shouldn't we add something in configure file to execute this automatically? No. We want to run this script only when we're updating the header files, which only happens when we have a valid kernel source tree available and you're a developer doing it as a specific action. configure is run by everybody and should definitely not be doing header updates. > Or instead of creating 'standard-headers/, why can't we go that existing > linux-headers/? The linux-headers/ directory contains header files which can only validly be included if the host we're compiling on is Linux. Some of them will cause compile failures on OSX or Windows if they are in the include path. The idea of this patch is that the standard-headers/ directory has "sanitized" header files which have had the linux-specific types and includes stripped out. So if we take the route this patch proposes we do need two directories. -- PMM
Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs
On 9 February 2015 at 19:56, Michael S. Tsirkin wrote: > +rm -rf "$output/standard-headers/linux" > +mkdir -p "$output/standard-headers/linux" > +for f in $tmpdir/include/linux/virtio*h; do > +header=$(expr "$f" : '.*/\(.*\)'); > +sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \ > +-e 's/linux\/types/inttypes/' \ > +-e 's/__bitwise__//' \ > +"$tmpdir/include/linux/$header" > \ > +"$output/standard-headers/linux/$header"; > +done This doesn't seem to be doing anything to fix up the '__attribute__((packed))' annotations. Presumably you're intending to put standard-headers/ on the include path? It would probably be better to not make the headers be in "linux/" in that case, since it would mean confusion/clashes for what "linux/virtio_net.h" etc mean -- are they the QEMU sanitized versions or the host OS's? (Having them in linux/ also makes code review harder since it breaks the current rule of thumb that is "no include of linux/anything in code that's not Linux-host-specific".) You need to strip out all the #include from these headers, otherwise this won't build on non Linux hosts. Then you need to add in whatever the equivalent is to get the defines/types those includes were providing (for instance our virtio-net.h does a simple #define of ETH_ALEN). You probably want to make the update script fail if there's an include it's not expecting to deal with, otherwise you're likely to end up with a set of headers that seem OK on Linux but fail when tested on other OSes -- better to fail early and for the person trying to do the header update than to end up with a change that won't pass my build tests and gets bounced. All that makes it seem to me like it's more trouble than it's worth compared to doing a one-time manual import. -- PMM
[Qemu-devel] block: low level read from disk for IDE_DMA_READ
Hi, I am implementing read equivalent routine in qemu. Can some one help me understand control flow of the qemu read/write implementation. I am using xen-4.2.0 and qemu-1.6.1 My requirement is simple: I have a 1024*1024 buffer already filled with some useful data. Now when windows (my guest OS) does IDE_DMA_READ command to the disk, I want to intercept it and fill data from my private buffer. my intention is to leverage existing dma_read infrastructure and overwrite the read buffer-data at the lowest level of qemu . That way the buffers /vectors "qiov" which are prepared due to cmd IDE_DMA_READ will return data from my data from buffer to guest-OS. I could trace the control from. ide_sector_start_dma -> s->bus->dma->ops->start_dma -> ide_dma_cb ->dma_bdrv_read -> bdrv_aio_readv . ->bdrv_co_aio_rw_vector -> bdrv_co_do_rw "coroutine" -> bdrv_co_do_readv -> drv->bdrv_co_readv (( in my case it is from raw.c raw_co_readv )) -> bdrv_co_readv -> bdrv_co_do_readv ->in bdrv_co_do_rw the bottom half is scheduled bdrv_co_em_bh -->> this will invoke -> ide_dma_cb () which is again the starting point. Looks like there a double-linked list maintained for the coroutine entries and are off loaded to qemu-wait queue during this process. Now I need help to understand where to look for to find the last read/write system call which will get the data out from the disk for guest-OS (windows) . I am seeking suggestions and help for the same. thanks Kumar
Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough
On 2015/2/9 19:05, Ian Campbell wrote: On Mon, 2015-02-09 at 14:28 +0800, Chen, Tiejun wrote: What about this? I've not read the code in detail,since I'm travelling but from a quick glance it looks to be implementing the sort of thing I meant, thanks. Thanks for your time. A couple of higher level comments: I'd suggest to put the code for reading the vid/did into a helper function so it can be reused. Looks good. You might like to optionally consider add a forcing option somehow so that people with new devices not in the list can control things without the need to recompile (e.g. gfx_passthru_kind_override?). Perhaps that isn't needed for a first cut though and it would be a libxl API so thought required. What about 'gfx_passthru_force'? Because what we're doing is, we want to make sure if we have a such a IGD that needs to workaround by posting a parameter to qemu. So in case of non-listed devices we just need to provide a bool to force this regardless of that real device. I think it should probably log something at a lowish level when it has autodetected IGD. So I tried to rebase that according to your all comments, diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 98687bd..398d9da 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -361,6 +361,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, libxl_defbool_setdefault(&b_info->u.hvm.nographic, false); libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru, false); +libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru_force, false); break; case LIBXL_DOMAIN_TYPE_PV: diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 8599a6a..507034f 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -710,9 +710,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, flexarray_append(dm_args, "-net"); flexarray_append(dm_args, "none"); } -if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { -flexarray_append(dm_args, "-gfx_passthru"); -} } else { if (!sdl && !vnc) { flexarray_append(dm_args, "-nographic"); @@ -757,6 +754,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, machinearg, max_ram_below_4g); } } + +if (libxl__is_igd_vga_passthru(gc, guest_config)) { +machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg); +} + flexarray_append(dm_args, machinearg); for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) flexarray_append(dm_args, b_info->extra_hvm[i]); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 934465a..35ec5fc 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1177,6 +1177,9 @@ _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int num); _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid); +_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc, + const libxl_domain_config *d_config); + /*- xswait: wait for a xenstore node to be suitable -*/ typedef struct libxl__xswait_state libxl__xswait_state; diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index f3ae132..07b9e22 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev, return 0; } +static unsigned long sysfs_dev_get_vendor(libxl__gc *gc, + libxl_device_pci *pcidev) +{ +char *pci_device_vendor_path = +libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor", + pcidev->domain, pcidev->bus, pcidev->dev, + pcidev->func); +int read_items; +unsigned long pci_device_vendor; + +FILE *f = fopen(pci_device_vendor_path, "r"); +if (!f) { +LOGE(ERROR, + "pci device "PCI_BDF" does not have vendor attribute", + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); +return 0x; +} +read_items = fscanf(f, "0x%lx\n", &pci_device_vendor); +fclose(f); +if (read_items != 1) { +LOGE(ERROR, + "cannot read vendor of pci device "PCI_BDF, + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); +return 0x; +} + +return pci_device_vendor; +} + +static unsigned long sysfs_dev_get_device(libxl__gc *gc, + libxl_device_pci *pcidev) +{ +char *pci_device_device_path = +libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/device", + pcidev->domain, pcidev->bus, pcidev->dev,
Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs
On 2015/2/11 10:03, Peter Maydell wrote: On 11 February 2015 at 01:36, Chen, Tiejun wrote: On 2015/2/10 3:56, Michael S. Tsirkin wrote: It doesn't make sense to copy values manually: the only issue with getting headers from linux seems to be dealing with linux/types, we can easily fix that automatically while importing. Signed-off-by: Michael S. Tsirkin --- FYI this is what I propose instead of the recently suggested virtio: uniform virtio device IDs we can then rework existing code to include these headers. Will automatically bring in goodies as they arrive in linux. This doesn't yet import virtio ccw header, that won't be hard to add later. scripts/update-linux-headers.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index c8e026d..0bd8437 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -76,4 +76,14 @@ else cp "$linux/COPYING" "$output/linux-headers" fi +rm -rf "$output/standard-headers/linux" +mkdir -p "$output/standard-headers/linux" Shouldn't we add something in configure file to execute this automatically? No. We want to run this script only when we're updating the header files, which only happens when we have a valid kernel source tree available and you're a developer doing it as a specific action. configure is run by everybody and should definitely not be doing header updates. Or instead of creating 'standard-headers/, why can't we go that existing linux-headers/? The linux-headers/ directory contains header files which can only validly be included if the host we're compiling on is Linux. Some of them will cause compile failures on OSX or Windows if they are in the include path. The idea of this patch is that the standard-headers/ directory has "sanitized" header files which have had the linux-specific types and includes stripped out. So if we take the route this patch proposes we do need two directories. This confounds me since for instance, one of goals based on this patch is, it exposes those Virtio devices ID definition to hw/virtio, instead of my original patch, right? So without this sort of standard-hearders, how can we compile virtio? Or you mean we still keep those original stuff in include/hw/virtio*, but somehow update them once we execute that script manually. Thanks Tiejun
Re: [Qemu-devel] [PATCH 7/9] cosmetic changes preparing for the following patches
On Fri, 02/06 17:55, Paolo Bonzini wrote: > From: Mike Day > > Signed-off-by: Mike Day > Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng
[Qemu-devel] [v5 02/12] migration: Add the framework of multi-thread compression
Add the code to create and destroy the multiple threads those will be used to do data compression. Left some functions empty to keep clearness, and the code will be added later. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert --- arch_init.c | 79 ++- include/migration/migration.h | 9 + migration/migration.c | 37 3 files changed, 124 insertions(+), 1 deletion(-) diff --git a/arch_init.c b/arch_init.c index 89c8fa4..709036c 100644 --- a/arch_init.c +++ b/arch_init.c @@ -332,6 +332,68 @@ static uint64_t migration_dirty_pages; static uint32_t last_version; static bool ram_bulk_stage; +struct CompressParam { +/* To be done */ +}; +typedef struct CompressParam CompressParam; + +static CompressParam *comp_param; +static bool quit_comp_thread; + +static void *do_data_compress(void *opaque) +{ +while (!quit_comp_thread) { + +/* To be done */ + +} + +return NULL; +} + +static inline void terminate_compression_threads(void) +{ +quit_comp_thread = true; + +/* To be done */ +} + +void migrate_compress_threads_join(MigrationState *s) +{ +int i, thread_count; + +if (!migrate_use_compression()) { +return; +} +terminate_compression_threads(); +thread_count = migrate_compress_threads(); +for (i = 0; i < thread_count; i++) { +qemu_thread_join(s->compress_thread + i); +} +g_free(s->compress_thread); +g_free(comp_param); +s->compress_thread = NULL; +comp_param = NULL; +} + +void migrate_compress_threads_create(MigrationState *s) +{ +int i, thread_count; + +if (!migrate_use_compression()) { +return; +} +quit_comp_thread = false; +thread_count = migrate_compress_threads(); +s->compress_thread = g_new0(QemuThread, thread_count); +comp_param = g_new0(CompressParam, thread_count); +for (i = 0; i < thread_count; i++) { +qemu_thread_create(s->compress_thread + i, "compress", + do_data_compress, comp_param + i, + QEMU_THREAD_JOINABLE); +} +} + /* Update the xbzrle cache to reflect a page that's been sent as all 0. * The important thing is that a stale (not-yet-0'd) page be replaced * by the new data. @@ -645,6 +707,16 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, return bytes_sent; } +static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block, +ram_addr_t offset, bool last_stage) +{ +int bytes_sent = -1; + +/* To be done*/ + +return bytes_sent; +} + /* * ram_find_and_save_block: Finds a page to send and sends it to f * @@ -679,7 +751,12 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage) ram_bulk_stage = false; } } else { -bytes_sent = ram_save_page(f, block, offset, last_stage); +if (migrate_use_compression()) { +bytes_sent = ram_save_compressed_page(f, block, offset, + last_stage); +} else { +bytes_sent = ram_save_page(f, block, offset, last_stage); +} /* if page is unmodified, continue to the next */ if (bytes_sent > 0) { diff --git a/include/migration/migration.h b/include/migration/migration.h index f37348b..228badb 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -50,6 +50,9 @@ struct MigrationState QemuThread thread; QEMUBH *cleanup_bh; QEMUFile *file; +QemuThread *compress_thread; +int compress_thread_count; +int compress_level; int state; MigrationParams params; @@ -108,6 +111,8 @@ bool migration_has_finished(MigrationState *); bool migration_has_failed(MigrationState *); MigrationState *migrate_get_current(void); +void migrate_compress_threads_create(MigrationState *s); +void migrate_compress_threads_join(MigrationState *s); uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_transferred(void); uint64_t ram_bytes_total(void); @@ -157,6 +162,10 @@ int64_t migrate_xbzrle_cache_size(void); int64_t xbzrle_cache_resize(int64_t new_size); +bool migrate_use_compression(void); +int migrate_compress_level(void); +int migrate_compress_threads(void); + void ram_control_before_iterate(QEMUFile *f, uint64_t flags); void ram_control_after_iterate(QEMUFile *f, uint64_t flags); void ram_control_load_hook(QEMUFile *f, uint64_t flags); diff --git a/migration/migration.c b/migration/migration.c index b3adbc6..309443e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -43,6 +43,11 @@ enum { #define BUFFER_DELAY 100 #define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY) +/* Default compression thread count */ +#define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8 +/*0: means nocompress, 1: best speed, ...
[Qemu-devel] [PATCH v5 0/12] migration: Add a new feature to do live migration
This feature can help to reduce the data transferred about 60%, and the migration time can also be reduced about 70%. Summary of changed from v4->v5 -Fix some typo errors -Fix the bug related with XBZRLE -Added some comments -Squash setting and querying commands into one patch -Fix the issue when doing migrate_cancel arch_init.c | 485 -- docs/multi-thread-compression.txt | 149 hmp-commands.hx | 17 ++ hmp.c | 56 + hmp.h | 4 + include/migration/migration.h | 11 + include/migration/qemu-file.h | 3 + migration/migration.c | 130 ++ migration/qemu-file.c | 39 +++ monitor.c | 25 ++ qapi-schema.json | 93 +++- qmp-commands.hx | 66 ++ 12 files changed, 1056 insertions(+), 22 deletions(-) create mode 100644 docs/multi-thread-compression.txt
[Qemu-devel] [v5 04/12] qemu-file: Add compression functions to QEMUFile
qemu_put_compression_data() compress the data and put it to QEMUFile. qemu_put_qemu_file() put the data in the buffer of source QEMUFile to destination QEMUFile. Signed-off-by: Liang Li Signed-off-by: Yang Zhang --- include/migration/qemu-file.h | 3 +++ migration/qemu-file.c | 39 +++ 2 files changed, 42 insertions(+) diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h index a923cec..52192ad 100644 --- a/include/migration/qemu-file.h +++ b/include/migration/qemu-file.h @@ -160,6 +160,9 @@ void qemu_put_be32(QEMUFile *f, unsigned int v); void qemu_put_be64(QEMUFile *f, uint64_t v); int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset); int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size); +ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size, + int level); +int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src); /* * Note that you can only peek continuous bytes from where the current pointer * is; you aren't guaranteed to be able to peak to +n bytes unless you've diff --git a/migration/qemu-file.c b/migration/qemu-file.c index e66e557..3e54f6d 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include #include "qemu-common.h" #include "qemu/iov.h" #include "qemu/sockets.h" @@ -545,3 +546,41 @@ uint64_t qemu_get_be64(QEMUFile *f) v |= qemu_get_be32(f); return v; } + +/* compress size bytes of data start at p with specific compression + * level and store the compressed data to the buffer of f. + */ + +ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size, + int level) +{ +ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t); + +if (blen < compressBound(size)) { +return 0; +} +if (compress2(f->buf + f->buf_index + sizeof(int32_t), (uLongf *)&blen, + (Bytef *)p, size, level) != Z_OK) { +error_report("Compress Failed!"); +return 0; +} +qemu_put_be32(f, blen); +f->buf_index += blen; +return blen + sizeof(int32_t); +} + +/* Put the data in the buffer of f_src to the buffer of f_des, and + * then reset the buf_index of f_src to 0. + */ + +int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src) +{ +int len = 0; + +if (f_src->buf_index > 0) { +len = f_src->buf_index; +qemu_put_buffer(f_des, f_src->buf, f_src->buf_index); +f_src->buf_index = 0; +} +return len; +} -- 1.9.1
[Qemu-devel] [v5 06/12] arch_init: Add and free data struct for decompression
Define the data structure and variables used to do multiple thread decompression, and add the code to initialize and free them. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert --- arch_init.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/arch_init.c b/arch_init.c index 7ccad8b..66f4bc1 100644 --- a/arch_init.c +++ b/arch_init.c @@ -345,7 +345,12 @@ struct CompressParam { typedef struct CompressParam CompressParam; struct DecompressParam { -/* To be done */ +bool busy; +QemuMutex mutex; +QemuCond cond; +void *des; +uint8 *compbuf; +int len; }; typedef struct DecompressParam DecompressParam; @@ -1189,6 +1194,9 @@ void migrate_decompress_threads_create(int count) compressed_data_buf = g_malloc0(compressBound(TARGET_PAGE_SIZE)); quit_decomp_thread = false; for (i = 0; i < count; i++) { +qemu_mutex_init(&decomp_param[i].mutex); +qemu_cond_init(&decomp_param[i].cond); +decomp_param[i].compbuf = g_malloc0(compressBound(TARGET_PAGE_SIZE)); qemu_thread_create(decompress_threads + i, "decompress", do_data_decompress, decomp_param + i, QEMU_THREAD_JOINABLE); @@ -1203,6 +1211,9 @@ void migrate_decompress_threads_join(void) thread_count = migrate_decompress_threads(); for (i = 0; i < thread_count; i++) { qemu_thread_join(decompress_threads + i); +qemu_mutex_destroy(&decomp_param[i].mutex); +qemu_cond_destroy(&decomp_param[i].cond); +g_free(decomp_param[i].compbuf); } g_free(decompress_threads); g_free(decomp_param); -- 1.9.1
[Qemu-devel] [v5 01/12] docs: Add a doc about multiple thread compression
Give some details about the multiple thread (de)compression and how to use it in live migration. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert --- docs/multi-thread-compression.txt | 149 ++ 1 file changed, 149 insertions(+) create mode 100644 docs/multi-thread-compression.txt diff --git a/docs/multi-thread-compression.txt b/docs/multi-thread-compression.txt new file mode 100644 index 000..0d4d212 --- /dev/null +++ b/docs/multi-thread-compression.txt @@ -0,0 +1,149 @@ +Use multiple thread (de)compression in live migration += +Copyright (C) 2015 Intel Corporation +Author: Liang Li + +This work is licensed under the terms of the GNU GPLv2 or later. See +the COPYING file in the top-level directory. + +Contents: += +* Introduction +* When to use +* Performance +* Usage +* TODO + +Introduction + +Instead of sending the guest memory directly, this solution will +compress the RAM page before sending; after receiving, the data will +be decompressed. Using compression in live migration can help +to reduce the data transferred about 60%, this is very useful when the +bandwidth is limited, and the total migration time can also be reduced +about 70% in a typical case. In addition to this, the VM downtime can be +reduced about 50%. The benefit depends on data's compressibility in VM. + +The process of compression will consume additional CPU cycles, and the +extra CPU cycles will increase the migration time. On the other hand, +the amount of data transferred will decrease; this factor can reduce +the total migration time. If the process of the compression is quick +enough, then the total migration time can be reduced, and multiple +thread compression can be used to accelerate the compression process. + +The decompression speed of Zlib is at least 4 times as quick as +compression, if the source and destination CPU have equal speed, +keeping the compression thread count 4 times the decompression +thread count can avoid CPU waste. + +Compression level can be used to control the compression speed and the +compression ratio. High compression ratio will take more time, level 0 +stands for no compression, level 1 stands for the best compression +speed, and level 9 stands for the best compression ratio. Users can +select a level number between 0 and 9. + + +When to use the multiple thread compression in live migration += +Compression of data will consume extra CPU cycles; so in a system with +high overhead of CPU, avoid using this feature. When the network +bandwidth is very limited and the CPU resource is adequate, use of +multiple thread compression will be very helpful. If both the CPU and +the network bandwidth are adequate, use of multiple thread compression +can still help to reduce the migration time. + +Performance +=== +Test environment: + +CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz +Socket Count: 2 +RAM: 128G +NIC: Intel I350 (10/100/1000Mbps) +Host OS: CentOS 7 64-bit +Guest OS: RHEL 6.5 64-bit +Parameter: qemu-system-x86_64 -enable-kvm -smp 4 -m 4096 + /share/ia32e_rhel6u5.qcow -monitor stdio + +There is no additional application is running on the guest when doing +the test. + + +Speed limit: 1000Gb/s +--- +| original | compress thread: 8 +| way | decompress thread: 2 +| | compression level: 1 +--- +total time(msec): | | 1833 +--- +downtime(msec): |100| 27 +--- +transferred ram(kB):| 363536 | 107819 +--- +throughput(mbps): | 893.73 | 482.22 +--- +total ram(kB): | 4211524 | 4211524 +--- + +There is an application running on the guest which write random numbers +to RAM block areas periodically. + +Speed limit: 1000Gb/s +--- +| original | compress thread: 8 +| way | decompress thread: 2 +| | compression level: 1 +--- +total time(msec): | 37369 | 15989 +--- +downtime(msec): |337| 173 +--- +transferred ram(kB):| 4274143 | 1699824 +--- +throughput(mbps): | 936.99 | 870.95
[Qemu-devel] [v5 03/12] migration: Add the framework of multi-thread decompression
Add the code to create and destroy the multiple threads those will be used to do data decompression. Left some functions empty just to keep clearness, and the code will be added later. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert --- arch_init.c | 76 +++ include/migration/migration.h | 4 +++ migration/migration.c | 18 ++ 3 files changed, 98 insertions(+) diff --git a/arch_init.c b/arch_init.c index 709036c..bbb584c 100644 --- a/arch_init.c +++ b/arch_init.c @@ -24,6 +24,7 @@ #include #include #include +#include #ifndef _WIN32 #include #include @@ -126,6 +127,7 @@ static uint64_t bitmap_sync_count; #define RAM_SAVE_FLAG_CONTINUE 0x20 #define RAM_SAVE_FLAG_XBZRLE 0x40 /* 0x80 is reserved in migration.h start with 0x100 next */ +#define RAM_SAVE_FLAG_COMPRESS_PAGE0x100 static struct defconfig_file { const char *filename; @@ -337,8 +339,17 @@ struct CompressParam { }; typedef struct CompressParam CompressParam; +struct DecompressParam { +/* To be done */ +}; +typedef struct DecompressParam DecompressParam; + static CompressParam *comp_param; static bool quit_comp_thread; +static bool quit_decomp_thread; +static DecompressParam *decomp_param; +static QemuThread *decompress_threads; +static uint8_t *compressed_data_buf; static void *do_data_compress(void *opaque) { @@ -1128,10 +1139,58 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size) } } +static void *do_data_decompress(void *opaque) +{ +while (!quit_decomp_thread) { +/* To be done */ +} + +return NULL; +} + +void migrate_decompress_threads_create(int count) +{ +int i; + +decompress_threads = g_new0(QemuThread, count); +decomp_param = g_new0(DecompressParam, count); +compressed_data_buf = g_malloc0(compressBound(TARGET_PAGE_SIZE)); +quit_decomp_thread = false; +for (i = 0; i < count; i++) { +qemu_thread_create(decompress_threads + i, "decompress", + do_data_decompress, decomp_param + i, + QEMU_THREAD_JOINABLE); +} +} + +void migrate_decompress_threads_join(void) +{ +int i, thread_count; + +quit_decomp_thread = true; +thread_count = migrate_decompress_threads(); +for (i = 0; i < thread_count; i++) { +qemu_thread_join(decompress_threads + i); +} +g_free(decompress_threads); +g_free(decomp_param); +g_free(compressed_data_buf); +decompress_threads = NULL; +decomp_param = NULL; +compressed_data_buf = NULL; +} + +static void decompress_data_with_multi_threads(uint8_t *compbuf, + void *host, int len) +{ +/* To be done */ +} + static int ram_load(QEMUFile *f, void *opaque, int version_id) { int flags = 0, ret = 0; static uint64_t seq_iter; +int len = 0; seq_iter++; @@ -1208,6 +1267,23 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) qemu_get_buffer(f, host, TARGET_PAGE_SIZE); break; +case RAM_SAVE_FLAG_COMPRESS_PAGE: +host = host_from_stream_offset(f, addr, flags); +if (!host) { +error_report("Invalid RAM offset " RAM_ADDR_FMT, addr); +ret = -EINVAL; +break; +} + +len = qemu_get_be32(f); +if (len < 0 || len > compressBound(TARGET_PAGE_SIZE)) { +error_report("Invalid compressed data length: %d", len); +ret = -EINVAL; +break; +} +qemu_get_buffer(f, compressed_data_buf, len); +decompress_data_with_multi_threads(compressed_data_buf, host, len); +break; case RAM_SAVE_FLAG_XBZRLE: host = host_from_stream_offset(f, addr, flags); if (!host) { diff --git a/include/migration/migration.h b/include/migration/migration.h index 228badb..9ac1b23 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -52,6 +52,7 @@ struct MigrationState QEMUFile *file; QemuThread *compress_thread; int compress_thread_count; +int decompress_thread_count; int compress_level; int state; @@ -113,6 +114,8 @@ MigrationState *migrate_get_current(void); void migrate_compress_threads_create(MigrationState *s); void migrate_compress_threads_join(MigrationState *s); +void migrate_decompress_threads_create(int count); +void migrate_decompress_threads_join(void); uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_transferred(void); uint64_t ram_bytes_total(void); @@ -165,6 +168,7 @@ int64_t xbzrle_cache_resize(int64_t new_size); bool migrate_use_compression(void); int migrate_compress_level(void); int migrate_compress_threads(void); +int migrate_decompress_threads(void); void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
[Qemu-devel] [PATCH v17 1/2] sPAPR: Implement EEH RTAS calls
The emulation for EEH RTAS requests from guest isn't covered by QEMU yet and the patch implements them. The patch defines constants used by EEH RTAS calls and adds callbacks sPAPRPHBClass::{eeh_set_option, eeh_get_state, eeh_reset, eeh_configure}, which are going to be used as follows: * RTAS calls are received in spapr_pci.c, sanity check is done there. * RTAS handlers handle what they can. If there is something it cannot handle and the corresponding sPAPRPHBClass callback is defined, it is called. * Those callbacks are only implemented for VFIO now. They do ioctl() to the IOMMU container fd to complete the calls. Error codes from that ioctl() are transferred back to the guest. [aik: defined RTAS tokens for EEH RTAS calls] Signed-off-by: Gavin Shan --- hw/ppc/spapr_pci.c | 295 include/hw/pci-host/spapr.h | 4 + include/hw/ppc/spapr.h | 43 ++- 3 files changed, 340 insertions(+), 2 deletions(-) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 6deeb19..9c050e1 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -406,6 +406,282 @@ static void rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ } +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu, +sPAPREnvironment *spapr, +uint32_t token, uint32_t nargs, +target_ulong args, uint32_t nret, +target_ulong rets) +{ +sPAPRPHBState *sphb; +sPAPRPHBClass *spc; +uint32_t addr, option; +uint64_t buid; +int ret; + +if ((nargs != 4) || (nret != 1)) { +goto param_error_exit; +} + +buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); +addr = rtas_ld(args, 0); +option = rtas_ld(args, 3); + +sphb = find_phb(spapr, buid); +if (!sphb) { +goto param_error_exit; +} + +spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); +if (!spc->eeh_set_option) { +goto param_error_exit; +} + +switch (option) { +case RTAS_EEH_ENABLE: +if (!find_dev(spapr, buid, addr)) { +goto param_error_exit; +} +break; +case RTAS_EEH_DISABLE: +case RTAS_EEH_THAW_IO: +case RTAS_EEH_THAW_DMA: +break; +default: +goto param_error_exit; +} + +ret = spc->eeh_set_option(sphb, option); +rtas_st(rets, 0, ret); +return; + +param_error_exit: +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); +} + +static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, + sPAPREnvironment *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ +sPAPRPHBState *sphb; +sPAPRPHBClass *spc; +PCIDevice *pdev; +uint32_t addr, option; +uint64_t buid; + +if ((nargs != 4) || (nret != 2)) { +goto param_error_exit; +} + +buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); +sphb = find_phb(spapr, buid); +if (!sphb) { +goto param_error_exit; +} + +spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); +if (!spc->eeh_set_option) { +goto param_error_exit; +} + +/* + * We always have PE address of form "00BB0001". "BB" + * represents the bus number of PE's primary bus. + */ +option = rtas_ld(args, 3); +switch (option) { +case RTAS_GET_PE_ADDR: +addr = rtas_ld(args, 0); +pdev = find_dev(spapr, buid, addr); +if (!pdev) { +goto param_error_exit; +} + +rtas_st(rets, 1, (pci_bus_num(pdev->bus) << 16) + 1); +break; +case RTAS_GET_PE_MODE: +rtas_st(rets, 1, RTAS_PE_MODE_SHARED); +break; +default: +goto param_error_exit; +} + +rtas_st(rets, 0, RTAS_OUT_SUCCESS); +return; + +param_error_exit: +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); +} + +static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu, +sPAPREnvironment *spapr, +uint32_t token, uint32_t nargs, +target_ulong args, uint32_t nret, +target_ulong rets) +{ +sPAPRPHBState *sphb; +sPAPRPHBClass *spc; +uint64_t buid; +int state, ret; + +if ((nargs != 3) || (nret != 4 && nret != 5)) { +goto param_error_exit; +} + +buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); +sphb = find_phb(spapr, buid); +if (!sphb) { +goto param_error_exit; +} + +spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); +if (!spc->eeh_get_state) { +goto param_error_exit; +}
[Qemu-devel] [PATCH v17 2/2] sPAPR: Implement sPAPRPHBClass EEH callbacks
The patch implements sPAPRPHBClass EEH callbacks so that the EEH RTAS requests can be routed to VFIO for further handling. Signed-off-by: Gavin Shan --- hw/ppc/spapr_pci_vfio.c | 98 + hw/vfio/common.c| 1 + 2 files changed, 99 insertions(+) diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c index 144912b..285d8d5 100644 --- a/hw/ppc/spapr_pci_vfio.c +++ b/hw/ppc/spapr_pci_vfio.c @@ -76,6 +76,100 @@ static void spapr_phb_vfio_reset(DeviceState *qdev) /* Do nothing */ } +static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, int option) +{ +sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); +struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; +int ret; + +switch (option) { +case RTAS_EEH_DISABLE: +op.op = VFIO_EEH_PE_DISABLE; +break; +case RTAS_EEH_ENABLE: +op.op = VFIO_EEH_PE_ENABLE; +break; +case RTAS_EEH_THAW_IO: +op.op = VFIO_EEH_PE_UNFREEZE_IO; +break; +case RTAS_EEH_THAW_DMA: +op.op = VFIO_EEH_PE_UNFREEZE_DMA; +break; +default: +return RTAS_OUT_PARAM_ERROR; +} + +ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid, + VFIO_EEH_PE_OP, &op); +if (ret < 0) { +return RTAS_OUT_HW_ERROR; +} + +return RTAS_OUT_SUCCESS; +} + +static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state) +{ +sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); +struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; +int ret; + +op.op = VFIO_EEH_PE_GET_STATE; +ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid, + VFIO_EEH_PE_OP, &op); +if (ret < 0) { +return RTAS_OUT_PARAM_ERROR; +} + +*state = ret; +return RTAS_OUT_SUCCESS; +} + +static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) +{ +sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); +struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; +int ret; + +switch (option) { +case RTAS_SLOT_RESET_DEACTIVATE: +op.op = VFIO_EEH_PE_RESET_DEACTIVATE; +break; +case RTAS_SLOT_RESET_HOT: +op.op = VFIO_EEH_PE_RESET_HOT; +break; +case RTAS_SLOT_RESET_FUNDAMENTAL: +op.op = VFIO_EEH_PE_RESET_FUNDAMENTAL; +break; +default: +return RTAS_OUT_PARAM_ERROR; +} + +ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid, + VFIO_EEH_PE_OP, &op); +if (ret < 0) { +return RTAS_OUT_HW_ERROR; +} + +return RTAS_OUT_SUCCESS; +} + +static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) +{ +sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); +struct vfio_eeh_pe_op op = { .argsz = sizeof(op) }; +int ret; + +op.op = VFIO_EEH_PE_CONFIGURE; +ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid, + VFIO_EEH_PE_OP, &op); +if (ret < 0) { +return RTAS_OUT_PARAM_ERROR; +} + +return RTAS_OUT_SUCCESS; +} + static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -84,6 +178,10 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) dc->props = spapr_phb_vfio_properties; dc->reset = spapr_phb_vfio_reset; spc->finish_realize = spapr_phb_vfio_finish_realize; +spc->eeh_set_option = spapr_phb_vfio_eeh_set_option; +spc->eeh_get_state = spapr_phb_vfio_eeh_get_state; +spc->eeh_reset = spapr_phb_vfio_eeh_reset; +spc->eeh_configure = spapr_phb_vfio_eeh_configure; } static const TypeInfo spapr_phb_vfio_info = { diff --git a/hw/vfio/common.c b/hw/vfio/common.c index cf483ff..8a10c8b 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -948,6 +948,7 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid, switch (req) { case VFIO_CHECK_EXTENSION: case VFIO_IOMMU_SPAPR_TCE_GET_INFO: +case VFIO_EEH_PE_OP: break; default: /* Return an error on unknown requests */ -- 1.8.3.2
[Qemu-devel] [v5 05/12] arch_init: Alloc and free data struct for compression
Define the data structure and variables used to do multiple thread compression, and add the code to initialize and free them. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert --- arch_init.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/arch_init.c b/arch_init.c index bbb584c..7ccad8b 100644 --- a/arch_init.c +++ b/arch_init.c @@ -335,7 +335,12 @@ static uint32_t last_version; static bool ram_bulk_stage; struct CompressParam { -/* To be done */ +bool busy; +QEMUFile *file; +QemuMutex mutex; +QemuCond cond; +RAMBlock *block; +ram_addr_t offset; }; typedef struct CompressParam CompressParam; @@ -345,6 +350,14 @@ struct DecompressParam { typedef struct DecompressParam DecompressParam; static CompressParam *comp_param; +/* comp_done_cond is used to wake up the migration thread when + * one of the compression threads has finished the compression. + * comp_done_lock is used to co-work with comp_done_cond. + */ +static QemuMutex *comp_done_lock; +static QemuCond *comp_done_cond; +/* The empty QEMUFileOps will be used by file in CompressParam */ +static const QEMUFileOps empty_ops = { }; static bool quit_comp_thread; static bool quit_decomp_thread; static DecompressParam *decomp_param; @@ -380,11 +393,20 @@ void migrate_compress_threads_join(MigrationState *s) thread_count = migrate_compress_threads(); for (i = 0; i < thread_count; i++) { qemu_thread_join(s->compress_thread + i); +qemu_fclose(comp_param[i].file); +qemu_mutex_destroy(&comp_param[i].mutex); +qemu_cond_destroy(&comp_param[i].cond); } +qemu_mutex_destroy(comp_done_lock); +qemu_cond_destroy(comp_done_cond); g_free(s->compress_thread); g_free(comp_param); +g_free(comp_done_cond); +g_free(comp_done_lock); s->compress_thread = NULL; comp_param = NULL; +comp_done_cond = NULL; +comp_done_lock = NULL; } void migrate_compress_threads_create(MigrationState *s) @@ -398,7 +420,17 @@ void migrate_compress_threads_create(MigrationState *s) thread_count = migrate_compress_threads(); s->compress_thread = g_new0(QemuThread, thread_count); comp_param = g_new0(CompressParam, thread_count); +comp_done_cond = g_new0(QemuCond, 1); +comp_done_lock = g_new0(QemuMutex, 1); +qemu_cond_init(comp_done_cond); +qemu_mutex_init(comp_done_lock); for (i = 0; i < thread_count; i++) { +/* com_param[i].file is just used as a dummy buffer to save data, set + * it's ops to empty. + */ +comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops); +qemu_mutex_init(&comp_param[i].mutex); +qemu_cond_init(&comp_param[i].cond); qemu_thread_create(s->compress_thread + i, "compress", do_data_compress, comp_param + i, QEMU_THREAD_JOINABLE); -- 1.9.1
[Qemu-devel] [v5 07/12] migration: Split the function ram_save_page
Split the function ram_save_page for code reuse purpose. Signed-off-by: Liang Li Signed-off-by: Yang Zhang --- arch_init.c | 51 +-- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/arch_init.c b/arch_init.c index 66f4bc1..fe062db 100644 --- a/arch_init.c +++ b/arch_init.c @@ -681,12 +681,28 @@ static void migration_bitmap_sync(void) } } +static int save_zero_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset, + uint8_t *p, int cont) +{ +int bytes_sent = -1; + +if (is_zero_range(p, TARGET_PAGE_SIZE)) { +acct_info.dup_pages++; +bytes_sent = save_block_hdr(f, block, offset, cont, +RAM_SAVE_FLAG_COMPRESS); +qemu_put_byte(f, 0); +bytes_sent++; +} + +return bytes_sent; +} + /* * ram_save_page: Send the given page to the stream * * Returns: Number of bytes written. */ -static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, +static int ram_save_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset, bool last_stage) { int bytes_sent; @@ -717,24 +733,23 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, acct_info.dup_pages++; } } -} else if (is_zero_range(p, TARGET_PAGE_SIZE)) { -acct_info.dup_pages++; -bytes_sent = save_block_hdr(f, block, offset, cont, -RAM_SAVE_FLAG_COMPRESS); -qemu_put_byte(f, 0); -bytes_sent++; -/* Must let xbzrle know, otherwise a previous (now 0'd) cached - * page would be stale - */ -xbzrle_cache_zero_page(current_addr); -} else if (!ram_bulk_stage && migrate_use_xbzrle()) { -bytes_sent = save_xbzrle_page(f, &p, current_addr, block, - offset, cont, last_stage); -if (!last_stage) { -/* Can't send this cached data async, since the cache page - * might get updated before it gets to the wire +} else { +bytes_sent = save_zero_page(f, block, offset, p, cont); +if (bytes_sent > 0) { + +/* Must let xbzrle know, otherwise a previous (now 0'd) cached + * page would be stale */ -send_async = false; +xbzrle_cache_zero_page(current_addr); +} else if (!ram_bulk_stage && migrate_use_xbzrle()) { +bytes_sent = save_xbzrle_page(f, &p, current_addr, block, + offset, cont, last_stage); +if (!last_stage) { +/* Can't send this cached data async, since the cache page + * might get updated before it gets to the wire + */ +send_async = false; +} } } -- 1.9.1
[Qemu-devel] [v5 09/12] migration: Make compression co-work with xbzrle
Now, multiple thread compression can co-work with xbzrle. when xbzrle is on, multiple thread compression will only work at the first round of RAM data sync. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert --- arch_init.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch_init.c b/arch_init.c index 17b7f15..12dfa34 100644 --- a/arch_init.c +++ b/arch_init.c @@ -370,6 +370,7 @@ static const QEMUFileOps empty_ops = { }; * make bytes_transferred accurate. */ static int one_byte_count; +static bool compression_switch; static bool quit_comp_thread; static bool quit_decomp_thread; static DecompressParam *decomp_param; @@ -454,6 +455,7 @@ void migrate_compress_threads_create(MigrationState *s) return; } quit_comp_thread = false; +compression_switch = true; thread_count = migrate_compress_threads(); s->compress_thread = g_new0(QemuThread, thread_count); comp_param = g_new0(CompressParam, thread_count); @@ -989,9 +991,16 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage) block = QTAILQ_FIRST(&ram_list.blocks); complete_round = true; ram_bulk_stage = false; +if (migrate_use_xbzrle()) { +/* If xbzrle is on, stop using the data compression at this + * point. In theory, xbzrle can do better than compression. + */ +flush_compressed_data(f); +compression_switch = false; +} } } else { -if (migrate_use_compression()) { +if (compression_switch && migrate_use_compression()) { bytes_sent = ram_save_compressed_page(f, block, offset, last_stage); } else { -- 1.9.1
[Qemu-devel] [PATCH v17 0/2] EEH Support for VFIO Devices
The series of patches adds support EEH for VFIO PCI devices on sPAPR platform. It requires corresponding host kernel support, which was merged during 3.17 merge window. This patchset has been rebased to Alex Graf's QEMU repository: git://github.com/agraf/qemu.git (branch: ppc-next) The implementations notes are below. Please consider for merging! * RTAS calls are received in spapr_pci.c, sanity check is done there. RTAS handlers handle what they can. If there is something it cannot handle and sPAPRPHBClass EEH callback is defined, it is called. * sPAPRPHBClass EEH callbacks are only implemented for VFIO now. It does ioctl() to the IOMMU container fd to complete the call. Error codes from that ioctl() are transferred back to the guest. Changelog = v12 -> v13: * Rebase to Alex Graf's QEMU repository ("ppc-next" branch). * Drop the patch for header file (vfio.h) changes, which was merged to QEMU repository by commit a9fd1654 ("linux-headers: update to 3.17-rc7"). * Retested on Emulex adapter and EEH errors are recovered successfully. v13 -> v14: * Check if sPAPRPHBState instance is valid before converting it to the corresponding class as pointed by Alex Graf. v14 -> v15: * Dropped unrelated patch making find_phb()/find_dev() public. * Checking RTAS parameter number before accessing RTAS parameter buffer for more safety. * Return hardware error from RTAS call "ibm,set-eeh-option" and "ibm,set-slot-reset" for some cases according to PAPR spec. v15 -> v16: * Drop rtas_handle_eeh_request() and merge the logic to its callers so that more accurate return values can be returned for RTAS calls in the callers * Always return 1 ("No error log") for RTAS call "ibm,slot-error-detail" and correct wrong return values for other RTAS calls according to David Gibson's suggestions. * Make fall-through more obvious for case of negative return value from sPAPRPHBClass::eeh_handler() * Clear the argument buffer passed to ioctl() * Rename sPAPRPHBClass variable from "info" to "spc" v16 -> v17: * Split sPAPRPHBClass::eeh_handler() to multiple callbacks according to David Gibson's suggestion * Make comments for the form of PE address more precise and merge the condition checking on the option in rtas_ibm_get_config_addr_info2() to improve code readability * Return RTAS_OUT_PARAM_ERROR for invalid number of arguments to function rtas_ibm_slot_error_detail(), which is inconsistent with PAPR spec Gavin Shan (2): sPAPR: Implement EEH RTAS calls sPAPR: Implement sPAPRPHBClass EEH callbacks hw/ppc/spapr_pci.c | 295 hw/ppc/spapr_pci_vfio.c | 98 +++ hw/vfio/common.c| 1 + include/hw/pci-host/spapr.h | 4 + include/hw/ppc/spapr.h | 43 ++- 5 files changed, 439 insertions(+), 2 deletions(-) -- 1.8.3.2
[Qemu-devel] [v5 11/12] migration: Add interface to control compression
The multiple compression threads can be turned on/off through qmp and hmp interface before doing live migration. Signed-off-by: Liang Li Signed-off-by: Yang Zhang Reviewed-by: Dr.David Alan Gilbert Reviewed-by: Eric Blake --- migration/migration.c | 7 +-- qapi-schema.json | 7 ++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 8e15a79..55f749e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -590,8 +590,11 @@ bool migrate_zero_blocks(void) bool migrate_use_compression(void) { -/* Disable compression before the patch series are applied */ -return false; +MigrationState *s; + +s = migrate_get_current(); + +return s->enabled_capabilities[MIGRATION_CAPABILITY_COMPRESS]; } int migrate_compress_level(void) diff --git a/qapi-schema.json b/qapi-schema.json index e16f8eb..0dfc4ce 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -491,13 +491,18 @@ # to enable the capability on the source VM. The feature is disabled by # default. (since 1.6) # +# @compress: Use multiple compression threads to accelerate live migration. +# This feature can help to reduce the migration traffic, by sending +# compressed pages. The feature is disabled by default. (since 2.3) +# # @auto-converge: If enabled, QEMU will automatically throttle down the guest # to speed up convergence of RAM migration. (since 1.6) # # Since: 1.2 ## { 'enum': 'MigrationCapability', - 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks'] } + 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', + 'compress'] } ## # @MigrationCapabilityStatus -- 1.9.1
[Qemu-devel] [v5 08/12] migration: Add the core code of multi-thread compression
Implement the core logic of the multiple thread compression. At this point, multiple thread compression can't co-work with xbzrle yet. Signed-off-by: Liang Li Signed-off-by: Yang Zhang --- arch_init.c | 193 +--- 1 file changed, 185 insertions(+), 8 deletions(-) diff --git a/arch_init.c b/arch_init.c index fe062db..17b7f15 100644 --- a/arch_init.c +++ b/arch_init.c @@ -363,18 +363,44 @@ static QemuMutex *comp_done_lock; static QemuCond *comp_done_cond; /* The empty QEMUFileOps will be used by file in CompressParam */ static const QEMUFileOps empty_ops = { }; + +/* one_byte_count is used to count the bytes that is added to + * bytes_transferred but not actually transferred, at the proper + * time, we should sub one_byte_count from bytes_transferred to + * make bytes_transferred accurate. + */ +static int one_byte_count; static bool quit_comp_thread; static bool quit_decomp_thread; static DecompressParam *decomp_param; static QemuThread *decompress_threads; static uint8_t *compressed_data_buf; +static int do_compress_ram_page(CompressParam *param); + static void *do_data_compress(void *opaque) { -while (!quit_comp_thread) { - -/* To be done */ +CompressParam *param = opaque; +while (!quit_comp_thread) { +qemu_mutex_lock(¶m->mutex); +/* Re-check the quit_comp_thread in case of + * terminate_compression_threads is called just before + * qemu_mutex_lock(¶m->mutex) and after + * while(!quit_comp_thread), re-check it here can make + * sure the compression thread terminate as expected. + */ +while (!param->busy && !quit_comp_thread) { +qemu_cond_wait(¶m->cond, ¶m->mutex); +} +qemu_mutex_unlock(¶m->mutex); +if (!quit_comp_thread) { +do_compress_ram_page(param); +} +qemu_mutex_lock(comp_done_lock); +param->busy = false; +qemu_cond_signal(comp_done_cond); +qemu_mutex_unlock(comp_done_lock); } return NULL; @@ -382,9 +408,15 @@ static void *do_data_compress(void *opaque) static inline void terminate_compression_threads(void) { -quit_comp_thread = true; +int idx, thread_count; -/* To be done */ +thread_count = migrate_compress_threads(); +quit_comp_thread = true; +for (idx = 0; idx < thread_count; idx++) { +qemu_mutex_lock(&comp_param[idx].mutex); +qemu_cond_signal(&comp_param[idx].cond); +qemu_mutex_unlock(&comp_param[idx].mutex); +} } void migrate_compress_threads_join(MigrationState *s) @@ -770,12 +802,157 @@ static int ram_save_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset, return bytes_sent; } +static int do_compress_ram_page(CompressParam *param) +{ +int bytes_sent, cont; +int blen; +uint8_t *p; +RAMBlock *block = param->block; +ram_addr_t offset = param->offset; + +cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0; +p = memory_region_get_ram_ptr(block->mr) + offset; + +bytes_sent = save_block_hdr(param->file, block, offset, cont, +RAM_SAVE_FLAG_COMPRESS_PAGE); +blen = qemu_put_compression_data(param->file, p, TARGET_PAGE_SIZE, + migrate_compress_level()); +bytes_sent += blen; +atomic_inc(&acct_info.norm_pages); + +return bytes_sent; +} + +static inline void start_compression(CompressParam *param) +{ +qemu_mutex_lock(¶m->mutex); +param->busy = true; +qemu_cond_signal(¶m->cond); +qemu_mutex_unlock(¶m->mutex); +} + + +static uint64_t bytes_transferred; + +static void flush_compressed_data(QEMUFile *f) +{ +int idx, len, thread_count; + +if (!migrate_use_compression()) { +return; +} +thread_count = migrate_compress_threads(); +for (idx = 0; idx < thread_count; idx++) { +if (comp_param[idx].busy) { +qemu_mutex_lock(comp_done_lock); +while (comp_param[idx].busy && !quit_comp_thread) { +qemu_cond_wait(comp_done_cond, comp_done_lock); +} +qemu_mutex_unlock(comp_done_lock); +} +len = qemu_put_qemu_file(f, comp_param[idx].file); +bytes_transferred += len; +} +if ((one_byte_count > 0) && (bytes_transferred > one_byte_count)) { +bytes_transferred -= one_byte_count; +one_byte_count = 0; +} +} + +static inline void set_compress_params(CompressParam *param, RAMBlock *block, + ram_addr_t offset) +{ +param->block = block; +param->offset = offset; +} + +static int compress_page_with_multi_thread(QEMUFile *f, RAMBlock *block, + ram_addr_t offset) +{ +int idx, thread_count, bytes_sent = 0; + +thread_count = migrate_compress_threads(); +qemu_mutex_lock(comp_done_lock); +while (true) { +for (idx = 0; idx < t
Re: [Qemu-devel] [PATCH 6/9] exec: protect mru_block with RCU
On Fri, 02/06 17:55, Paolo Bonzini wrote: > Hence, freeing a RAMBlock has to be switched to call_rcu. > > Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng
[Qemu-devel] [v5 10/12] migration: Add the core code for decompression
Implement the core logic of multiple thread decompression, the decompression can work now. Signed-off-by: Liang Li Signed-off-by: Yang Zhang --- arch_init.c | 50 -- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/arch_init.c b/arch_init.c index 12dfa34..f9e6a95 100644 --- a/arch_init.c +++ b/arch_init.c @@ -833,6 +833,13 @@ static inline void start_compression(CompressParam *param) qemu_mutex_unlock(¶m->mutex); } +static inline void start_decompression(DecompressParam *param) +{ +qemu_mutex_lock(¶m->mutex); +param->busy = true; +qemu_cond_signal(¶m->cond); +qemu_mutex_unlock(¶m->mutex); +} static uint64_t bytes_transferred; @@ -1379,8 +1386,26 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size) static void *do_data_decompress(void *opaque) { +DecompressParam *param = opaque; +size_t pagesize; + while (!quit_decomp_thread) { -/* To be done */ +qemu_mutex_lock(¶m->mutex); +while (!param->busy && !quit_decomp_thread) { +qemu_cond_wait(¶m->cond, ¶m->mutex); +pagesize = TARGET_PAGE_SIZE; +if (!quit_decomp_thread) { +/* uncompress() will return failed in some case, especially + * when the page is dirted when doing the compression, it's + * not a problem because the dirty page will be retransferred + * and uncompress() won't break the data in other pages. + */ +uncompress((Bytef *)param->des, &pagesize, + (const Bytef *)param->compbuf, param->len); +} +param->busy = false; +} +qemu_mutex_unlock(¶m->mutex); } return NULL; @@ -1411,6 +1436,11 @@ void migrate_decompress_threads_join(void) quit_decomp_thread = true; thread_count = migrate_decompress_threads(); for (i = 0; i < thread_count; i++) { +qemu_mutex_lock(&decomp_param[i].mutex); +qemu_cond_signal(&decomp_param[i].cond); +qemu_mutex_unlock(&decomp_param[i].mutex); +} +for (i = 0; i < thread_count; i++) { qemu_thread_join(decompress_threads + i); qemu_mutex_destroy(&decomp_param[i].mutex); qemu_cond_destroy(&decomp_param[i].cond); @@ -1427,7 +1457,23 @@ void migrate_decompress_threads_join(void) static void decompress_data_with_multi_threads(uint8_t *compbuf, void *host, int len) { -/* To be done */ +int idx, thread_count; + +thread_count = migrate_decompress_threads(); +while (true) { +for (idx = 0; idx < thread_count; idx++) { +if (!decomp_param[idx].busy) { +memcpy(decomp_param[idx].compbuf, compbuf, len); +decomp_param[idx].des = host; +decomp_param[idx].len = len; +start_decompression(&decomp_param[idx]); +break; +} +} +if (idx < thread_count) { +break; +} +} } static int ram_load(QEMUFile *f, void *opaque, int version_id) -- 1.9.1
[Qemu-devel] [v5 12/12] migration: Add commands to set and query parameter
Add the qmp and hmp commands to tune and query the parameters used in live migration. Signed-off-by: Liang Li Signed-off-by: Yang Zhang --- hmp-commands.hx | 17 hmp.c | 56 + hmp.h | 4 ++ include/migration/migration.h | 4 +- migration/migration.c | 96 +-- monitor.c | 25 +++ qapi-schema.json | 86 ++ qmp-commands.hx | 66 + 8 files changed, 339 insertions(+), 15 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index e37bc8b..ed0c06a 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -985,6 +985,21 @@ Enable/Disable the usage of a capability @var{capability} for migration. ETEXI { +.name = "migrate_set_parameter", +.args_type = "parameter:s,value:i", +.params = "parameter value", +.help = "Set the parameter for migration", +.mhandler.cmd = hmp_migrate_set_parameter, +.command_completion = migrate_set_parameter_completion, +}, + +STEXI +@item migrate_set_parameter @var{parameter} @var{value} +@findex migrate_set_parameter +Set the parameter @var{parameter} for migration. +ETEXI + +{ .name = "client_migrate_info", .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", @@ -1764,6 +1779,8 @@ show user network stack connection states show migration status @item info migrate_capabilities show current migration capabilities +@item info migrate_parameters +show current migration parameters @item info migrate_cache_size show current migration XBZRLE cache size @item info balloon diff --git a/hmp.c b/hmp.c index b47f331..1f67651 100644 --- a/hmp.c +++ b/hmp.c @@ -246,6 +246,27 @@ void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict) qapi_free_MigrationCapabilityStatusList(caps); } +void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) +{ +MigrationParameterStatusList *params, *p; +MigrationParameterInt *data; + +params = qmp_query_migrate_parameters(NULL); + +if (params) { +monitor_printf(mon, "parameters:"); +for (p = params; p; p = p->next) { +data = (MigrationParameterInt *)p->value->data; +monitor_printf(mon, " %s: %" PRId64, + MigrationParameter_lookup[p->value->kind], + data->value); +} +monitor_printf(mon, "\n"); +} + +qapi_free_MigrationParameterStatusList(params); +} + void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict) { monitor_printf(mon, "xbzrel cache size: %" PRId64 " kbytes\n", @@ -1140,6 +1161,41 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict) } } +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) +{ +const char *param = qdict_get_str(qdict, "parameter"); +int value = qdict_get_int(qdict, "value"); +Error *err = NULL; +MigrationParameterStatusList *params = g_malloc0(sizeof(*params)); +MigrationParameterInt *data; +int i; + +for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) { +if (strcmp(param, MigrationParameter_lookup[i]) == 0) { +params->value = g_malloc0(sizeof(*params->value)); +params->value->kind = i; +params->value->data = g_malloc0(sizeof(MigrationParameterInt)); +data = (MigrationParameterInt *)params->value->data; +data->value = value; +params->next = NULL; +qmp_migrate_set_parameters(params, &err); +break; +} +} + +if (i == MIGRATION_PARAMETER_MAX) { +error_set(&err, QERR_INVALID_PARAMETER, param); +} + +qapi_free_MigrationParameterStatusList(params); + +if (err) { +monitor_printf(mon, "migrate_set_parameter: %s\n", + error_get_pretty(err)); +error_free(err); +} +} + void hmp_set_password(Monitor *mon, const QDict *qdict) { const char *protocol = qdict_get_str(qdict, "protocol"); diff --git a/hmp.h b/hmp.h index 4bb5dca..b2b2d2c 100644 --- a/hmp.h +++ b/hmp.h @@ -28,6 +28,7 @@ void hmp_info_chardev(Monitor *mon, const QDict *qdict); void hmp_info_mice(Monitor *mon, const QDict *qdict); void hmp_info_migrate(Monitor *mon, const QDict *qdict); void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict); +void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict); void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict); void hmp_info_cpus(Monitor *mon, const QDict *qdict); void hmp_info_block(Monitor *mon, const QDict *qdict); @@ -63,6 +64,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict); void hmp_migrate_set_downtime(Mon
Re: [Qemu-devel] [PATCH 8/9] exec: convert ram_list to QLIST
On Fri, 02/06 17:55, Paolo Bonzini wrote: > From: Mike Day > > QLIST has RCU-friendly primitives, so switch to it. > > Signed-off-by: Mike Day > Signed-off-by: Paolo Bonzini > --- > arch_init.c | 19 > exec.c | 52 > +--- > include/exec/cpu-all.h | 4 ++-- > scripts/dump-guest-memory.py | 8 +++ > 4 files changed, 46 insertions(+), 37 deletions(-) > Reviewed-by: Fam Zheng
Re: [Qemu-devel] CPU TLB flush with multithread TCG.
Frederic Konrad writes: > Hi everybody, > > In multithread tlb_flush is broken as CPUA can flush an other CPUB and > CPUB can be > executing code, and fixing this can be quite hard: >* We need to exit the CPU which is flushed. >* Makes sure the CPU is stopped. >* Then we can flush tlb. > The big issues are: >* Two threads can be doing a flush at the same time. >* Something can restart the CPU during the flush. > > A better idea I think is that instead of flushing tlb we can put a flag > in CPUState such > as flush_request and ask the cpu to exit. > Then later once the CPU is exited we can flush tlbs if flush_request is set. > It will ensure that the CPU won't execute code as it's associated thread > will be > flushing. > > Can this work? Does this imply deferring the work? Surely if we don't flush when instructed things could break down very quickly? > > Thanks, > Fred -- Alex Bennée
Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs
On 11 February 2015 at 02:50, Chen, Tiejun wrote: > On 2015/2/11 10:03, Peter Maydell wrote: >> The linux-headers/ directory contains header files which can only >> validly be included if the host we're compiling on is Linux. Some >> of them will cause compile failures on OSX or Windows if they >> are in the include path. The idea of this patch is that the >> standard-headers/ directory has "sanitized" header files which >> have had the linux-specific types and includes stripped out. >> So if we take the route this patch proposes we do need two >> directories. >> > > This confounds me since for instance, one of goals based on this patch is, > it exposes those Virtio devices ID definition to hw/virtio, instead of my > original patch, right? So without this sort of standard-hearders, how can we > compile virtio? Or you mean we still keep those original stuff in > include/hw/virtio*, but somehow update them once we execute that script > manually. I'm confused about why you're confused. We have two basic approaches we can take: (1) What we do at the moment. There are headers defining the virtio interface in include/hw/virtio, and these are basically manually created and updated as necessary. (2) What this patch is proposing. The headers defining virtio are automatically copied into standard-headers/ and fixed up to make them work with QEMU on all the hosts we support. This happens when this script is run by a developer to update QEMU's headers based on some new upstream kernel. Personally I think that option 1 is more reliable and overall less effort, since automatiing the fixups is hard and virtio doesn't change very much. -- PMM
[Qemu-devel] [Xen-devel] [Block dev] : Qemu block ide_dma_read call routine
Hi, I am implementing read equivalent routine in qemu. Can some one help me understand control flow of the qemu read/write implementation. I am using xen-4.2.0 and qemu-1.6.1 My requirement is simple: I have a 1024*1024 buffer already filled with some useful data. Now when windows (my guest OS) does IDE_DMA_READ command to the disk, I want to intercept it and fill data from my private buffer. my intention is to leverage existing dma_read infrastructure and overwrite the read buffer-data at the lowest level of qemu . That way the buffers /vectors "qiov" which are prepared due to cmd IDE_DMA_READ will copy and return data from my data-buffer to guest-OS. I could trace the control from. ide_sector_start_dma -> s->bus->dma->ops->start_dma -> ide_dma_cb ->dma_bdrv_read -> bdrv_aio_readv . ->bdrv_co_aio_rw_vector -> bdrv_co_do_rw "coroutine" -> bdrv_co_do_readv -> drv->bdrv_co_readv (( in my case it is from raw.c raw_co_readv )) -> bdrv_co_readv -> bdrv_co_do_readv ->in bdrv_co_do_rw the bottom half is scheduled bdrv_co_em_bh -->> this will invoke -> ide_dma_cb () which is again the starting point. Looks like there a double-linked list maintained for the coroutine entries and are off loaded to qemu-wait queue during this process. Now I need help to understand where to look for to find the last read/write system call which will get the data out from the disk for guest-OS (windows) . I am seeking suggestions and help for the same. thanks S. Kumar
Re: [Qemu-devel] [PATCH v1] add intel restricted transactional memory test case.
Please someone review this patch. Thanks, Xin On Thu, Feb 5, 2015 at 2:04 PM, Xin Tong wrote: > I am planning to implement support for Intel RTM. similar to what is > done for PowerPC. we can default to fault (transaction abort) to the > fallback code path. Would like to check in this test case first. > > > On Thu, Feb 5, 2015 at 1:56 PM, wrote: >> Add test case for intel restrict transactional memory. compiled with >> Intel ICC 15.0 as well as GCC 4.8. This test case can be used to test >> intel RTM support in the target-i386 frontend. >> >> Signed-off-by: Xin Tong >> >> diff --git a/tests/tcg/test-intelrtm.c b/tests/tcg/test-intelrtm.c >> new file mode 100644 >> index 000..ca1ce16 >> --- /dev/null >> +++ b/tests/tcg/test-intelrtm.c >> @@ -0,0 +1,86 @@ >> +/ >> +This file contains an example of Intel Restricted Transactional Memory >> (RTM). >> +In the example multiple threads are created to simulatenously increment >> +elements in a shared array. >> + >> +This test case should be compiled with Intel ICC compiler as it uses ICC RTM >> +intrnsics. >> +/ >> +#include >> +#include >> +#include /* _XBEGIN_START */ >> + >> +#define ARRAY_SIZE 1024 >> +#define TESTTHREAD_COUNT 32 >> +/* lock for atomic access to the testarray */ >> +static pthread_mutex_t arraylock; >> +static pthread_t *threadarray; >> +static int testarray[ARRAY_SIZE]; >> + >> +static void increment_func(void *arg) >> +{ >> + int i; >> + long int threadnum = (long int)arg; >> + unsigned int status = _xbegin(); >> + if (status == _XBEGIN_STARTED) { >> +/* test _xtest() and _xabort(). fallback path for odd-num thread */ >> +if (_xtest() && (threadnum % 2)) { >> +_xabort(_XABORT_EXPLICIT); >> +} >> +for (i = 0; i < ARRAY_SIZE; ++i) { >> +testarray[i]++; >> +} >> +_xend(); >> + } else { >> + /* fallback path. hold array lock */ >> + pthread_mutex_lock(&arraylock); >> + for (i = 0; i < ARRAY_SIZE; ++i) { >> +testarray[i]++; >> + } >> + /* release array lock */ >> + pthread_mutex_unlock(&arraylock); >> +} >> +} >> + >> +static void run_test(int threadcount) >> +{ >> +int i; >> +/* create the test threads */ >> +for (i = 0; i < threadcount; ++i) { >> +pthread_create(&threadarray[i], NULL, >> + (void *(*)(void *))increment_func, (void*)i); >> +} >> +/* wait for the test threads to finish */ >> +for (i = 0; i < threadcount; ++i) { >> +pthread_join(threadarray[i], NULL); >> +} >> +/* print the end results of the array */ >> +for (i = 0; i < ARRAY_SIZE; ++i) { >> +printf("testarray[%d] is %d\n", i, testarray[i]); >> +} >> +} >> + >> +static void run_setup(int threadcount) >> +{ >> +/* allocate the pthread_t structures */ >> +threadarray = (pthread_t *) malloc(sizeof(pthread_t)*threadcount); >> +/* initialize the arraylock mutex */ >> +if (pthread_mutex_init(&arraylock, NULL) != 0) { >> +printf("arraylock mutex init failed. exiting ...\n"); >> +exit(-1); >> +} >> +} >> + >> +static void run_cleanup(void) >> +{ >> +free(threadarray); >> +pthread_mutex_destroy(&arraylock); >> +} >> + >> +int main(void) >> +{ >> +run_setup(TESTTHREAD_COUNT); >> +run_test(TESTTHREAD_COUNT); >> +run_cleanup(); >> +return 0; >> +}
Re: [Qemu-devel] [PATCH v4 1/4] target-arm: Add CPU property to disable AArch64
On 10 February 2015 at 10:50, Greg Bellows wrote: > Adds registration and get/set functions for enabling/disabling the AArch64 > execution state on AArch64 CPUs. By default AArch64 execution state is > enabled > on AArch64 CPUs, setting the property to off, will disable the execution > state. > The below QEMU invocation would have AArch64 execution state disabled. > > $ ./qemu-system-aarch64 -machine virt -cpu cortex-a57,aarch64=off > > Also adds stripping of features from CPU model string in acquiring the ARM CPU > by name. > > Signed-off-by: Greg Bellows > > --- > > v3 -> v4 > - Switch from using strtok to g_strsplit > - Add disablement of aarch64 option if KVM is not enabled. > > v1 -> v2 > - Scrap the custom CPU feature parsing in favor of using the default CPU > parsing. > - Add registration of CPU AArch64 property to disable/enable the AArch64 > feature. > --- > target-arm/cpu.c | 5 - > target-arm/cpu64.c | 39 +++ > 2 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/target-arm/cpu.c b/target-arm/cpu.c > index d38af74..986f04c 100644 > --- a/target-arm/cpu.c > +++ b/target-arm/cpu.c > @@ -544,13 +544,16 @@ static ObjectClass *arm_cpu_class_by_name(const char > *cpu_model) > { > ObjectClass *oc; > char *typename; > +char **cpuname; > > if (!cpu_model) { > return NULL; > } > > -typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpu_model); > +cpuname = g_strsplit(cpu_model, ",", 1); > +typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpuname[0]); > oc = object_class_by_name(typename); > +g_strfreev(cpuname); > g_free(typename); > if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU) || > object_class_is_abstract(oc)) { > diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c > index bb778b3..d03f203 100644 > --- a/target-arm/cpu64.c > +++ b/target-arm/cpu64.c > @@ -32,6 +32,11 @@ static inline void set_feature(CPUARMState *env, int > feature) > env->features |= 1ULL << feature; > } > > +static inline void unset_feature(CPUARMState *env, int feature) > +{ > +env->features &= ~(1ULL << feature); > +} > + > #ifndef CONFIG_USER_ONLY > static uint64_t a57_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri) > { > @@ -170,8 +175,42 @@ static const ARMCPUInfo aarch64_cpus[] = { > { .name = NULL } > }; > > +static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp) > +{ > +ARMCPU *cpu = ARM_CPU(obj); > + > +return arm_feature(&cpu->env, ARM_FEATURE_AARCH64); > +} > + > +static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) > +{ > +ARMCPU *cpu = ARM_CPU(obj); > + > +/* At this time, this property is only allowed if KVM is enabled. This > + * restriction allows us to avoid fixing up functionality that assumes a > + * uniform execution state like do_interrupt. > + */ > +if (!kvm_enabled()) { > +error_setg(errp, > + "'aarch64' option only supported with '-enable-kvm'"); > +return; This check needs to go in the "we're unsetting the feature bit" code path, and we could make the error message a little clearer: "'aarch64' feature cannot be disabled unless KVM is enabled" (setting the feature to on is harmless and will work with TCG :-)) > +} > + > +if (value == false) { > +unset_feature(&cpu->env, ARM_FEATURE_AARCH64); > +} else { > +set_feature(&cpu->env, ARM_FEATURE_AARCH64); > +} > +} > + > static void aarch64_cpu_initfn(Object *obj) > { > +object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64, > + aarch64_cpu_set_aarch64, NULL); > +object_property_set_description(obj, "aarch64", > +"Set on/off to enable/disable aarch64 " > +"execution state ", > +NULL); "Set on/off to enable/disable support for AArch64 execution state. Disable this to boot 32-bit guests in AArch32 state." (Is that space at the end of your description deliberate or accidental?) Otherwise, Reviewed-by: Peter Maydell -- PMM
[Qemu-devel] [PATCH 05/11] Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- exec.c | 2 +- hw/ide/pci.c | 2 +- hw/microblaze/boot.c | 2 +- migration/rdma.c | 2 +- qemu-img.c | 2 +- target-s390x/kvm.c | 2 +- trace/control.c | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/exec.c b/exec.c index 6b79ad1..86653e5 100644 --- a/exec.c +++ b/exec.c @@ -1155,7 +1155,7 @@ static void *file_ram_alloc(RAMBlock *block, error: if (mem_prealloc) { -error_report("%s\n", error_get_pretty(*errp)); +error_report("%s", error_get_pretty(*errp)); exit(1); } return NULL; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index e3f2054..913a976 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -99,7 +99,7 @@ static int32_t bmdma_prepare_buf(IDEDMA *dma, int is_write) * This should accommodate the largest ATA transaction * for LBA48 (65,536 sectors) and 32K sector sizes. */ if (s->sg.size > INT32_MAX) { -error_report("IDE: sglist describes more than 2GiB.\n"); +error_report("IDE: sglist describes more than 2GiB."); break; } bm->cur_prd_addr += l; diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c index a2843cd..38c59db 100644 --- a/hw/microblaze/boot.c +++ b/hw/microblaze/boot.c @@ -185,7 +185,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base, ram_size - initrd_offset); } if (initrd_size < 0) { -error_report("qemu: could not load initrd '%s'\n", +error_report("qemu: could not load initrd '%s'", initrd_filename); exit(EXIT_FAILURE); } diff --git a/migration/rdma.c b/migration/rdma.c index 6bee30c..1989f61 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -1627,7 +1627,7 @@ static int qemu_rdma_exchange_get_response(RDMAContext *rdma, return -EIO; } if (head->len > RDMA_CONTROL_MAX_BUFFER - sizeof(*head)) { -error_report("too long length: %d\n", head->len); +error_report("too long length: %d", head->len); return -EINVAL; } if (sizeof(*head) + head->len != byte_len) { diff --git a/qemu-img.c b/qemu-img.c index e148af8..ec05ccf 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1638,7 +1638,7 @@ static int img_convert(int argc, char **argv) if (skip_create) { int64_t output_sectors = bdrv_nb_sectors(out_bs); if (output_sectors < 0) { -error_report("unable to get output image length: %s\n", +error_report("unable to get output image length: %s", strerror(-output_sectors)); ret = -1; goto out; diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 6f2d5b4..83c03d1 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -210,7 +210,7 @@ void kvm_s390_reset_vcpu(S390CPU *cpu) * Before this ioctl cpu_synchronize_state() is called in common kvm * code (kvm-all) */ if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL)) { -error_report("Initial CPU reset failed on CPU %i\n", cs->cpu_index); +error_report("Initial CPU reset failed on CPU %i", cs->cpu_index); } } diff --git a/trace/control.c b/trace/control.c index 0d30801..995beb3 100644 --- a/trace/control.c +++ b/trace/control.c @@ -126,7 +126,7 @@ static void trace_init_events(const char *fname) error_report("WARNING: trace event '%s' does not exist", line_ptr); } else if (!trace_event_get_state_static(ev)) { -error_report("WARNING: trace event '%s' is not traceable\n", +error_report("WARNING: trace event '%s' is not traceable", line_ptr); } else { trace_event_set_state_dynamic(ev, enable); -- 1.7.12.4
[Qemu-devel] [PATCH 10/11] arm/digic_boards: Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- hw/arm/digic_boards.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c index 2a4b872..7114c36 100644 --- a/hw/arm/digic_boards.c +++ b/hw/arm/digic_boards.c @@ -65,7 +65,7 @@ static void digic4_board_init(DigicBoard *board) s->digic = DIGIC(object_new(TYPE_DIGIC)); object_property_set_bool(OBJECT(s->digic), true, "realized", &err); if (err != NULL) { -error_report("Couldn't realize DIGIC SoC: %s\n", +error_report("Couldn't realize DIGIC SoC: %s", error_get_pretty(err)); exit(1); } @@ -104,13 +104,13 @@ static void digic_load_rom(DigicBoardState *s, hwaddr addr, char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, filename); if (!fn) { -error_report("Couldn't find rom image '%s'.\n", filename); +error_report("Couldn't find rom image '%s'.", filename); exit(1); } rom_size = load_image_targphys(fn, addr, max_size); if (rom_size < 0 || rom_size > max_size) { -error_report("Couldn't load rom image '%s'.\n", filename); +error_report("Couldn't load rom image '%s'.", filename); exit(1); } } -- 1.7.12.4
[Qemu-devel] [PATCH 09/11] tpm: Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- hw/tpm/tpm_passthrough.c | 12 ++-- tpm.c| 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 2bf3c6f..13ac1d2 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -126,7 +126,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, if (!tpm_pt->tpm_op_canceled || (tpm_pt->tpm_op_canceled && errno != ECANCELED)) { error_report("tpm_passthrough: error while transmitting data " - "to TPM: %s (%i)\n", + "to TPM: %s (%i)", strerror(errno), errno); } goto err_exit; @@ -139,14 +139,14 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, if (!tpm_pt->tpm_op_canceled || (tpm_pt->tpm_op_canceled && errno != ECANCELED)) { error_report("tpm_passthrough: error while reading data from " - "TPM: %s (%i)\n", + "TPM: %s (%i)", strerror(errno), errno); } } else if (ret < sizeof(struct tpm_resp_hdr) || tpm_passthrough_get_size_from_buffer(out) != ret) { ret = -1; error_report("tpm_passthrough: received invalid response " - "packet from TPM\n"); + "packet from TPM"); } err_exit: @@ -282,7 +282,7 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb) if (tpm_pt->cancel_fd >= 0) { n = write(tpm_pt->cancel_fd, "-", 1); if (n != 1) { -error_report("Canceling TPM command failed: %s\n", +error_report("Canceling TPM command failed: %s", strerror(errno)); } else { tpm_pt->tpm_op_canceled = true; @@ -413,13 +413,13 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb) tpm_pt->tpm_fd = qemu_open(tpm_pt->tpm_dev, O_RDWR); if (tpm_pt->tpm_fd < 0) { -error_report("Cannot access TPM device using '%s': %s\n", +error_report("Cannot access TPM device using '%s': %s", tpm_pt->tpm_dev, strerror(errno)); goto err_free_parameters; } if (tpm_passthrough_test_tpmdev(tpm_pt->tpm_fd)) { -error_report("'%s' is not a TPM device.\n", +error_report("'%s' is not a TPM device.", tpm_pt->tpm_dev); goto err_close_tpmdev; } diff --git a/tpm.c b/tpm.c index c371023..0541535 100644 --- a/tpm.c +++ b/tpm.c @@ -134,7 +134,7 @@ static int configure_tpm(QemuOpts *opts) Error *local_err = NULL; if (!QLIST_EMPTY(&tpm_backends)) { -error_report("Only one TPM is allowed.\n"); +error_report("Only one TPM is allowed."); return 1; } -- 1.7.12.4
[Qemu-devel] [PATCH 08/11] xtensa: Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- hw/xtensa/sim.c| 2 +- hw/xtensa/xtfpga.c | 10 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c index 37ea9ae..328d209 100644 --- a/hw/xtensa/sim.c +++ b/hw/xtensa/sim.c @@ -64,7 +64,7 @@ static void xtensa_sim_init(MachineState *machine) for (n = 0; n < smp_cpus; n++) { cpu = cpu_xtensa_init(cpu_model); if (cpu == NULL) { -error_report("unable to find CPU definition '%s'\n", +error_report("unable to find CPU definition '%s'", cpu_model); exit(EXIT_FAILURE); } diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c index e5a6bba..bb90eb2 100644 --- a/hw/xtensa/xtfpga.c +++ b/hw/xtensa/xtfpga.c @@ -190,7 +190,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine) for (n = 0; n < smp_cpus; n++) { cpu = cpu_xtensa_init(cpu_model); if (cpu == NULL) { -error_report("unable to find CPU definition '%s'\n", +error_report("unable to find CPU definition '%s'", cpu_model); exit(EXIT_FAILURE); } @@ -235,7 +235,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine) board->flash_size / board->flash_sector_size, 4, 0x, 0x, 0x, 0x, be); if (flash == NULL) { -error_report("unable to mount pflash\n"); +error_report("unable to mount pflash"); exit(EXIT_FAILURE); } } @@ -287,7 +287,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine) uint32_t dtb_addr = tswap32(cur_lowmem); if (!fdt) { -error_report("could not load DTB '%s'\n", dtb_filename); +error_report("could not load DTB '%s'", dtb_filename); exit(EXIT_FAILURE); } @@ -307,7 +307,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine) lowmem_end - cur_lowmem); } if (initrd_size < 0) { -error_report("could not load initrd '%s'\n", initrd_filename); +error_report("could not load initrd '%s'", initrd_filename); exit(EXIT_FAILURE); } initrd_location.start = tswap32(cur_lowmem); @@ -333,7 +333,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine) if (success > 0 && is_linux) { entry_point = ep; } else { -error_report("could not load kernel '%s'\n", +error_report("could not load kernel '%s'", kernel_filename); exit(EXIT_FAILURE); } -- 1.7.12.4
[Qemu-devel] [PATCH 03/11] pl330.c: remove superfluous '\n' around error_setg
From: Gonglei Signed-off-by: Gonglei --- hw/dma/pl330.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c index 16cf77e..0f72b59 100644 --- a/hw/dma/pl330.c +++ b/hw/dma/pl330.c @@ -1566,7 +1566,7 @@ static void pl330_realize(DeviceState *dev, Error **errp) s->cfg[1] |= 5; break; default: -error_setg(errp, "Bad value for i-cache_len property: %" PRIx8 "\n", +error_setg(errp, "Bad value for i-cache_len property: %" PRIx8 "", s->i_cache_len); return; } @@ -1601,7 +1601,7 @@ static void pl330_realize(DeviceState *dev, Error **errp) s->cfg[CFG_CRD] |= 0x4; break; default: -error_setg(errp, "Bad value for data_width property: %" PRIx8 "\n", +error_setg(errp, "Bad value for data_width property: %" PRIx8 "", s->data_width); return; } -- 1.7.12.4
[Qemu-devel] [PATCH 01/11] block: remove superfluous '\n' around error_report/error_setg
From: Gonglei Signed-off-by: Gonglei --- block/archipelago.c | 6 +++--- hw/block/nand.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/archipelago.c b/block/archipelago.c index a8114b5..855655c 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -291,7 +291,7 @@ static int qemu_archipelago_init(BDRVArchipelagoState *s) ret = qemu_archipelago_xseg_init(s); if (ret < 0) { -error_report("Cannot initialize XSEG. Aborting...\n"); +error_report("Cannot initialize XSEG. Aborting..."); goto err_exit; } @@ -645,7 +645,7 @@ static int qemu_archipelago_create_volume(Error **errp, const char *volname, target = xseg_get_target(xseg, req); if (!target) { -error_setg(errp, "Cannot get XSEG target.\n"); +error_setg(errp, "Cannot get XSEG target."); goto err_exit; } memcpy(target, volname, targetlen); @@ -889,7 +889,7 @@ static BlockAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs, return &aio_cb->common; err_exit: -error_report("qemu_archipelago_aio_rw(): I/O Error\n"); +error_report("qemu_archipelago_aio_rw(): I/O Error"); qemu_aio_unref(aio_cb); return NULL; } diff --git a/hw/block/nand.c b/hw/block/nand.c index 1882a0c..61d2cec 100644 --- a/hw/block/nand.c +++ b/hw/block/nand.c @@ -393,7 +393,7 @@ static void nand_realize(DeviceState *dev, Error **errp) nand_init_2048(s); break; default: -error_setg(errp, "Unsupported NAND block size %#x\n", +error_setg(errp, "Unsupported NAND block size %#x", 1 << s->page_shift); return; } -- 1.7.12.4
[Qemu-devel] [PATCH 04/11] numa: remove superfluous '\n' around error_setg
From: Gonglei Signed-off-by: Gonglei --- numa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/numa.c b/numa.c index afd2866..6decd13 100644 --- a/numa.c +++ b/numa.c @@ -59,7 +59,7 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp) if (nodenr >= MAX_NODES) { error_setg(errp, "Max number of NUMA nodes reached: %" - PRIu16 "\n", nodenr); + PRIu16 "", nodenr); return; } @@ -78,7 +78,7 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp) } if (node->has_mem && node->has_memdev) { -error_setg(errp, "qemu: cannot specify both mem= and memdev=\n"); +error_setg(errp, "qemu: cannot specify both mem= and memdev="); return; } @@ -87,7 +87,7 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp) } if (node->has_memdev != have_memdevs) { error_setg(errp, "qemu: memdev option must be specified for either " - "all or no nodes\n"); + "all or no nodes"); return; } -- 1.7.12.4
[Qemu-devel] [PATCH 07/11] vfio: Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- hw/vfio/common.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index e71385e..6c49cc4 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -201,7 +201,7 @@ static int vfio_dma_unmap(VFIOContainer *container, }; if (ioctl(container->fd, VFIO_IOMMU_UNMAP_DMA, &unmap)) { -error_report("VFIO_UNMAP_DMA: %d\n", -errno); +error_report("VFIO_UNMAP_DMA: %d", -errno); return -errno; } @@ -234,7 +234,7 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr iova, return 0; } -error_report("VFIO_MAP_DMA: %d\n", -errno); +error_report("VFIO_MAP_DMA: %d", -errno); return -errno; } @@ -274,7 +274,7 @@ static void vfio_iommu_map_notify(Notifier *n, void *data) iotlb->translated_addr, &xlat, &len, iotlb->perm & IOMMU_WO); if (!memory_region_is_ram(mr)) { -error_report("iommu map to non memory area %"HWADDR_PRIx"\n", +error_report("iommu map to non memory area %"HWADDR_PRIx"", xlat); return; } @@ -283,7 +283,7 @@ static void vfio_iommu_map_notify(Notifier *n, void *data) * check that it did not truncate too much. */ if (len & iotlb->addr_mask) { -error_report("iommu has granularity incompatible with target AS\n"); +error_report("iommu has granularity incompatible with target AS"); return; } @@ -566,7 +566,7 @@ static void vfio_kvm_device_add_group(VFIOGroup *group) }; if (kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd)) { -error_report("Failed to create KVM VFIO device: %m\n"); +error_report("Failed to create KVM VFIO device: %m"); return; } -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Add 32/64-bit register sync
On 10 February 2015 at 10:50, Greg Bellows wrote: > Add AArch32 to AArch64 register sychronization functions. > Replace manual register synchronization with new functions in > aarch64_cpu_do_interrupt() and HELPER(exception_return)(). > > Signed-off-by: Greg Bellows > > --- > > v3 -> v4 > - Rework sync routines to cover various exception levels > - Move sync routines to helper.c > > v2 -> v3 > - Conditionalize interrupt handler update of aarch64. > --- > target-arm/cpu.h| 2 + > target-arm/helper-a64.c | 5 +- > target-arm/helper.c | 181 > > target-arm/op_helper.c | 6 +- > 4 files changed, 186 insertions(+), 8 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 1830a12..11845a6 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -495,6 +495,8 @@ typedef struct CPUARMState { > ARMCPU *cpu_arm_init(const char *cpu_model); > int cpu_arm_exec(CPUARMState *s); > uint32_t do_arm_semihosting(CPUARMState *env); > +void aarch64_sync_32_to_64(CPUARMState *env); > +void aarch64_sync_64_to_32(CPUARMState *env); > > static inline bool is_a64(CPUARMState *env) > { > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index 8aa40e9..7e0d038 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -466,7 +466,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > unsigned int new_el = arm_excp_target_el(cs, cs->exception_index); > target_ulong addr = env->cp15.vbar_el[new_el]; > unsigned int new_mode = aarch64_pstate_mode(new_el, true); > -int i; > > if (arm_current_el(env) < new_el) { > if (env->aarch64) { > @@ -530,9 +529,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > } > env->elr_el[new_el] = env->regs[15]; > > -for (i = 0; i < 15; i++) { > -env->xregs[i] = env->regs[i]; > -} > +aarch64_sync_32_to_64(env); > > env->condexec_bits = 0; > } > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 1a1a005..c1d6764 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -4096,6 +4096,15 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned > int excp_idx) > return 1; > } > > +void aarch64_sync_64_to_32(CPUARMState *env) > +{ > +int i; > + > +for (i = 0; i < 15; i++) { > +env->regs[i] = env->xregs[i]; > +} > +} This is inside CONFIG_USER_ONLY, right? Is it called at all? If so, when? > + > #else > > /* Map CPU modes onto saved register banks. */ > @@ -4425,6 +4434,178 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) > env->thumb = addr & 1; > } > > +/* Function used to synchronize QEMU's AArch64 register set with AArch32 > + * register set. This is necessary when switching between AArch32 and > AArch64 > + * execution state. This is a bit vague... > + */ > +void aarch64_sync_32_to_64(CPUARMState *env) > +{ > +int i; > +uint32_t mode = env->uncached_cpsr & CPSR_M; > + > +/* We can blanket copy R[0:7] to X[0:7] */ > +for (i = 0; i < 8; i++) { > +env->xregs[i] = env->regs[i]; > +} > + > +/* The latest copy of some registers depend on the current executing > mode. "depends" > + * The general purpose copy is used when the current mode corresponds to > + * the mapping for the given register. Otherwise, the banked copy > + * corresponding to the register mapping is used. > + */ > +if (mode == ARM_CPU_MODE_USR) { > +for (i = 8; i < 15; i++) { > +env->xregs[i] = env->regs[i]; > +} > +} else { > +for (i = 8; i < 13; i++) { > +env->xregs[i] = env->usr_regs[i - 8]; > +} > +env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)]; > +env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)]; > +} > + > +if (mode == ARM_CPU_MODE_HYP) { > +env->xregs[15] = env->regs[13]; > +} else { > +env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)]; > +} > + > +if (mode == ARM_CPU_MODE_IRQ) { > +env->xregs[16] = env->regs[13]; > +env->xregs[17] = env->regs[14]; > +} else { > +env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)]; > +env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)]; > +} > + > +if (mode == ARM_CPU_MODE_SVC) { > +env->xregs[18] = env->regs[13]; > +env->xregs[19] = env->regs[14]; > +} else { > +env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)]; > +env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)]; > +} > + > +if (mode == ARM_CPU_MODE_ABT) { > +env->xregs[20] = env->regs[13]; > +env->xregs[21] = env->regs[14]; > +} else { > +env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)]; > +env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)]; > +} > + > +
[Qemu-devel] [PATCH 02/11] a9gtimer: remove superfluous '\n' around error_setg
From: Gonglei Signed-off-by: Gonglei --- hw/timer/a9gtimer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c index 435142a..b087bbd 100644 --- a/hw/timer/a9gtimer.c +++ b/hw/timer/a9gtimer.c @@ -289,7 +289,7 @@ static void a9_gtimer_realize(DeviceState *dev, Error **errp) int i; if (s->num_cpu < 1 || s->num_cpu > A9_GTIMER_MAX_CPUS) { -error_setg(errp, "%s: num-cpu must be between 1 and %d\n", +error_setg(errp, "%s: num-cpu must be between 1 and %d", __func__, A9_GTIMER_MAX_CPUS); return; } -- 1.7.12.4
[Qemu-devel] [PATCH 11/11] vhost: Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- hw/virtio/vhost-backend.c | 2 +- net/vhost-user.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c index ff4f200..4d68a27 100644 --- a/hw/virtio/vhost-backend.c +++ b/hw/virtio/vhost-backend.c @@ -61,7 +61,7 @@ int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType backend_type) dev->vhost_ops = &user_ops; break; default: -error_report("Unknown vhost backend type\n"); +error_report("Unknown vhost backend type"); r = -1; } diff --git a/net/vhost-user.c b/net/vhost-user.c index 24e050c..23babd6 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -122,12 +122,12 @@ static void net_vhost_user_event(void *opaque, int event) case CHR_EVENT_OPENED: vhost_user_start(s); net_vhost_link_down(s, false); -error_report("chardev \"%s\" went up\n", s->chr->label); +error_report("chardev \"%s\" went up", s->chr->label); break; case CHR_EVENT_CLOSED: net_vhost_link_down(s, true); vhost_user_stop(s); -error_report("chardev \"%s\" went down\n", s->chr->label); +error_report("chardev \"%s\" went down", s->chr->label); break; } } -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4 4/4] target-arm: Add AArch32 guest support to KVM64
On 10 February 2015 at 10:50, Greg Bellows wrote: > Add 32-bit to/from 64-bit register synchronization on register gets and puts. > Set EL1_32BIT feature flag passed to KVM > > Signed-off-by: Greg Bellows > > --- > > v3 -> v4 > - Add check that to make sure KVM64 is only being used on AArch64 family of > machines. > - Relocate register sync to follow register fetches. > - Refresh env->aarch64 prior to use. > > v2 -> v3 > - Conditionalize sync of 32-bit and 64-bit registers > --- > target-arm/kvm64.c | 38 ++ > 1 file changed, 34 insertions(+), 4 deletions(-) > > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c > index 033babf..789933e 100644 > --- a/target-arm/kvm64.c > +++ b/target-arm/kvm64.c > @@ -81,8 +81,8 @@ int kvm_arch_init_vcpu(CPUState *cs) > int ret; > ARMCPU *cpu = ARM_CPU(cs); > > -if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE || > -!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > +if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE && > +object_dynamic_cast(cpu, TYPE_AARCH64_CPU)) { You've changed an OR check (fail if this CPU isn't supported by KVM at all, or if it's not an AArch64-capable CPU) into an AND check... > fprintf(stderr, "KVM is not supported for this guest CPU type\n"); > return -EINVAL; > } > @@ -96,6 +96,9 @@ int kvm_arch_init_vcpu(CPUState *cs) > cpu->psci_version = 2; > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; > } > +if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > +cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT; > +} > > /* Do KVM_ARM_VCPU_INIT ioctl */ > ret = kvm_arm_vcpu_init(cs); > @@ -133,6 +136,13 @@ int kvm_arch_put_registers(CPUState *cs, int level) > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; > > +/* If we are in AArch32 mode then we need to sync the AArch64 regs with > the > + * AArch32 regs before pushing them out 64-bit KVM. "out to". Also, you're not syncing the 64 bit regs with the 32 bit ones, you're copying the data from the 32-bit register state fields into the 64 bit fields. > + */ > +if (!is_a64(env)) { > +aarch64_sync_32_to_64(env); > +} > + > for (i = 0; i < 31; i++) { > reg.id = AARCH64_CORE_REG(regs.regs[i]); > reg.addr = (uintptr_t) &env->xregs[i]; > @@ -162,7 +172,11 @@ int kvm_arch_put_registers(CPUState *cs, int level) > } > > /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */ > -val = pstate_read(env); > +if (is_a64(env)) { > +val = pstate_read(env); > +} else { > +val = cpsr_read(env); > +} > reg.id = AARCH64_CORE_REG(regs.pstate); > reg.addr = (uintptr_t) &val; > ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > @@ -242,7 +256,14 @@ int kvm_arch_get_registers(CPUState *cs) > if (ret) { > return ret; > } > -pstate_write(env, val); > + > +env->aarch64 = ((val & PSTATE_nRW) == 0); > +if (is_a64(env)) { > +pstate_write(env, val); > +} else { > +env->uncached_cpsr = val & CPSR_M; > +cpsr_write(env, val, 0x); > +} > > /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the > * QEMU side we keep the current SP in xregs[31] as well. > @@ -256,6 +277,15 @@ int kvm_arch_get_registers(CPUState *cs) > return ret; > } > > +/* If we are in AArch32 mode then we need to sync the AArch32 regs with > the > + * incoming AArch64 regs received from 64-bit KVM. > + * We must perform this after all of the registers have been acquired > from > + * the kernel. > + */ > +if (!is_a64(env)) { > +aarch64_sync_64_to_32(env); > +} > + > reg.id = AARCH64_CORE_REG(elr_el1); > reg.addr = (uintptr_t) &env->elr_el[1]; > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > -- > 1.8.3.2 > thanks -- PMM
[Qemu-devel] [PATCH 06/11] vhost-scsi: Remove superfluous '\n' around error_report()
From: Gonglei Signed-off-by: Gonglei --- hw/scsi/vhost-scsi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index dcb2bc5..54f916e 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -82,7 +82,7 @@ static int vhost_scsi_start(VHostSCSI *s) if (abi_version > VHOST_SCSI_ABI_VERSION) { error_report("vhost-scsi: The running tcm_vhost kernel abi_version:" " %d is greater than vhost_scsi userspace supports: %d, please" - " upgrade your version of QEMU\n", abi_version, + " upgrade your version of QEMU", abi_version, VHOST_SCSI_ABI_VERSION); return -ENOSYS; } @@ -140,7 +140,7 @@ static void vhost_scsi_stop(VHostSCSI *s) if (k->set_guest_notifiers) { ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false); if (ret < 0) { -error_report("vhost guest notifier cleanup failed: %d\n", ret); +error_report("vhost guest notifier cleanup failed: %d", ret); } } assert(ret >= 0); @@ -185,7 +185,7 @@ static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) ret = vhost_scsi_start(s); if (ret < 0) { -error_report("virtio-scsi: unable to start vhost: %s\n", +error_report("virtio-scsi: unable to start vhost: %s", strerror(-ret)); /* There is no userspace virtio-scsi fallback so exit */ -- 1.7.12.4
[Qemu-devel] [PATCH 00/11] trivial: Remove superfluous '\n' around error_report/error_setg
From: Gonglei Yestoday, I found that some files have superflous '\n' charactor around error_report/error_setg when reviewed patches. By a simply script, the below files were listed. Classify and fix them. It's suitable for appling via qemu-trivial IMHO. Gonglei (11): block: remove superfluous '\n' around error_report/error_setg a9gtimer: remove superfluous '\n' around error_setg pl330.c: remove superfluous '\n' around error_setg numa: remove superfluous '\n' around error_setg Remove superfluous '\n' around error_report() vhost-scsi: Remove superfluous '\n' around error_report() vfio: Remove superfluous '\n' around error_report() xtensa: Remove superfluous '\n' around error_report() tpm: Remove superfluous '\n' around error_report() arm/digic_boards: Remove superfluous '\n' around error_report() vhost: Remove superfluous '\n' around error_report() block/archipelago.c | 6 +++--- exec.c| 2 +- hw/arm/digic_boards.c | 6 +++--- hw/block/nand.c | 2 +- hw/dma/pl330.c| 4 ++-- hw/ide/pci.c | 2 +- hw/microblaze/boot.c | 2 +- hw/scsi/vhost-scsi.c | 6 +++--- hw/timer/a9gtimer.c | 2 +- hw/tpm/tpm_passthrough.c | 12 ++-- hw/vfio/common.c | 10 +- hw/virtio/vhost-backend.c | 2 +- hw/xtensa/sim.c | 2 +- hw/xtensa/xtfpga.c| 10 +- migration/rdma.c | 2 +- net/vhost-user.c | 4 ++-- numa.c| 6 +++--- qemu-img.c| 2 +- target-s390x/kvm.c| 2 +- tpm.c | 2 +- trace/control.c | 2 +- 21 files changed, 44 insertions(+), 44 deletions(-) -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4 1/4] target-arm: Add CPU property to disable AArch64
On Tue, Feb 10, 2015 at 10:03 PM, Peter Maydell wrote: > On 10 February 2015 at 10:50, Greg Bellows > wrote: > > Adds registration and get/set functions for enabling/disabling the > AArch64 > > execution state on AArch64 CPUs. By default AArch64 execution state is > enabled > > on AArch64 CPUs, setting the property to off, will disable the execution > state. > > The below QEMU invocation would have AArch64 execution state disabled. > > > > $ ./qemu-system-aarch64 -machine virt -cpu cortex-a57,aarch64=off > > > > Also adds stripping of features from CPU model string in acquiring the > ARM CPU > > by name. > > > > Signed-off-by: Greg Bellows > > > > --- > > > > v3 -> v4 > > - Switch from using strtok to g_strsplit > > - Add disablement of aarch64 option if KVM is not enabled. > > > > v1 -> v2 > > - Scrap the custom CPU feature parsing in favor of using the default CPU > > parsing. > > - Add registration of CPU AArch64 property to disable/enable the AArch64 > > feature. > > --- > > target-arm/cpu.c | 5 - > > target-arm/cpu64.c | 39 +++ > > 2 files changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/target-arm/cpu.c b/target-arm/cpu.c > > index d38af74..986f04c 100644 > > --- a/target-arm/cpu.c > > +++ b/target-arm/cpu.c > > @@ -544,13 +544,16 @@ static ObjectClass *arm_cpu_class_by_name(const > char *cpu_model) > > { > > ObjectClass *oc; > > char *typename; > > +char **cpuname; > > > > if (!cpu_model) { > > return NULL; > > } > > > > -typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpu_model); > > +cpuname = g_strsplit(cpu_model, ",", 1); > > +typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpuname[0]); > > oc = object_class_by_name(typename); > > +g_strfreev(cpuname); > > g_free(typename); > > if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU) || > > object_class_is_abstract(oc)) { > > diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c > > index bb778b3..d03f203 100644 > > --- a/target-arm/cpu64.c > > +++ b/target-arm/cpu64.c > > @@ -32,6 +32,11 @@ static inline void set_feature(CPUARMState *env, int > feature) > > env->features |= 1ULL << feature; > > } > > > > +static inline void unset_feature(CPUARMState *env, int feature) > > +{ > > +env->features &= ~(1ULL << feature); > > +} > > + > > #ifndef CONFIG_USER_ONLY > > static uint64_t a57_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo > *ri) > > { > > @@ -170,8 +175,42 @@ static const ARMCPUInfo aarch64_cpus[] = { > > { .name = NULL } > > }; > > > > +static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp) > > +{ > > +ARMCPU *cpu = ARM_CPU(obj); > > + > > +return arm_feature(&cpu->env, ARM_FEATURE_AARCH64); > > +} > > + > > +static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error > **errp) > > +{ > > +ARMCPU *cpu = ARM_CPU(obj); > > + > > +/* At this time, this property is only allowed if KVM is enabled. > This > > + * restriction allows us to avoid fixing up functionality that > assumes a > > + * uniform execution state like do_interrupt. > > + */ > > +if (!kvm_enabled()) { > > +error_setg(errp, > > + "'aarch64' option only supported with > '-enable-kvm'"); > > +return; > > This check needs to go in the "we're unsetting the feature bit" > code path, and we could make the error message a little clearer: > "'aarch64' feature cannot be disabled unless KVM is enabled" > (setting the feature to on is harmless and will work with TCG :-)) > > Although it may be benign, the user may be doing something they think may have an effect which is why I catch any case (not just off). I see this as being cleaner. As far as the message, the user does not really know about an "aarch64" feature, that is internal to QEMU. Given this is a command line option error, I believe it makes more sense to keep it in that domain. The message as is describes the error in terms the command line options. Maybe a compromise such as below would work: "aarch64 execution state can only be disabled when enabling KVM using the '-enable-kvm' option > > +} > > + > > +if (value == false) { > > +unset_feature(&cpu->env, ARM_FEATURE_AARCH64); > > +} else { > > +set_feature(&cpu->env, ARM_FEATURE_AARCH64); > > +} > > +} > > + > > static void aarch64_cpu_initfn(Object *obj) > > { > > +object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64, > > + aarch64_cpu_set_aarch64, NULL); > > +object_property_set_description(obj, "aarch64", > > +"Set on/off to enable/disable > aarch64 " > > +"execution state ", > > +NULL); > > "Set on/off to enable/disable support for AArch64 execution > state. Disable this to boot 32-bit guests in AArch32 state." > I'll add suggested wor
Re: [Qemu-devel] [PATCH v4 4/4] target-arm: Add AArch32 guest support to KVM64
On Tue, Feb 10, 2015 at 10:16 PM, Peter Maydell wrote: > On 10 February 2015 at 10:50, Greg Bellows > wrote: > > Add 32-bit to/from 64-bit register synchronization on register gets and > puts. > > Set EL1_32BIT feature flag passed to KVM > > > > Signed-off-by: Greg Bellows > > > > --- > > > > v3 -> v4 > > - Add check that to make sure KVM64 is only being used on AArch64 family > of > > machines. > > - Relocate register sync to follow register fetches. > > - Refresh env->aarch64 prior to use. > > > > v2 -> v3 > > - Conditionalize sync of 32-bit and 64-bit registers > > --- > > target-arm/kvm64.c | 38 ++ > > 1 file changed, 34 insertions(+), 4 deletions(-) > > > > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c > > index 033babf..789933e 100644 > > --- a/target-arm/kvm64.c > > +++ b/target-arm/kvm64.c > > @@ -81,8 +81,8 @@ int kvm_arch_init_vcpu(CPUState *cs) > > int ret; > > ARMCPU *cpu = ARM_CPU(cs); > > > > -if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE || > > -!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > > +if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE && > > +object_dynamic_cast(cpu, TYPE_AARCH64_CPU)) { > > You've changed an OR check (fail if this CPU isn't supported > by KVM at all, or if it's not an AArch64-capable CPU) into > an AND check... > Bah... that was inadvertent, will fix. > > > fprintf(stderr, "KVM is not supported for this guest CPU > type\n"); > > return -EINVAL; > > } > > @@ -96,6 +96,9 @@ int kvm_arch_init_vcpu(CPUState *cs) > > cpu->psci_version = 2; > > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; > > } > > +if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > > +cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT; > > +} > > > > /* Do KVM_ARM_VCPU_INIT ioctl */ > > ret = kvm_arm_vcpu_init(cs); > > @@ -133,6 +136,13 @@ int kvm_arch_put_registers(CPUState *cs, int level) > > ARMCPU *cpu = ARM_CPU(cs); > > CPUARMState *env = &cpu->env; > > > > +/* If we are in AArch32 mode then we need to sync the AArch64 regs > with the > > + * AArch32 regs before pushing them out 64-bit KVM. > > "out to". Also, you're not syncing the 64 bit regs with the 32 bit ones, > you're copying the data from the 32-bit register state fields into > the 64 bit fields. > Fixed in next version. > > > + */ > > +if (!is_a64(env)) { > > +aarch64_sync_32_to_64(env); > > +} > > + > > for (i = 0; i < 31; i++) { > > reg.id = AARCH64_CORE_REG(regs.regs[i]); > > reg.addr = (uintptr_t) &env->xregs[i]; > > @@ -162,7 +172,11 @@ int kvm_arch_put_registers(CPUState *cs, int level) > > } > > > > /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */ > > -val = pstate_read(env); > > +if (is_a64(env)) { > > +val = pstate_read(env); > > +} else { > > +val = cpsr_read(env); > > +} > > reg.id = AARCH64_CORE_REG(regs.pstate); > > reg.addr = (uintptr_t) &val; > > ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > > @@ -242,7 +256,14 @@ int kvm_arch_get_registers(CPUState *cs) > > if (ret) { > > return ret; > > } > > -pstate_write(env, val); > > + > > +env->aarch64 = ((val & PSTATE_nRW) == 0); > > +if (is_a64(env)) { > > +pstate_write(env, val); > > +} else { > > +env->uncached_cpsr = val & CPSR_M; > > +cpsr_write(env, val, 0x); > > +} > > > > /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the > > * QEMU side we keep the current SP in xregs[31] as well. > > @@ -256,6 +277,15 @@ int kvm_arch_get_registers(CPUState *cs) > > return ret; > > } > > > > +/* If we are in AArch32 mode then we need to sync the AArch32 regs > with the > > + * incoming AArch64 regs received from 64-bit KVM. > > + * We must perform this after all of the registers have been > acquired from > > + * the kernel. > > + */ > > +if (!is_a64(env)) { > > +aarch64_sync_64_to_32(env); > > +} > > + > > reg.id = AARCH64_CORE_REG(elr_el1); > > reg.addr = (uintptr_t) &env->elr_el[1]; > > ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); > > -- > > 1.8.3.2 > > > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH] memory: unregister AddressSpace MemoryListener within BQL
Quoting Paolo Bonzini (2015-02-10 06:52:49) > address_space_destroy_dispatch is called from an RCU callback and hence > outside the iothread mutex (BQL). However, after address_space_destroy > no new accesses can hit the destroyed AddressSpace so it is not necessary > to observe changes to the memory map. Move the memory_listener_unregister > call earlier, to make it thread-safe again. > > Reported-by: Alex Williamson > Fixes: 374f2981d1f10bc4307f250f24b2a7ddb9b14be0 > Signed-off-by: Paolo Bonzini Prior to this patch I was seeing segfaults in various parts of memory listener register/unregister path running a workload that rapidly hot plugs/unplugs a sizeable number of devices, which seems to be addressed with this patch applied. But now I'm seeing a less frequent segfault in the RCU thread when running the same workload: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x3fffb689ec20 (LWP 26230)] call_rcu_thread (opaque=) at /home/mdroth/w/qemu.git/util/rcu.c:250 250 node->func(node); (gdb) bt #0 call_rcu_thread (opaque=) at /home/mdroth/w/qemu.git/util/rcu.c:250 #1 0x3fffb787c29c in .start_thread () from /lib64/libpthread.so.0 #2 0x3fffb779cd30 in .__clone () from /lib64/libc.so.6 (gdb) ptype node type = struct rcu_head { struct rcu_head *next; RCUCBFunc *func; } * (gdb) print node $1 = (struct rcu_head *) 0x11189a68 (gdb) print node->func $2 = (RCUCBFunc *) 0x0 (gdb) print node->next $3 = (struct rcu_head *) 0x3fff9800d4f0 I've seen it on both x86 and pseries (with spapr hotplug patches applied), and have only seen it occur at this spot. AFAICT node->func is only set via 1 of: call_rcu(old_view, flatview_unref, rcu); call_rcu(as, do_address_space_destroy, rcu); so it shouldn't ever be NULL... and there's a wmb after node->func is set, prior to the node being made available to the RCU thread via enqueue(), so that doesn't seem to be the issue. I think the node in this case is a FlatView*, if that helps narrow it down: (gdb) print ((AddressSpace *)(0x3fff9800d4f0))->name $5 = 0x1 (gdb) print ((FlatView *)(0x3fff9800d4f0))->ref $6 = 1 (gdb) print ((FlatView *)(0x3fff9800d4f0))->nr $7 = 34 (gdb) print ((FlatView *)(0x3fff9800d4f0))->nr_allocated $8 = 40 (gdb) The workload is basically this, run in a tight loop: device_add virtio-net-pci,id=0 sleep .5 ... device_add virtio-net-pci,id=14 sleep .5 sleep 3 device_del 0 ... device_del 14 Let me know if there's anything else I can do to narrow it down further. > --- > exec.c | 6 +- > include/exec/memory-internal.h | 1 + > memory.c | 1 + > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 6b79ad1..6dff7bc 100644 > --- a/exec.c > +++ b/exec.c > @@ -2059,11 +2059,15 @@ void address_space_init_dispatch(AddressSpace *as) > memory_listener_register(&as->dispatch_listener, as); > } > > +void address_space_unregister(AddressSpace *as) > +{ > +memory_listener_unregister(&as->dispatch_listener); > +} > + > void address_space_destroy_dispatch(AddressSpace *as) > { > AddressSpaceDispatch *d = as->dispatch; > > -memory_listener_unregister(&as->dispatch_listener); > g_free(d); > as->dispatch = NULL; > } > diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h > index 25c43c0..fb467ac 100644 > --- a/include/exec/memory-internal.h > +++ b/include/exec/memory-internal.h > @@ -23,6 +23,7 @@ > typedef struct AddressSpaceDispatch AddressSpaceDispatch; > > void address_space_init_dispatch(AddressSpace *as); > +void address_space_unregister(AddressSpace *as); > void address_space_destroy_dispatch(AddressSpace *as); > > extern const MemoryRegionOps unassigned_mem_ops; > diff --git a/memory.c b/memory.c > index 9b91243..130152c 100644 > --- a/memory.c > +++ b/memory.c > @@ -1978,6 +1978,7 @@ void address_space_destroy(AddressSpace *as) > as->root = NULL; > memory_region_transaction_commit(); > QTAILQ_REMOVE(&address_spaces, as, address_spaces_link); > +address_space_unregister(as); > > /* At this point, as->dispatch and as->current_map are dummy > * entries that the guest should never use. Wait for the old > -- > 1.8.3.1
Re: [Qemu-devel] [PATCH v4 1/4] target-arm: Add CPU property to disable AArch64
On 11 February 2015 at 04:27, Greg Bellows wrote: > > > On Tue, Feb 10, 2015 at 10:03 PM, Peter Maydell > wrote: >> >> On 10 February 2015 at 10:50, Greg Bellows >> wrote: >> > +if (!kvm_enabled()) { >> > +error_setg(errp, >> > + "'aarch64' option only supported with >> > '-enable-kvm'"); >> > +return; >> >> This check needs to go in the "we're unsetting the feature bit" >> code path, and we could make the error message a little clearer: >> "'aarch64' feature cannot be disabled unless KVM is enabled" >> (setting the feature to on is harmless and will work with TCG :-)) >> > Although it may be benign, the user may be doing something they think may > have an effect which is why I catch any case (not just off). I see this as > being cleaner. OK; I don't feel strongly about this. > As far as the message, the user does not really know about an "aarch64" > feature, that is internal to QEMU. Given this is a command line option > error, I believe it makes more sense to keep it in that domain. The point is that it's not a command line option. That would be "qemu-system-aarch64 -aarch64", but what we actually have is "-cpu whatever,-aarch64=off", which is a feature enable/disable. You could say "Setting CPU property 'aarch64' to off is not valid unless KVM is enabled" if you prefer that wording. > The message > as is describes the error in terms the command line options. Maybe a > compromise such as below would work: > > "aarch64 execution state can only be disabled when enabling KVM using the > '-enable-kvm' option This seems backwards, because it isn't precise about what the user has done wrong on their command line (asked us to disable the 'aarch64' cpu feature) but is precise about something that's not very important (the option you can use to enable KVM, and in fact you can enable KVM via other command line syntax too). Also, if you're referring to the CPU state, that needs capitals: AArch64. Only use lowercase if you're talking about the user-facing feature-switch name (in which case it should go in quotes). thanks -- PMM
Re: [Qemu-devel] [PULL 0/6] vfio fixes
On 10 February 2015 at 18:37, Alex Williamson wrote: > Sorry for the short interval of vfio pull requests, but this fixes > hotplug of vfio-pci devices after the last RCU merge. Thanks, > > Alex > > The following changes since commit a2f2d288b5a06e6c680c387c9980d91363f59c61: > > softfloat: expand out STATUS macro (2015-02-06 16:11:38 +) > > are available in the git repository at: > > git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20150210.0 > > for you to fetch changes up to bc5baffa3554e4c0d20c1dbe879aec931866bd69: > > vfio: Fix debug message compile error (2015-02-10 10:25:44 -0700) > > > RCU fixes and cleanup (Paolo Bonzini) > Switch to v2 IOMMU interface (Alex Williamson) > DEBUG build fix (Alexey Kardashevskiy) > > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Add 32/64-bit register sync
On Tue, Feb 10, 2015 at 10:13 PM, Peter Maydell wrote: > On 10 February 2015 at 10:50, Greg Bellows > wrote: > > Add AArch32 to AArch64 register sychronization functions. > > Replace manual register synchronization with new functions in > > aarch64_cpu_do_interrupt() and HELPER(exception_return)(). > > > > Signed-off-by: Greg Bellows > > > > --- > > > > v3 -> v4 > > - Rework sync routines to cover various exception levels > > - Move sync routines to helper.c > > > > v2 -> v3 > > - Conditionalize interrupt handler update of aarch64. > > --- > > target-arm/cpu.h| 2 + > > target-arm/helper-a64.c | 5 +- > > target-arm/helper.c | 181 > > > target-arm/op_helper.c | 6 +- > > 4 files changed, 186 insertions(+), 8 deletions(-) > > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > > index 1830a12..11845a6 100644 > > --- a/target-arm/cpu.h > > +++ b/target-arm/cpu.h > > @@ -495,6 +495,8 @@ typedef struct CPUARMState { > > ARMCPU *cpu_arm_init(const char *cpu_model); > > int cpu_arm_exec(CPUARMState *s); > > uint32_t do_arm_semihosting(CPUARMState *env); > > +void aarch64_sync_32_to_64(CPUARMState *env); > > +void aarch64_sync_64_to_32(CPUARMState *env); > > > > static inline bool is_a64(CPUARMState *env) > > { > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > > index 8aa40e9..7e0d038 100644 > > --- a/target-arm/helper-a64.c > > +++ b/target-arm/helper-a64.c > > @@ -466,7 +466,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > unsigned int new_el = arm_excp_target_el(cs, cs->exception_index); > > target_ulong addr = env->cp15.vbar_el[new_el]; > > unsigned int new_mode = aarch64_pstate_mode(new_el, true); > > -int i; > > > > if (arm_current_el(env) < new_el) { > > if (env->aarch64) { > > @@ -530,9 +529,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > } > > env->elr_el[new_el] = env->regs[15]; > > > > -for (i = 0; i < 15; i++) { > > -env->xregs[i] = env->regs[i]; > > -} > > +aarch64_sync_32_to_64(env); > > > > env->condexec_bits = 0; > > } > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 1a1a005..c1d6764 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -4096,6 +4096,15 @@ unsigned int arm_excp_target_el(CPUState *cs, > unsigned int excp_idx) > > return 1; > > } > > > > +void aarch64_sync_64_to_32(CPUARMState *env) > > +{ > > +int i; > > + > > +for (i = 0; i < 15; i++) { > > +env->regs[i] = env->xregs[i]; > > +} > > +} > > This is inside CONFIG_USER_ONLY, right? Is it called at all? > If so, when? > The exception_return helper calls the function so I had to either add a USER_CONFIG version of wrap the call in exception return with CONFIG_USER_ONLY. I chose the former, but either would work. As you would already know, the exception_return helper is likely not getting called in a USER_ONLY build. > > > + > > #else > > > > /* Map CPU modes onto saved register banks. */ > > @@ -4425,6 +4434,178 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) > > env->thumb = addr & 1; > > } > > > > +/* Function used to synchronize QEMU's AArch64 register set with AArch32 > > + * register set. This is necessary when switching between AArch32 and > AArch64 > > + * execution state. > > This is a bit vague... > I'll elaborate. > > > + */ > > +void aarch64_sync_32_to_64(CPUARMState *env) > > +{ > > +int i; > > +uint32_t mode = env->uncached_cpsr & CPSR_M; > > + > > +/* We can blanket copy R[0:7] to X[0:7] */ > > +for (i = 0; i < 8; i++) { > > +env->xregs[i] = env->regs[i]; > > +} > > + > > +/* The latest copy of some registers depend on the current > executing mode. > > "depends" > Fixed in next version. > > > + * The general purpose copy is used when the current mode > corresponds to > > + * the mapping for the given register. Otherwise, the banked copy > > + * corresponding to the register mapping is used. > > + */ > > +if (mode == ARM_CPU_MODE_USR) { > > +for (i = 8; i < 15; i++) { > > +env->xregs[i] = env->regs[i]; > > +} > > +} else { > > +for (i = 8; i < 13; i++) { > > +env->xregs[i] = env->usr_regs[i - 8]; > > +} > > +env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)]; > > +env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)]; > > +} > > + > > +if (mode == ARM_CPU_MODE_HYP) { > > +env->xregs[15] = env->regs[13]; > > +} else { > > +env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)]; > > +} > > + > > +if (mode == ARM_CPU_MODE_IRQ) { > > +env->xregs[16] = env->regs[13]; > > +env->xregs[17] = env->regs[14]; > > +} else { > > +env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)]; > > +env->xregs[17] =
Re: [Qemu-devel] [PATCH v4 3/4] target-arm: Add 32/64-bit register sync
On 11 February 2015 at 06:08, Greg Bellows wrote: > > > On Tue, Feb 10, 2015 at 10:13 PM, Peter Maydell > wrote: >> >> On 10 February 2015 at 10:50, Greg Bellows >> wrote: >> > +void aarch64_sync_64_to_32(CPUARMState *env) >> > +{ >> > +int i; >> > + >> > +for (i = 0; i < 15; i++) { >> > +env->regs[i] = env->xregs[i]; >> > +} >> > +} >> >> This is inside CONFIG_USER_ONLY, right? Is it called at all? >> If so, when? > > > The exception_return helper calls the function so I had to either add a > USER_CONFIG version of wrap the call in exception return with > CONFIG_USER_ONLY. I chose the former, but either would work. As you would > already know, the exception_return helper is likely not getting called in a > USER_ONLY build. Right, so make it just g_assert_not_reached(). >> >> > + * The AArch32 registers 8-12 are only sync'd when we are in USR or >> > FIQ >> > + * mode as they are the only modes where AArch64 registers map to >> > these >> > + * registers. In all other cases, the respective USR and FIQ banks >> > are >> > + * sync'd. >> > + * The AArch32 registers 13 & 14 are sync'd depending on the >> > execution mode >> > + * we are in. If we are not in a given mode, the bank >> > corresponding to the >> > + * AArch64 register is sync'd. >> > + */ >> > +if (mode == ARM_CPU_MODE_USR) { >> > +for (i = 8; i < 15; i++) { >> > +env->regs[i] = env->xregs[i]; >> > +} >> >> Something is wrong here, because we don't seem to be writing >> anything to env->regs[8..15] if mode is neither USR nor FIQ. >> > I wrestled with this myself. As I understand it, nothing maps to > regs[8..15] unless we are in USR or FIQ, which I covered. This based on the > ARM ARM xregs[8:15] are defined to specifically map to USR, Outside of the > these modes, what should be copied to regs[8..15]? There is always *something* that is the architecturally defined state for all the AArch32 registers. Otherwise a 64-bit hypervisor would be unable to interrupt and restart a 32-bit guest. (And all the registers r0..r15 exist in all AArch32 modes; the question is just whether they're banked registers or shared with some other mode). All modes other than FIQ use the USR registers for r0..r12. (See the v8 ARM ARM fig G1-3, and note the footnote about the meaning of empty cells in the table.) r13 is the appropriate sp for the mode (noting that system mode shares with usr), and r14 ditto (but system and hyp share with usr). -- PMM
[Qemu-devel] [PATCH] monitor: Fix Resource leak
From: Gonglei Coverity spot: qemu_using_spice allocates memory that is stored into err, but not freed before return. Cc:Markus Armbruster Signed-off-by: Gonglei --- monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index c3cc060..137d23f 100644 --- a/monitor.c +++ b/monitor.c @@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict, const char *subject = qdict_get_try_str(qdict, "cert-subject"); int port = qdict_get_try_int(qdict, "port", -1); int tls_port = qdict_get_try_int(qdict, "tls-port", -1); -Error *err; +Error *err = NULL; int ret; if (strcmp(protocol, "spice") == 0) { if (!qemu_using_spice(&err)) { qerror_report_err(err); +error_free(err); return -1; } -- 1.7.12.4
Re: [Qemu-devel] [PATCH v1 1/2] vhost-user: support SET_MEM_TABLE waite the result of mmap
On 2015/2/10 20:04, Michael S. Tsirkin wrote: > So that's not good. We need a way to negotiate the capability, > we can't just deadlock with legacy slaves. Or add a new message to query slaves' version if slaves not reply we don't wait otherwise if the version as same as QEMU we wait the reply. Mostly the same as iotcl may be all messages need reply. -- Regards, Haifeng
[Qemu-devel] [PATCH] monitor: Fix Resource leak
From: Gonglei Coverity spot: qemu_using_spice allocates memory that is stored into err, but not freed before return. Cc: Markus Armbruster Signed-off-by: Gonglei --- monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index c3cc060..137d23f 100644 --- a/monitor.c +++ b/monitor.c @@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict, const char *subject = qdict_get_try_str(qdict, "cert-subject"); int port = qdict_get_try_int(qdict, "port", -1); int tls_port = qdict_get_try_int(qdict, "tls-port", -1); -Error *err; +Error *err = NULL; int ret; if (strcmp(protocol, "spice") == 0) { if (!qemu_using_spice(&err)) { qerror_report_err(err); +error_free(err); return -1; } -- 1.7.12.4
Re: [Qemu-devel] Revert commit 5af35d7feccaa7d26b72c6c3d14116421d736b36 - "usb-host-libusb: Fix reset handling"
Hi, On 10-02-15 22:02, Dennis Ostermann wrote: Hello Hans, thanks for taking care. 09-02-15 09:09, Hans de Goede wrote: Hi, On 09-02-15 22:09, Dennis Ostermann wrote: Hi there, please revert commit 5af35d7feccaa7d26b72c6c3d14116421d736b36 - "usb-host-libusb: Fix reset handling" This breaks usb pass through of FTDI based usb devices: On the host: lsusb | grep FT2232 Bus 003 Device 008: ID 0403:6010 Future Technology Devices International, Ltd FT2232C Dual USB-UART/FIFO IC ~/qemu-install/bin$ sudo ./qemu-system-x86_64 -monitor telnet:127.0.0.1:1234,server,nowait -hda /dev/sdd2 -redir tcp:20022::22 --enable-kvm -cpu host -smp 4 -vga vmware --vnc :0 -m 8192 -usb -device usb-ehci,id=ehci -device usb-host,bus=ehci.0,vendorid=0x0403,productid=0x6010 WARNING: Image format was not specified for '/dev/sdd2' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/005: No such file or directory libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/006: No such file or directory libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/007: No such device The device gets reset again and again and is re-enumerated every time and finally not passed through. This looks like the device drops of the bus when it is reset, that is not normal behavior, there seems to be something unique to your setup causing this. Have you tried this on multiple machines / different usb ports on your pc ? This may be something weird with the usb controller in your machine. The machine uses Intel H97 chipset, so not that unique. I tried every port, every USB BIOS, with and without hub, it doesn't make any difference. But you're right, I also tried it on a T61 with Intel 900 series chipset and it worked alright. But this is an 'old' USB 2.0 chipset. I'll try on another box with USB 3.0 chipset tomorrow, if I can get one. Ok, lets wait and see what results another model usb-3 capable pc / laptop gets us, I've the feeling that this is a machine specific issue. Normally a usb-reset should not cause a device disconnect as you're seeing here. Regards, Hans
Re: [Qemu-devel] [PATCH] memory: unregister AddressSpace MemoryListener within BQL
On 11/02/2015 06:13, Michael Roth wrote: > (gdb) print node > $1 = (struct rcu_head *) 0x11189a68 > (gdb) print node->func > $2 = (RCUCBFunc *) 0x0 > (gdb) print node->next > $3 = (struct rcu_head *) 0x3fff9800d4f0 > > I've seen it on both x86 and pseries (with spapr hotplug patches applied), and > have only seen it occur at this spot. > > AFAICT node->func is only set via 1 of: > > call_rcu(old_view, flatview_unref, rcu); > call_rcu(as, do_address_space_destroy, rcu); > > so it shouldn't ever be NULL... and there's a wmb after node->func is set, > prior to the node being made available to the RCU thread via enqueue(), so > that doesn't seem to be the issue. > > I think the node in this case is a FlatView*, if that helps narrow it down: > > (gdb) print ((AddressSpace *)(0x3fff9800d4f0))->name > $5 = 0x1 This is node->next, not node. The weird address looks almost like node == &dummy. I'll try to reproduce. Paolo
[Qemu-devel] [PATCH RFC v5 1/5] qemu-iotests: qemu machine type support
This patch adds qemu machine type support to the io test suite. Based on the qemu default machine type and alias of the default machine type the reference output file can now vary from the default to a machine specific output file if necessary. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com --- tests/qemu-iotests/check | 5 + tests/qemu-iotests/common.config | 9 + tests/qemu-iotests/iotests.py| 1 + 3 files changed, 15 insertions(+) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index baeae80..22b2e63 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -324,6 +324,11 @@ do fi reference="$source_iotests/$seq.out" +reference_machine="$source_iotests/$seq.$QEMU_DEFAULT_MACHINE.out" +if [ -f "$reference_machine" ]; then +reference="$reference_machine" +fi + if [ "$CACHEMODE" = "none" ]; then [ -f "$source_iotests/$seq.out.nocache" ] && reference="$source_iotests/$seq.out.nocache" fi diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index a1973ad..0288cb1 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -107,6 +107,15 @@ export QEMU=$QEMU_PROG export QEMU_IMG=$QEMU_IMG_PROG export QEMU_IO="$QEMU_IO_PROG $QEMU_IO_OPTIONS" export QEMU_NBD=$QEMU_NBD_PROG +default_machine=$($QEMU -machine \? | awk '/(default)/{print $1}') +default_alias_machine=$($QEMU -machine \? |\ +awk -v var_default_machine="$default_machine"\)\ +'{if ($(NF-2)=="(alias"&&$(NF-1)=="of"&&$(NF)==var_default_machine){print $1}}') +if [ ! -z "$default_alias_machine" ]; then +default_machine="$default_alias_machine" +fi + +export QEMU_DEFAULT_MACHINE="$default_machine" [ -f /etc/qemu-iotest.config ] && . /etc/qemu-iotest.config diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 241b5ee..1f520ed 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -39,6 +39,7 @@ imgproto = os.environ.get('IMGPROTO', 'file') test_dir = os.environ.get('TEST_DIR', '/var/tmp') output_dir = os.environ.get('OUTPUT_DIR', '.') cachemode = os.environ.get('CACHEMODE') +qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE') socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') -- 1.9.1
[Qemu-devel] [PATCH RFC v5 0/5] Update tests/qemu-iotests failing cases for the s390 platform
v5: 1. Add a pc specific output file for test 051. 2. Add a filter to test case 051 to filter s390 specific warnings. 3. Check whether the machine type is pc or not rather than check whether the machine type is s390. 4. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name. v4: 1. Generate all patches based on the latest master branch. 2. Rearrange patches v3: 1. Fix a typo in v2. v2: 1. Drop the patches for test 039 for it has been fixed in upstream. 2. Integrate patches for test 071, 067 and 087. 3. Keep the other patches. v1: 1. updated the test suite to be default-machine-type-aware, from the previous platform-aware 2. created a new patch "qemu-iotests: run qemu with -nodefaults" to counterpart the impact from the commit: c88930a6866e74953e931ae749781e98e486e5c8 qemu-char: Permit only a single "stdio" character device When more than one is used, the terminal settings aren't restored correctly on exit. Fixable. However, such usage makes no sense, because the users race for input, so outlaw it instead. If you want to connect multiple things to stdio, use the mux chardev. 3. updated all the checking of platform name to the current machine name Xiao Guang Chen (5): qemu-iotests: qemu machine type support qemu-iotests: run qemu with -nodefaults qemu-iotests: s390x: fix test 041 qemu-iotests: s390x: fix test 055 qemu-iotests: s390x: fix test 051 tests/qemu-iotests/041 | 6 + tests/qemu-iotests/051 | 79 +--- tests/qemu-iotests/051.out | 160 +-- tests/qemu-iotests/051.pc.out| 427 +++ tests/qemu-iotests/055 | 9 + tests/qemu-iotests/067 | 8 +- tests/qemu-iotests/067.out | 266 +--- tests/qemu-iotests/071.out | 4 - tests/qemu-iotests/087.out | 12 -- tests/qemu-iotests/check | 5 + tests/qemu-iotests/common| 1 + tests/qemu-iotests/common.config | 11 +- tests/qemu-iotests/common.filter | 7 + tests/qemu-iotests/common.qemu | 2 +- tests/qemu-iotests/iotests.py| 1 + 15 files changed, 570 insertions(+), 428 deletions(-) create mode 100644 tests/qemu-iotests/051.pc.out -- 1.9.1
[Qemu-devel] [PATCH RFC v5 5/5] qemu-iotests: s390x: fix test 051
The tests for device type "ide_cd" should only be tested for the pc platform. The default device id of hard disk on the s390 platform differs to that of the x86 platform. A new variable device_id is defined and "virtio0" set for the s390 platform. A x86 platform specific output file is also needed. A new filter was added to filter s390 warnings. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com --- tests/qemu-iotests/051 | 79 +--- tests/qemu-iotests/051.out | 160 +-- tests/qemu-iotests/051.pc.out| 427 +++ tests/qemu-iotests/common.filter | 7 + 4 files changed, 532 insertions(+), 141 deletions(-) create mode 100644 tests/qemu-iotests/051.pc.out diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 11c858f..fedf2d4 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -137,13 +137,19 @@ run_qemu -drive if=ide run_qemu -drive if=virtio run_qemu -drive if=scsi -run_qemu -drive if=none,id=disk -device ide-cd,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk - -run_qemu -drive if=none,id=disk -device ide-drive,drive=disk -run_qemu -drive if=none,id=disk -device ide-hd,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk +case "$QEMU_DEFAULT_MACHINE" in +pc) +run_qemu -drive if=none,id=disk -device ide-cd,drive=disk +run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk + +run_qemu -drive if=none,id=disk -device ide-drive,drive=disk +run_qemu -drive if=none,id=disk -device ide-hd,drive=disk +run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk +run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk +;; +*) +;; +esac echo echo === Read-only === @@ -157,13 +163,19 @@ run_qemu -drive file="$TEST_IMG",if=ide,readonly=on run_qemu -drive file="$TEST_IMG",if=virtio,readonly=on run_qemu -drive file="$TEST_IMG",if=scsi,readonly=on -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-cd,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk +case "$QEMU_DEFAULT_MACHINE" in +pc) +run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-cd,drive=disk +run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-drive,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-hd,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk +run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-drive,drive=disk +run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-hd,drive=disk +run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk +run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk +;; +*) +;; +esac echo echo === Cache modes === @@ -172,12 +184,12 @@ echo # Cannot use the test image because cache=none might not work on the host FS # Use cdrom so that we won't get errors about missing media -run_qemu -drive media=cdrom,cache=none -run_qemu -drive media=cdrom,cache=directsync -run_qemu -drive media=cdrom,cache=writeback -run_qemu -drive media=cdrom,cache=writethrough -run_qemu -drive media=cdrom,cache=unsafe -run_qemu -drive media=cdrom,cache=invalid_value +run_qemu -drive if=scsi,media=cdrom,cache=none +run_qemu -drive if=scsi,media=cdrom,cache=directsync +run_qemu -drive if=scsi,media=cdrom,cache=writeback +run_qemu -drive if=scsi,media=cdrom,cache=writethrough +run_qemu -drive if=scsi,media=cdrom,cache=unsafe +run_qemu -drive if=scsi,media=cdrom,cache=invalid_value echo echo === Specifying the protocol layer === @@ -241,28 +253,37 @@ echo echo === Snapshot mode === echo +case "$QEMU_DEFAULT_MACHINE" in +pc) +device_id="ide0-hd0" +;; +*) +device_id="virtio0" +;; +esac + $QEMU_IO -c "write -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io -echo 'qemu-io ide0-hd
[Qemu-devel] [PATCH RFC v5 4/5] qemu-iotests: s390x: fix test 055
There is no 'ide-cd' device defined on s390 platform, so test_medium_not_found() test should be skipped. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com --- tests/qemu-iotests/055 | 9 + 1 file changed, 9 insertions(+) diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index e81d4d0..0f80bc1 100755 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -104,11 +104,17 @@ class TestSingleDrive(iotests.QMPTestCase): self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img) def test_medium_not_found(self): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('drive-backup', device='ide1-cd0', target=target_img, sync='full') self.assert_qmp(result, 'error/class', 'GenericError') def test_medium_not_found_blockdev_backup(self): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('blockdev-backup', device='ide1-cd0', target='drive1', sync='full') self.assert_qmp(result, 'error/class', 'GenericError') @@ -320,6 +326,9 @@ class TestSingleTransaction(iotests.QMPTestCase): self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img) def do_test_medium_not_found(self, cmd, target): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('transaction', actions=[{ 'type': cmd, 'data': { 'device': 'ide1-cd0', -- 1.9.1
[Qemu-devel] [PATCH RFC v5 2/5] qemu-iotests: run qemu with -nodefaults and fix 067, 071, 087
This patch fixes an io test suite issue that was introduced with the commit c88930a6866e74953e931ae749781e98e486e5c8 'qemu-char: Permit only a single "stdio" character device'. The option supresses the creation of default devices such as the floopy and cdrom. Output files for test case 067, 071 and 087 need to be updated to accommodate this change. Use virtio-blk instead of virtio-blk-pci as the device driver for test case 067. For virtio-blk-pci is the same with virtio-blk as device driver but other platform such as s390 may not recognize the virtio-blk-pci. Reviewed-by: Michael Mueller Signed-off-by: Xiao Guang Chen --- tests/qemu-iotests/067 | 8 +- tests/qemu-iotests/067.out | 266 +-- tests/qemu-iotests/071.out | 4 - tests/qemu-iotests/087.out | 12 -- tests/qemu-iotests/common| 1 + tests/qemu-iotests/common.config | 2 +- tests/qemu-iotests/common.qemu | 2 +- 7 files changed, 8 insertions(+), 287 deletions(-) diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067 index 0508c69..d979645 100755 --- a/tests/qemu-iotests/067 +++ b/tests/qemu-iotests/067 @@ -57,7 +57,7 @@ echo echo === -drive/-device and device_del === echo -run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0 <"${fifo_out}" \ 2>&1 \ <"${fifo_in}" & -- 1.9.1
[Qemu-devel] [PATCH RFC v5 3/5] qemu-iotests: s390x: fix test 041
There is no 'ide-cd' device defined on s390 platform, so test_medium_not_found() test should be skipped. Signed-off-by: Xiao Guang Chen,che...@linux.vnet.ibm.com --- tests/qemu-iotests/041 | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 59a8f73..c6abe3c 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -197,6 +197,9 @@ class TestSingleDrive(ImageMirroringTestCase): 'target image does not match source after mirroring') def test_medium_not_found(self): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full', target=target_img) self.assert_qmp(result, 'error/class', 'GenericError') @@ -867,6 +870,9 @@ class TestRepairQuorum(ImageMirroringTestCase): if not self.has_quorum(): return +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full', node_name='repair0', replaces='img1', -- 1.9.1
Re: [Qemu-devel] [PATCH] checkpatch: port fix from kernel "## is not a valid modifier"
Am 09.02.2015 um 23:54 schrieb Peter Maydell: > On 9 February 2015 at 19:43, Christian Borntraeger > wrote: >> From: Andy Whitcroft >> >> checkpatch currently loops on fpu/softfloat.c >> Turns out this is fixed in the Linux version of checkpatch. >> >> So this is a port of Andy Whitcrofts fix from Linux, >> Original commit was commit 89a883530fe7 ("checkpatch: ## is not a >> valid modifier") >> >> Cc: Andy Whitcroft >> Signed-off-by: Christian Borntraeger >> --- >> scripts/checkpatch.pl | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index 5df61f9..8635f4c 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -1061,7 +1061,9 @@ sub possible { >> case| >> else| >> asm|__asm__| >> - do >> + do| >> + \#| >> + \#\#| > > Are you sure this line should end with a '|', given it's the > last item in the alternation ? Good question - dont know. But the kernel version looks the same.
Re: [Qemu-devel] Revert commit 5af35d7feccaa7d26b72c6c3d14116421d736b36 - "usb-host-libusb: Fix reset handling"
Hi, On 09-02-15 22:09, Dennis Ostermann wrote: Hi there, please revert commit 5af35d7feccaa7d26b72c6c3d14116421d736b36 - "usb-host-libusb: Fix reset handling" This breaks usb pass through of FTDI based usb devices: On the host: lsusb | grep FT2232 Bus 003 Device 008: ID 0403:6010 Future Technology Devices International, Ltd FT2232C Dual USB-UART/FIFO IC ~/qemu-install/bin$ sudo ./qemu-system-x86_64 -monitor telnet:127.0.0.1:1234,server,nowait -hda /dev/sdd2 -redir tcp:20022::22 --enable-kvm -cpu host -smp 4 -vga vmware --vnc :0 -m 8192 -usb -device usb-ehci,id=ehci -device usb-host,bus=ehci.0,vendorid=0x0403,productid=0x6010 WARNING: Image format was not specified for '/dev/sdd2' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/005: No such file or directory libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/006: No such file or directory libusbx: error [_get_usbfs_fd] libusbx couldn't open USB device /dev/bus/usb/003/007: No such device The device gets reset again and again and is re-enumerated every time and finally not passed through. This looks like the device drops of the bus when it is reset, that is not normal behavior, there seems to be something unique to your setup causing this. Have you tried this on multiple machines / different usb ports on your pc ? This may be something weird with the usb controller in your machine. After reverting the commit: ~/qemu-patched-install/bin$ sudo ./qemu-system-x86_64 -monitor telnet:127.0.0.1:1234,server,nowait -hda /dev/sdd2 -redir tcp:20022::22 --enable-kvm -cpu host -smp 4 -vga vmware --vnc :0 -m 8192 -usb -device usb-ehci,id=ehci -device usb-host,bus=ehci.0,vendorid=0x0403,productid=0x6010 WARNING: Image format was not specified for '/dev/sdd2' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. The device appears in the guest OS and can be used. Tested with HEAD and several libusb versions. Affects at least FTDI FT2232H and FTDI FT232R. Which versions of libusb have you tested exactly ? Regards, Hans
Re: [Qemu-devel] [PATCH v2 2/2] xen-pt: fix Out-of-bounds read
On Tue, 10 Feb 2015, arei.gong...@huawei.com wrote: > From: Gonglei > > The array length of s->real_device.io_regions[] is > "PCI_NUM_REGIONS - 1". > > Signed-off-by: Gonglei Acked-by: Stefano Stabellini I am happy for these patches to go in via the qemu-trivial tree. > hw/xen/xen_pt_config_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c > index 710fe50..d99c22e 100644 > --- a/hw/xen/xen_pt_config_init.c > +++ b/hw/xen/xen_pt_config_init.c > @@ -438,7 +438,7 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState *s, > XenPTReg *cfg_entry, > > /* get BAR index */ > index = xen_pt_bar_offset_to_index(reg->offset); > -if (index < 0 || index >= PCI_NUM_REGIONS) { > +if (index < 0 || index >= PCI_NUM_REGIONS - 1) { > XEN_PT_ERR(&s->dev, "Internal error: Invalid BAR index [%d].\n", > index); > return -1; > } > -- > 1.7.12.4 > >
Re: [Qemu-devel] [PATCH v2 2/2] xen-pt: fix Out-of-bounds read
On 2015/2/10 16:11, Stefano Stabellini wrote: > On Tue, 10 Feb 2015, arei.gong...@huawei.com wrote: >> From: Gonglei >> >> The array length of s->real_device.io_regions[] is >> "PCI_NUM_REGIONS - 1". >> >> Signed-off-by: Gonglei > > Acked-by: Stefano Stabellini > > I am happy for these patches to go in via the qemu-trivial tree. > Thanks. Let me cc /mjt. Regards, -Gonglei >> hw/xen/xen_pt_config_init.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c >> index 710fe50..d99c22e 100644 >> --- a/hw/xen/xen_pt_config_init.c >> +++ b/hw/xen/xen_pt_config_init.c >> @@ -438,7 +438,7 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState >> *s, XenPTReg *cfg_entry, >> >> /* get BAR index */ >> index = xen_pt_bar_offset_to_index(reg->offset); >> -if (index < 0 || index >= PCI_NUM_REGIONS) { >> +if (index < 0 || index >= PCI_NUM_REGIONS - 1) { >> XEN_PT_ERR(&s->dev, "Internal error: Invalid BAR index [%d].\n", >> index); >> return -1; >> } >> -- >> 1.7.12.4 >> >>
Re: [Qemu-devel] [PATCH v4] sheepdog: selectable object size support
(2015/02/10 12:10), Liu Yuan wrote: On Tue, Jan 27, 2015 at 05:35:27PM +0900, Teruaki Ishizaki wrote: Previously, qemu block driver of sheepdog used hard-coded VDI object size. This patch enables users to handle "block_size_shift" value for calculating VDI object size. When you start qemu, you don't need to specify additional command option. But when you create the VDI which doesn't have default object size with qemu-img command, you specify block_size_shift option. If you want to create a VDI of 8MB(1 << 23) object size, you need to specify following command option. # qemu-img create -o block_size_shift=23 sheepdog:test1 100M In addition, when you don't specify qemu-img command option, a default value of sheepdog cluster is used for creating VDI. # qemu-img create sheepdog:test2 100M Signed-off-by: Teruaki Ishizaki --- V4: - Limit a read/write buffer size for creating a preallocated VDI. - Replace a parse function for the block_size_shift option. - Fix an error message. V3: - Delete the needless operation of buffer. - Delete the needless operations of request header. for SD_OP_GET_CLUSTER_DEFAULT. - Fix coding style problems. V2: - Fix coding style problem (white space). - Add members, store_policy and block_size_shift to struct SheepdogVdiReq. - Initialize request header to use block_size_shift specified by user. --- block/sheepdog.c | 138 ++--- include/block/block_int.h |1 + 2 files changed, 119 insertions(+), 20 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index be3176f..a43b947 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -37,6 +37,7 @@ #define SD_OP_READ_VDIS 0x15 #define SD_OP_FLUSH_VDI 0x16 #define SD_OP_DEL_VDI0x17 +#define SD_OP_GET_CLUSTER_DEFAULT 0x18 #define SD_FLAG_CMD_WRITE0x01 #define SD_FLAG_CMD_COW 0x02 @@ -167,7 +168,8 @@ typedef struct SheepdogVdiReq { uint32_t base_vdi_id; uint8_t copies; uint8_t copy_policy; -uint8_t reserved[2]; +uint8_t store_policy; +uint8_t block_size_shift; uint32_t snapid; uint32_t type; uint32_t pad[2]; @@ -186,6 +188,21 @@ typedef struct SheepdogVdiRsp { uint32_t pad[5]; } SheepdogVdiRsp; +typedef struct SheepdogClusterRsp { +uint8_t proto_ver; +uint8_t opcode; +uint16_t flags; +uint32_t epoch; +uint32_t id; +uint32_t data_length; +uint32_t result; +uint8_t nr_copies; +uint8_t copy_policy; +uint8_t block_size_shift; +uint8_t __pad1; +uint32_t __pad2[6]; +} SheepdogClusterRsp; + typedef struct SheepdogInode { char name[SD_MAX_VDI_LEN]; char tag[SD_MAX_VDI_TAG_LEN]; @@ -1544,6 +1561,7 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot, hdr.vdi_size = s->inode.vdi_size; hdr.copy_policy = s->inode.copy_policy; hdr.copies = s->inode.nr_copies; +hdr.block_size_shift = s->inode.block_size_shift; ret = do_req(fd, s->aio_context, (SheepdogReq *)&hdr, buf, &wlen, &rlen); @@ -1569,9 +1587,12 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot, static int sd_prealloc(const char *filename, Error **errp) { BlockDriverState *bs = NULL; +BDRVSheepdogState *base = NULL; +unsigned long buf_size; uint32_t idx, max_idx; +uint32_t object_size; int64_t vdi_size; -void *buf = g_malloc0(SD_DATA_OBJ_SIZE); +void *buf = NULL; int ret; ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, @@ -1585,18 +1606,24 @@ static int sd_prealloc(const char *filename, Error **errp) ret = vdi_size; goto out; } -max_idx = DIV_ROUND_UP(vdi_size, SD_DATA_OBJ_SIZE); + +base = bs->opaque; +object_size = (UINT32_C(1) << base->inode.block_size_shift); +buf_size = MIN(object_size, SD_DATA_OBJ_SIZE); +buf = g_malloc0(buf_size); + +max_idx = DIV_ROUND_UP(vdi_size, buf_size); for (idx = 0; idx < max_idx; idx++) { /* * The created image can be a cloned image, so we need to read * a data from the source image. */ -ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE); +ret = bdrv_pread(bs, idx * buf_size, buf, buf_size); if (ret < 0) { goto out; } -ret = bdrv_pwrite(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE); +ret = bdrv_pwrite(bs, idx * buf_size, buf, buf_size); if (ret < 0) { goto out; } @@ -1669,6 +1696,21 @@ static int parse_redundancy(BDRVSheepdogState *s, const char *opt) return 0; } +static int parse_block_size_shift(BDRVSheepdogState *s, QemuOpts *opt) +{ +struct SheepdogInode *inode = &s->inode; +inode->block_size_shift = +(uint8_t)qemu_opt_get_number_del(opt, "block_size_shift", 0); +if (inode->block_size_shift
Re: [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all()
On 09/02/2015 19:38, Max Reitz wrote: > Currently, bdrv_close_all() force-closes all BDSs with a BlockBackend, > which can lead to data corruption (see the iotest added in the final > patch of this series) and is most certainly very ugly. > > This series reworks bdrv_close_all() to notify all owners of a > BlockBackend that they should release their reference (and additionally > the monitor releases all its references to BB-less BDSs). This way, > force-closing becomes unnecessary. The NBD parts look good. Paolo
Re: [Qemu-devel] [PATCH v1 1/2] vhost-user: support SET_MEM_TABLE waite the result of mmap
On Tue, Feb 10, 2015 at 01:48:12PM +0800, linhaifeng wrote: > From: Linhaifeng > > Slave should reply to master and set u64 to 0 if > mmap all regions success otherwise set u64 to 1. > > Signed-off-by: Linhaifeng How does this work with existig slaves though? > --- > docs/specs/vhost-user.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 650bb18..c96bf6b 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -171,6 +171,7 @@ Message types >Id: 5 >Equivalent ioctl: VHOST_SET_MEM_TABLE >Master payload: memory regions description > + Slave payload: u64 (0:success >0:failed) > >Sets the memory map regions on the slave so it can translate the vring >addresses. In the ancillary data there is an array of file descriptors > -- > 1.7.12.4 >
Re: [Qemu-devel] [PATCH v4] sheepdog: selectable object size support
On Tue, Feb 10, 2015 at 05:22:02PM +0900, Teruaki Ishizaki wrote: > (2015/02/10 12:10), Liu Yuan wrote: > >On Tue, Jan 27, 2015 at 05:35:27PM +0900, Teruaki Ishizaki wrote: > >>Previously, qemu block driver of sheepdog used hard-coded VDI object size. > >>This patch enables users to handle "block_size_shift" value for > >>calculating VDI object size. > >> > >>When you start qemu, you don't need to specify additional command option. > >> > >>But when you create the VDI which doesn't have default object size > >>with qemu-img command, you specify block_size_shift option. > >> > >>If you want to create a VDI of 8MB(1 << 23) object size, > >>you need to specify following command option. > >> > >> # qemu-img create -o block_size_shift=23 sheepdog:test1 100M > >> > >>In addition, when you don't specify qemu-img command option, > >>a default value of sheepdog cluster is used for creating VDI. > >> > >> # qemu-img create sheepdog:test2 100M > >> > >>Signed-off-by: Teruaki Ishizaki > >>--- > >>V4: > >> - Limit a read/write buffer size for creating a preallocated VDI. > >> - Replace a parse function for the block_size_shift option. > >> - Fix an error message. > >> > >>V3: > >> - Delete the needless operation of buffer. > >> - Delete the needless operations of request header. > >>for SD_OP_GET_CLUSTER_DEFAULT. > >> - Fix coding style problems. > >> > >>V2: > >> - Fix coding style problem (white space). > >> - Add members, store_policy and block_size_shift to struct SheepdogVdiReq. > >> - Initialize request header to use block_size_shift specified by user. > >>--- > >> block/sheepdog.c | 138 > >> ++--- > >> include/block/block_int.h |1 + > >> 2 files changed, 119 insertions(+), 20 deletions(-) > >> > >>diff --git a/block/sheepdog.c b/block/sheepdog.c > >>index be3176f..a43b947 100644 > >>--- a/block/sheepdog.c > >>+++ b/block/sheepdog.c > >>@@ -37,6 +37,7 @@ > >> #define SD_OP_READ_VDIS 0x15 > >> #define SD_OP_FLUSH_VDI 0x16 > >> #define SD_OP_DEL_VDI0x17 > >>+#define SD_OP_GET_CLUSTER_DEFAULT 0x18 > >> > >> #define SD_FLAG_CMD_WRITE0x01 > >> #define SD_FLAG_CMD_COW 0x02 > >>@@ -167,7 +168,8 @@ typedef struct SheepdogVdiReq { > >> uint32_t base_vdi_id; > >> uint8_t copies; > >> uint8_t copy_policy; > >>-uint8_t reserved[2]; > >>+uint8_t store_policy; > >>+uint8_t block_size_shift; > >> uint32_t snapid; > >> uint32_t type; > >> uint32_t pad[2]; > >>@@ -186,6 +188,21 @@ typedef struct SheepdogVdiRsp { > >> uint32_t pad[5]; > >> } SheepdogVdiRsp; > >> > >>+typedef struct SheepdogClusterRsp { > >>+uint8_t proto_ver; > >>+uint8_t opcode; > >>+uint16_t flags; > >>+uint32_t epoch; > >>+uint32_t id; > >>+uint32_t data_length; > >>+uint32_t result; > >>+uint8_t nr_copies; > >>+uint8_t copy_policy; > >>+uint8_t block_size_shift; > >>+uint8_t __pad1; > >>+uint32_t __pad2[6]; > >>+} SheepdogClusterRsp; > >>+ > >> typedef struct SheepdogInode { > >> char name[SD_MAX_VDI_LEN]; > >> char tag[SD_MAX_VDI_TAG_LEN]; > >>@@ -1544,6 +1561,7 @@ static int do_sd_create(BDRVSheepdogState *s, > >>uint32_t *vdi_id, int snapshot, > >> hdr.vdi_size = s->inode.vdi_size; > >> hdr.copy_policy = s->inode.copy_policy; > >> hdr.copies = s->inode.nr_copies; > >>+hdr.block_size_shift = s->inode.block_size_shift; > >> > >> ret = do_req(fd, s->aio_context, (SheepdogReq *)&hdr, buf, &wlen, > >> &rlen); > >> > >>@@ -1569,9 +1587,12 @@ static int do_sd_create(BDRVSheepdogState *s, > >>uint32_t *vdi_id, int snapshot, > >> static int sd_prealloc(const char *filename, Error **errp) > >> { > >> BlockDriverState *bs = NULL; > >>+BDRVSheepdogState *base = NULL; > >>+unsigned long buf_size; > >> uint32_t idx, max_idx; > >>+uint32_t object_size; > >> int64_t vdi_size; > >>-void *buf = g_malloc0(SD_DATA_OBJ_SIZE); > >>+void *buf = NULL; > >> int ret; > >> > >> ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | > >> BDRV_O_PROTOCOL, > >>@@ -1585,18 +1606,24 @@ static int sd_prealloc(const char *filename, Error > >>**errp) > >> ret = vdi_size; > >> goto out; > >> } > >>-max_idx = DIV_ROUND_UP(vdi_size, SD_DATA_OBJ_SIZE); > >>+ > >>+base = bs->opaque; > >>+object_size = (UINT32_C(1) << base->inode.block_size_shift); > >>+buf_size = MIN(object_size, SD_DATA_OBJ_SIZE); > >>+buf = g_malloc0(buf_size); > >>+ > >>+max_idx = DIV_ROUND_UP(vdi_size, buf_size); > >> > >> for (idx = 0; idx < max_idx; idx++) { > >> /* > >> * The created image can be a cloned image, so we need to read > >> * a data from the source image. > >> */ > >>-ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, > >>SD_DATA_OBJ_SIZE); > >>+ret = bdrv_pread(bs, idx * buf_size, buf, buf_size); > >> if (ret
[Qemu-devel] [PULL 21/46] libcacard: stop linking against every single 3rd party library
From: "Daniel P. Berrange" Building QEMU results in a libcacard.so that links against practically the entire world linux-vdso.so.1 => (0x7fff71e99000) libssl3.so => /usr/lib64/libssl3.so (0x7f49f94b6000) libsmime3.so => /usr/lib64/libsmime3.so (0x7f49f928e000) libnss3.so => /usr/lib64/libnss3.so (0x7f49f8f67000) libnssutil3.so => /usr/lib64/libnssutil3.so (0x7f49f8d3b000) libplds4.so => /usr/lib64/libplds4.so (0x7f49f8b36000) libplc4.so => /usr/lib64/libplc4.so (0x7f49f8931000) libnspr4.so => /usr/lib64/libnspr4.so (0x7f49f86f2000) libdl.so.2 => /usr/lib64/libdl.so.2 (0x7f49f84ed000) libm.so.6 => /usr/lib64/libm.so.6 (0x7f49f81e5000) libgthread-2.0.so.0 => /usr/lib64/libgthread-2.0.so.0 (0x7f49f7fe3000) librt.so.1 => /usr/lib64/librt.so.1 (0x7f49f7dda000) libz.so.1 => /usr/lib64/libz.so.1 (0x7f49f7bc4000) libcap-ng.so.0 => /usr/lib64/libcap-ng.so.0 (0x7f49f79be000) libuuid.so.1 => /usr/lib64/libuuid.so.1 (0x7f49f77b8000) libgnutls.so.28 => /usr/lib64/libgnutls.so.28 (0x7f49f749a000) libSDL-1.2.so.0 => /usr/lib64/libSDL-1.2.so.0 (0x7f49f71fd000) libpthread.so.0 => /usr/lib64/libpthread.so.0 (0x7f49f6fe) libvte.so.9 => /usr/lib64/libvte.so.9 (0x7f49f6d3f000) libXext.so.6 => /usr/lib64/libXext.so.6 (0x7f49f6b2d000) libgtk-x11-2.0.so.0 => /usr/lib64/libgtk-x11-2.0.so.0 (0x7f49f64a) libgdk-x11-2.0.so.0 => /usr/lib64/libgdk-x11-2.0.so.0 (0x7f49f61de000) libpangocairo-1.0.so.0 => /usr/lib64/libpangocairo-1.0.so.0 (0x7f49f5fd1000) libatk-1.0.so.0 => /usr/lib64/libatk-1.0.so.0 (0x7f49f5daa000) libcairo.so.2 => /usr/lib64/libcairo.so.2 (0x7f49f5a9d000) libgdk_pixbuf-2.0.so.0 => /usr/lib64/libgdk_pixbuf-2.0.so.0 (0x7f49f5878000) libgio-2.0.so.0 => /usr/lib64/libgio-2.0.so.0 (0x7f49f550) libpangoft2-1.0.so.0 => /usr/lib64/libpangoft2-1.0.so.0 (0x7f49f52eb000) libpango-1.0.so.0 => /usr/lib64/libpango-1.0.so.0 (0x7f49f50a) libgobject-2.0.so.0 => /usr/lib64/libgobject-2.0.so.0 (0x7f49f4e4e000) libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7f49f4b15000) libfontconfig.so.1 => /usr/lib64/libfontconfig.so.1 (0x7f49f48d6000) libfreetype.so.6 => /usr/lib64/libfreetype.so.6 (0x7f49f462b000) libX11.so.6 => /usr/lib64/libX11.so.6 (0x7f49f42e8000) libxenstore.so.3.0 => /usr/lib64/libxenstore.so.3.0 (0x7f49f40de000) libxenctrl.so.4.4 => /usr/lib64/libxenctrl.so.4.4 (0x7f49f3eb6000) libxenguest.so.4.4 => /usr/lib64/libxenguest.so.4.4 (0x7f49f3c8b000) libseccomp.so.2 => /usr/lib64/libseccomp.so.2 (0x7f49f3a74000) librdmacm.so.1 => /usr/lib64/librdmacm.so.1 (0x7f49f385d000) libibverbs.so.1 => /usr/lib64/libibverbs.so.1 (0x7f49f364a000) libutil.so.1 => /usr/lib64/libutil.so.1 (0x7f49f3447000) libc.so.6 => /usr/lib64/libc.so.6 (0x7f49f3089000) /lib64/ld-linux-x86-64.so.2 (0x7f49f9902000) libp11-kit.so.0 => /usr/lib64/libp11-kit.so.0 (0x7f49f2e23000) libtspi.so.1 => /usr/lib64/libtspi.so.1 (0x7f49f2bb2000) libtasn1.so.6 => /usr/lib64/libtasn1.so.6 (0x7f49f299f000) libnettle.so.4 => /usr/lib64/libnettle.so.4 (0x7f49f276d000) libhogweed.so.2 => /usr/lib64/libhogweed.so.2 (0x7f49f2545000) libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x7f49f22cd000) libncurses.so.5 => /usr/lib64/libncurses.so.5 (0x7f49f20a5000) libtinfo.so.5 => /usr/lib64/libtinfo.so.5 (0x7f49f1e7a000) libgmodule-2.0.so.0 => /usr/lib64/libgmodule-2.0.so.0 (0x7f49f1c76000) libXfixes.so.3 => /usr/lib64/libXfixes.so.3 (0x7f49f1a6f000) libXrender.so.1 => /usr/lib64/libXrender.so.1 (0x7f49f1865000) libXinerama.so.1 => /usr/lib64/libXinerama.so.1 (0x7f49f1662000) libXi.so.6 => /usr/lib64/libXi.so.6 (0x7f49f1452000) libXrandr.so.2 => /usr/lib64/libXrandr.so.2 (0x7f49f1247000) libXcursor.so.1 => /usr/lib64/libXcursor.so.1 (0x7f49f103c000) libXcomposite.so.1 => /usr/lib64/libXcomposite.so.1 (0x7f49f0e39000) libXdamage.so.1 => /usr/lib64/libXdamage.so.1 (0x7f49f0c35000) libharfbuzz.so.0 => /usr/lib64/libharfbuzz.so.0 (0x7f49f09dd000) libpixman-1.so.0 => /usr/lib64/libpixman-1.so.0 (0x7f49f072f000) libEGL.so.1 => /usr/lib64/libEGL.so.1 (0x7f49f0505000) libpng16.so.16 => /usr/lib64/libpng16.so.16 (0x7f49f02d2000) libxcb-shm.so.0 => /usr/lib64/libxcb-shm.so.0 (0x7f49f00cd000) libxcb-render.so.0 => /usr/lib64/libxcb-render.so.0 (0x7f49efec3000) libxcb.so.1 => /usr/lib64/libxc
[Qemu-devel] [PULL 39/46] migration: Fix warning caused by missing declaration of vmstate_dummy
From: Stefan Weil Warning from the Sparse static analysis tool: stubs/vmstate.c:4:26: warning: symbol 'vmstate_dummy' was not declared. Should it be static? Cc: Juan Quintela Cc: Amit Shah Signed-off-by: Stefan Weil Signed-off-by: Michael Tokarev --- include/migration/vmstate.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 0b26bc6..c20f2d1 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -139,9 +139,7 @@ struct VMStateDescription { const VMStateSubsection *subsections; }; -#ifdef CONFIG_USER_ONLY extern const VMStateDescription vmstate_dummy; -#endif extern const VMStateInfo vmstate_info_bool; -- 2.1.4