Re: [PATCH 3/5] qom: Remove module_obj_name parameter from OBJECT_DECLARE* macros

2020-09-17 Thread Thomas Huth
On 16/09/2020 20.25, Eduardo Habkost wrote: > One of the goals of having less boilerplate on QOM declarations > is to avoid human error. Requiring an extra argument that is > never used is an opportunity for mistakes. > > Remove the unused argument from OBJECT_DECLARE_TYPE and >

Re: [PATCH 07/14] block/blklogwrites: drop error propagation

2020-09-17 Thread Markus Armbruster
Ari Sundholm writes: > Hi, > > On 9/11/20 11:03 AM, Markus Armbruster wrote: >> Ari Sundholm writes: >> >>> Hi Vladimir! >>> >>> Thank you for working on this. My comments below. >>> >>> On 9/9/20 9:59 PM, Vladimir Sementsov-Ogievskiy wrote: It's simple to avoid error propagation in

Re: [PATCH v1 0/2] Add timeout mechanism to qmp actions

2020-09-17 Thread Zhenyu Ye
Hi Stefan, On 2020/9/14 21:27, Stefan Hajnoczi wrote: >> >> Theoretically, everything running in an iothread is asynchronous. However, >> some 'asynchronous' actions are not non-blocking entirely, such as >> io_submit(). This will block while the iodepth is too big and I/O pressure >> is too

Re: [PATCH 4/5] [automated] Use OBJECT_DECLARE_TYPE when possible

2020-09-17 Thread Cédric Le Goater
On 9/16/20 8:25 PM, Eduardo Habkost wrote: > This converts existing DECLARE_OBJ_CHECKERS usage to > OBJECT_DECLARE_TYPE when possible. > > $ ./scripts/codeconverter/converter.py -i \ >--pattern=AddObjectDeclareType $(git grep -l '' -- '*.[ch]') > > Signed-off-by: Eduardo Habkost For the

Re: [PATCH 3/5] qom: Remove module_obj_name parameter from OBJECT_DECLARE* macros

2020-09-17 Thread Cornelia Huck
On Wed, 16 Sep 2020 14:25:17 -0400 Eduardo Habkost wrote: > One of the goals of having less boilerplate on QOM declarations > is to avoid human error. Requiring an extra argument that is > never used is an opportunity for mistakes. > > Remove the unused argument from OBJECT_DECLARE_TYPE and >

Re: [PATCH 3/5] qom: Remove module_obj_name parameter from OBJECT_DECLARE* macros

2020-09-17 Thread Cédric Le Goater
On 9/16/20 8:25 PM, Eduardo Habkost wrote: > One of the goals of having less boilerplate on QOM declarations > is to avoid human error. Requiring an extra argument that is > never used is an opportunity for mistakes. > > Remove the unused argument from OBJECT_DECLARE_TYPE and >

Re: [PATCH v1 0/2] Add timeout mechanism to qmp actions

2020-09-17 Thread Zhenyu Ye
Hi Daniel, On 2020/9/14 22:42, Daniel P. Berrangé wrote: > On Tue, Aug 11, 2020 at 09:54:08PM +0800, Zhenyu Ye wrote: >> Hi Kevin, >> >> On 2020/8/10 23:38, Kevin Wolf wrote: >>> Am 10.08.2020 um 16:52 hat Zhenyu Ye geschrieben: Before doing qmp actions, we need to lock the

Re: [PATCH 3/5] qom: Remove module_obj_name parameter from OBJECT_DECLARE* macros

2020-09-17 Thread Igor Mammedov
On Wed, 16 Sep 2020 14:25:17 -0400 Eduardo Habkost wrote: > One of the goals of having less boilerplate on QOM declarations > is to avoid human error. Requiring an extra argument that is > never used is an opportunity for mistakes. > > Remove the unused argument from OBJECT_DECLARE_TYPE and >

RE: [PATCH 5/5] [automated] Use OBJECT_DECLARE_SIMPLE_TYPE when possible

2020-09-17 Thread Paul Durrant
> -Original Message- > From: Eduardo Habkost > Sent: 16 September 2020 19:25 > To: qemu-de...@nongnu.org > Cc: Paolo Bonzini ; Daniel P. Berrange > ; Gonglei (Arei) > ; Igor Mammedov ; Laurent > Vivier ; > Amit Shah ; Stefan Berger ; Michael > S. Tsirkin > ; Greg Kurz ; Christian

RE: [PATCH 3/5] qom: Remove module_obj_name parameter from OBJECT_DECLARE* macros

2020-09-17 Thread Paul Durrant
> -Original Message- > From: Eduardo Habkost > Sent: 16 September 2020 19:25 > To: qemu-de...@nongnu.org > Cc: Paolo Bonzini ; Daniel P. Berrange > ; Marc-André Lureau > ; Gerd Hoffmann ; Michael S. > Tsirkin ; > Peter Maydell ; Corey Minyard > ; Cédric Le Goater > ; David Gibson ;

RE: [PATCH 4/5] [automated] Use OBJECT_DECLARE_TYPE when possible

2020-09-17 Thread Paul Durrant
> -Original Message- > From: Eduardo Habkost > Sent: 16 September 2020 19:25 > To: qemu-de...@nongnu.org > Cc: Paolo Bonzini ; Daniel P. Berrange > ; Peter Maydell > ; Andrzej Zaborowski ; Alistair > Francis > ; Kevin Wolf ; Max Reitz > ; Mark Cave- > Ayland ; David Gibson > ; Richard

Re: [PATCH v5 00/10] preallocate filter

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
01.09.2020 18:07, Max Reitz wrote: On 27.08.20 23:08, Vladimir Sementsov-Ogievskiy wrote: 21.08.2020 17:11, Vladimir Sementsov-Ogievskiy wrote: Hi all! Here is a filter, which does preallocation on write. In Virtuozzo we have to deal with some custom distributed storage solution, where

Re: [PATCH 13/14] block/qcow2: qcow2_do_open: deal with errp

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
17.09.2020 19:23, Alberto Garcia wrote: On Wed 09 Sep 2020 08:59:29 PM CEST, Vladimir Sementsov-Ogievskiy wrote: 1. Drop extra error propagation 2. Set errp always on failure. Generic bdrv_open_driver supports driver functions which can return negative value and forget to set errp. That's a

[PATCH v2 03/13] block: check return value of bdrv_open_child and drop error propagation

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
This patch is generated by cocci script: @@ symbol bdrv_open_child, errp, local_err; expression file; @@ file = bdrv_open_child(..., -_err +errp ); - if (local_err) + if (!file) { ... - error_propagate(errp,

Re: [PATCH 09/14] block/qcow2: qcow2_get_specific_info(): drop error propagation

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
17.09.2020 19:32, Alberto Garcia wrote: On Wed 09 Sep 2020 08:59:25 PM CEST, Vladimir Sementsov-Ogievskiy wrote: + * On success return true with bm_list set (probably to NULL, if no bitmaps), " probably " ? :-) I note this as "set to NULL" is not obvious thing (is it "unset" ? :).. And

[PATCH v2 02/13] block: use return status of bdrv_append()

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
Now bdrv_append returns status and we can drop all the local_err things around it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia --- block.c | 5 + block/backup-top.c | 20 block/commit.c

[PATCH v2 07/13] blockjob: return status from block_job_set_speed()

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
Better to return status together with setting errp. It allows to avoid error propagation in the caller. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia --- include/block/blockjob.h | 2 +- blockjob.c | 18 -- 2

[PATCH v2 06/13] block/mirror: drop extra error propagation in commit_active_start()

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
Let's check return value of mirror_start_job to check for failure instead of local_err. Rename ret to job, as ret is usually integer variable. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia --- block/mirror.c | 12 +--- 1 file changed,

[PATCH v2 11/13] block/qcow2: read_cache_sizes: return status value

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
It's better to return status together with setting errp. It allows to reduce error propagation. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia --- block/qcow2.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff

Re: [PATCH 02/14] block: use return status of bdrv_append()

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
10.09.2020 19:10, Greg Kurz wrote: On Wed, 9 Sep 2020 21:59:18 +0300 Vladimir Sementsov-Ogievskiy wrote: Now bdrv_append returns status and we can drop all the local_err things around it. Signed-off-by: Vladimir Sementsov-Ogievskiy --- Reviewed-by: Greg Kurz Just one suggestion for a

[PATCH v2 00/13] block: deal with errp: part I

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
v2: 01-07: add Greg's and Alberto's r-bs 08: fix wording in commit message add Greg's r-b 09: fix header_updated logic, add comment, drop unrelated style-change [Alberto] 10: - fix commit header - add note about ERRP_GUARD in commit message - add Greg's r-b 11: add Greg's and Alberto's

[PATCH v2 01/13] block: return status from bdrv_append and friends

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
The recommended use of qemu error api assumes returning status together with setting errp and avoid void functions with errp parameter. Let's improve bdrv_append and some friends to reduce error-propagation overhead in further patches. Choose int return status, because bdrv_replace_node() has

[PATCH v2 04/13] blockdev: fix drive_backup_prepare() missed error

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
We leak local_err and don't report failure to the caller. It's definitely wrong, let's fix. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia --- blockdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/blockdev.c

[PATCH v2 10/13] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
It's better to return status together with setting errp. It makes possible to avoid error propagation. While being here, put ERRP_GUARD() to fix error_prepend(errp, ...) usage inside qcow2_store_persistent_dirty_bitmaps() (see the comment above ERRP_GUARD() definition in include/qapi/error.h)

[PATCH v2 09/13] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
It's recommended for bool functions with errp to return true on success and false on failure. Non-standard interfaces don't help to understand the code. The change is also needed to reduce error propagation. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h| 3 ++-

Re: [PATCH 10/14] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
17.09.2020 19:35, Alberto Garcia wrote: On Wed 09 Sep 2020 08:59:26 PM CEST, Vladimir Sementsov-Ogievskiy wrote: -/* qcow2_load_dirty_bitmaps() - * Return value is a hint for caller: true means that the Qcow2 header was - * updated. (false doesn't mean that the header should be updated by the

[PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error propagation

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
Don't use error propagation in qcow2_get_specific_info(). For this refactor qcow2_get_bitmap_info_list, its current interface is rather weird. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz --- block/qcow2.h| 4 ++-- block/qcow2-bitmap.c | 27

[PATCH v2 05/13] block: drop extra error propagation for bdrv_set_backing_hd

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
bdrv_set_backing_hd now returns status, let's use it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia --- block.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index b4e36d6dd7..1cf825c349 100644 ---

[PATCH v2 13/13] block/qed: bdrv_qed_do_open: deal with errp

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
Set errp always on failure. Generic bdrv_open_driver supports driver functions which can return negative value and forget to set errp. That's a strange thing.. Let's improve bdrv_qed_do_open to not behave this way. This allows to simplify code in bdrv_qed_co_invalidate_cache(). Signed-off-by:

[PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache()

2020-09-17 Thread Vladimir Sementsov-Ogievskiy
qcow2_do_open correctly sets errp on each failure path. So, we can simplify code in qcow2_co_invalidate_cache() and drop explicit error propagation. We should use ERRP_GUARD() (accordingly to comment in include/qapi/error.h) together with error_append() call which we add to avoid problems with

Re: [PATCH v2 00/13] block: deal with errp: part I

2020-09-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200917195519.19589-1-vsement...@virtuozzo.com/ Hi, This series failed build test on FreeBSD host. Please find the details below. The full log is available at

Re: [PATCH 2/3] virtio-blk: undo destructive iov_discard_*() operations

2020-09-17 Thread Stefan Hajnoczi
On Wed, Sep 16, 2020 at 11:38:36PM +0800, Li Qiang wrote: > Stefan Hajnoczi 于2020年8月12日周三 下午6:51写道: > > @@ -644,7 +648,8 @@ static int virtio_blk_handle_request(VirtIOBlockReq > > *req, MultiReqBuffer *mrb) > > req->in = (void *)in_iov[in_num - 1].iov_base > >+ in_iov[in_num

[PATCH v2 2/3] virtio-blk: undo destructive iov_discard_*() operations

2020-09-17 Thread Stefan Hajnoczi
Fuzzing discovered that virtqueue_unmap_sg() is being called on modified req->in/out_sg iovecs. This means dma_memory_map() and dma_memory_unmap() calls do not have matching memory addresses. Fuzzing discovered that non-RAM addresses trigger a bug: void address_space_unmap(AddressSpace *as,

Re: [PATCH v2] qemu-img: Support bitmap --merge into backing image

2020-09-17 Thread Max Reitz
On 15.09.20 15:31, Eric Blake wrote: > On 9/15/20 3:57 AM, Max Reitz wrote: >> On 14.09.20 21:10, Eric Blake wrote: >>> If you have the chain 'base.qcow2 <- top.qcow2' and want to merge a >>> bitmap from top into base, qemu-img was failing with: >>> >>> qemu-img: Could not open 'top.qcow2': Could

Re: [PATCH v2] hw/ide: check null block before _cancel_dma_sync

2020-09-17 Thread Li Qiang
P J P 于2020年9月4日周五 上午2:34写道: > > From: Prasad J Pandit > > When cancelling an i/o operation via ide_cancel_dma_sync(), > a block pointer may be null. Add check to avoid null pointer > dereference. > > -> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Fide_nullptr1 >

Re: [PATCH 1/2] file-posix: Correctly read max_segments of SG nodes

2020-09-17 Thread Maxim Levitsky
On Thu, 2020-09-17 at 15:16 +0200, Max Reitz wrote: > On 12.08.20 00:51, Dmitry Fomichev wrote: > > If scsi-generic driver is in use, an SG node can be specified in > > the command line in place of a regular SCSI device. In this case, > > sg_get_max_segments() fails to open max_segments entry in

[PATCH v3 3/6] qemu: Block migration when transient disk option is enabled

2020-09-17 Thread Masayoshi Mizuma
From: Masayoshi Mizuma Block migration when transient disk option is enabled because migration requires some blockjobs. Signed-off-by: Masayoshi Mizuma --- src/qemu/qemu_migration.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_migration.c

[PATCH v3 0/6] qemu: implementation of transient disk option

2020-09-17 Thread Masayoshi Mizuma
This patchset tries to implement transient option for qcow2 and raw format disk. It gets user available to set to the domain xml file like as: Any changes which the Guest does to the disk is dropped when the Guest is shutdowned. There are some limitations

[PATCH v3 4/6] qemu: update the validation for transient disk option

2020-09-17 Thread Masayoshi Mizuma
From: Masayoshi Mizuma Update validation of transient disk option. The option for qemu is supported with under condistions. - qemu has blockdev feature - the type is file and the format is qcow2 and raw - writable disk Signed-off-by: Masayoshi Mizuma --- src/qemu/qemu_validate.c | 25

[PATCH v3 5/6] qemu: Introduce to handle transient disk

2020-09-17 Thread Masayoshi Mizuma
From: Masayoshi Mizuma Here is the implementation of transient option for qcow2 and raw format disk. This gets available directive in domain xml file like as: When the qemu command line options are built, a new qcow2 image is created with backing qcow2 by

Re: [PULL 0/8] Block odirect patches

2020-09-17 Thread Peter Maydell
On Wed, 16 Sep 2020 at 10:47, Daniel P. Berrangé wrote: > > The following changes since commit de39a045bd8d2b49e4f3d07976622c29d58e0bac: > > Merge remote-tracking branch 'remotes/kraxel/tags/vga-20200915-pull-request= > ' into staging (2020-09-15 14:25:05 +0100) > > are available in the Git

Re: [PATCH 1/2] file-posix: Correctly read max_segments of SG nodes

2020-09-17 Thread Max Reitz
On 12.08.20 00:51, Dmitry Fomichev wrote: > If scsi-generic driver is in use, an SG node can be specified in > the command line in place of a regular SCSI device. In this case, > sg_get_max_segments() fails to open max_segments entry in sysfs > because /dev/sgX is a character device. As the

Re: [PATCH 1/2] file-posix: Correctly read max_segments of SG nodes

2020-09-17 Thread Maxim Levitsky
On Thu, 2020-09-17 at 16:22 +0300, Maxim Levitsky wrote: > On Thu, 2020-09-17 at 15:16 +0200, Max Reitz wrote: > > On 12.08.20 00:51, Dmitry Fomichev wrote: > > > If scsi-generic driver is in use, an SG node can be specified in > > > the command line in place of a regular SCSI device. In this

[PATCH v3 1/6] qemu: Block blockjobs when transient disk option is enabled

2020-09-17 Thread Masayoshi Mizuma
From: Masayoshi Mizuma Signed-off-by: Masayoshi Mizuma --- src/qemu/qemu_domain.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 785cee6f18..1ce0f70971 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10794,6

[PATCH v3 2/6] qemu: Block disk hotplug when transient disk option is enabled

2020-09-17 Thread Masayoshi Mizuma
From: Masayoshi Mizuma Signed-off-by: Masayoshi Mizuma --- src/qemu/qemu_hotplug.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e2c6e14c2e..20dcec7b65 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@

[PATCH v3 6/6] qemu: Add transient disk handler to start and stop the guest

2020-09-17 Thread Masayoshi Mizuma
From: Masayoshi Mizuma The transient disk is attached before the guest starts. Remove the transient disk when the guest does shutdown. Signed-off-by: Masayoshi Mizuma --- src/qemu/qemu_process.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_process.c

Re: [PATCH v1 0/2] Add timeout mechanism to qmp actions

2020-09-17 Thread Stefan Hajnoczi
On Thu, Sep 17, 2020 at 03:36:57PM +0800, Zhenyu Ye wrote: > When the hang occurs, the QEMU is blocked at: > > #0 0x95762b64 in ?? () from target:/usr/lib64/libpthread.so.0 > #1 0x9575bd88 in pthread_mutex_lock () from > target:/usr/lib64/libpthread.so.0 > #2

Re: [PATCH v8 2/7] copy-on-read: add filter append/drop functions

2020-09-17 Thread Andrey Shinkevich
On 04.09.2020 14:22, Max Reitz wrote: On 28.08.20 18:52, Andrey Shinkevich wrote: Provide API for the COR-filter insertion/removal. ... Also, drop the filter child permissions for an inactive state when the filter node is being removed. Do we need .active for that? Shouldn’t it be

Re: [PATCH v1 0/2] Add timeout mechanism to qmp actions

2020-09-17 Thread Fam Zheng
On 2020-09-17 16:44, Stefan Hajnoczi wrote: > On Thu, Sep 17, 2020 at 03:36:57PM +0800, Zhenyu Ye wrote: > > When the hang occurs, the QEMU is blocked at: > > > > #0 0x95762b64 in ?? () from target:/usr/lib64/libpthread.so.0 > > #1 0x9575bd88 in pthread_mutex_lock ()

Re: [PATCH 13/14] block/qcow2: qcow2_do_open: deal with errp

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:29 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > 1. Drop extra error propagation > > 2. Set errp always on failure. Generic bdrv_open_driver supports driver > functions which can return negative value and forget to set errp. > That's a strange thing.. Let's improve

Re: [PATCH 14/14] block/qed: bdrv_qed_do_open: deal with errp

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:30 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > Set errp always on failure. Generic bdrv_open_driver supports driver > functions which can return negative value and forget to set errp. > That's a strange thing.. Let's improve bdrv_qed_do_open to not behave > this way.

Re: [PATCH 09/14] block/qcow2: qcow2_get_specific_info(): drop error propagation

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:25 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > + * On success return true with bm_list set (probably to NULL, if no bitmaps), " probably " ? :-) > + * on failure return false with errp set. > */ > -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState

Re: [PATCH 3/3] block/nvme: Align NVMeRegs structure to 4KiB and mark it packed

2020-09-17 Thread Stefan Hajnoczi
On Wed, Sep 16, 2020 at 10:40:04PM +0200, Philippe Mathieu-Daudé wrote: > In commit e5ff22ba9fc we changed the doorbells register > declaration but forgot to mark the structure packed (as > MMIO registers), allowing the compiler to optimize it. Does this patch actually change anything? NvmeBar is

Re: [PATCH 2/3] block/nvme: Use atomic operations instead of 'volatile' keyword

2020-09-17 Thread Stefan Hajnoczi
On Wed, Sep 16, 2020 at 10:40:03PM +0200, Philippe Mathieu-Daudé wrote: I think the current use of volatile is fine. It's widely used in device drivers (see Linux and DPDK) so I'm not sure eliminating it is worthwhile. > Follow docs/devel/atomics.rst guidelines and use atomic operations. > >

Re: [PATCH 2/3] block: add logging facility for long standing IO requests

2020-09-17 Thread Denis V. Lunev
On 9/17/20 4:56 PM, Max Reitz wrote: > On 10.08.20 12:14, Denis V. Lunev wrote: >> There are severe delays with IO requests processing if QEMU is running in >> virtual machine or over software defined storage. Such delays potentially >> results in unpredictable guest behavior. For example, guests

[PATCH v2 0/3] virtio: restore elem->in/out_sg after iov_discard_front/back()

2020-09-17 Thread Stefan Hajnoczi
v2: * Add missing undo in virtio-blk write zeroes error path [Li Qiang] Both virtio-blk and virtio-crypto use destructive iov_discard_front/back() operations on elem->in/out_sg. virtqueue_push() calls dma_memory_unmap() on the modified iovec arrays. The memory addresses may not match those

[PATCH v2 3/3] virtio-crypto: don't modify elem->in/out_sg

2020-09-17 Thread Stefan Hajnoczi
A number of iov_discard_front/back() operations are made by virtio-crypto. The elem->in/out_sg iovec arrays are modified by these operations, resulting virtqueue_unmap_sg() calls on different addresses than were originally mapped. This is problematic because dirty memory may not be logged

[PATCH v2 1/3] util/iov: add iov_discard_undo()

2020-09-17 Thread Stefan Hajnoczi
The iov_discard_front/back() operations are useful for parsing iovecs but they modify the array elements. If the original array is needed after parsing finishes there is currently no way to restore it. Although g_memdup() can be used before performing destructive iov_discard_front/back()

Re: [PATCH 1/3] block/nvme: Initialize constant values with const_le32()

2020-09-17 Thread Stefan Hajnoczi
On Wed, Sep 16, 2020 at 10:40:02PM +0200, Philippe Mathieu-Daudé wrote: > To avoid multiple endianess conversion, as we know the device > registers are in little-endian, directly use const_le32() with > constant values. Can cpu_to_X() be extended to handle constant expressions? That way the

Re: [PATCH v1 0/2] Add timeout mechanism to qmp actions

2020-09-17 Thread Fam Zheng
On 2020-09-17 15:36, Zhenyu Ye wrote: > Hi Stefan, > > On 2020/9/14 21:27, Stefan Hajnoczi wrote: > >> > >> Theoretically, everything running in an iothread is asynchronous. However, > >> some 'asynchronous' actions are not non-blocking entirely, such as > >> io_submit(). This will block while

Re: [PATCH 2/3] block: add logging facility for long standing IO requests

2020-09-17 Thread Max Reitz
On 10.08.20 12:14, Denis V. Lunev wrote: > There are severe delays with IO requests processing if QEMU is running in > virtual machine or over software defined storage. Such delays potentially > results in unpredictable guest behavior. For example, guests over IDE or > SATA drive could remount

Re: [PATCH 1/3] block/block-backend: add converter from BlockAcctStats to BlockBackend

2020-09-17 Thread Max Reitz
On 10.08.20 12:14, Denis V. Lunev wrote: > Right now BlockAcctStats is always reside on BlockBackend. This structure > is not used in any other place. Thus we are able to create a converter > from one pointer to another. > > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir

Re: [PATCH 01/14] block: return status from bdrv_append and friends

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:17 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > The recommended use of qemu error api assumes returning status together > with setting errp and avoid void functions with errp parameter. Let's > improve bdrv_append and some friends to reduce error-propagation > overhead

Re: [PATCH 03/14] block: check return value of bdrv_open_child and drop error propagation

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:19 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > This patch is generated by cocci script: > > @@ > symbol bdrv_open_child, errp, local_err; > expression file; > @@ > > file = bdrv_open_child(..., > -_err > +errp >

Re: [PATCH 12/14] block/qcow2: read_cache_sizes: return status value

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:28 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > It's better to return status together with setting errp. It allows to > reduce error propagation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 06/14] block/mirror: drop extra error propagation in commit_active_start()

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:22 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > Let's check return value of mirror_start_job to check for failure > instead of local_err. > > Rename ret to job, as ret is usually integer variable. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto

Re: [PATCH 1/3] block/nvme: Initialize constant values with const_le32()

2020-09-17 Thread Philippe Mathieu-Daudé
On 9/17/20 11:55 AM, Stefan Hajnoczi wrote: > On Wed, Sep 16, 2020 at 10:40:02PM +0200, Philippe Mathieu-Daudé wrote: >> To avoid multiple endianess conversion, as we know the device >> registers are in little-endian, directly use const_le32() with >> constant values. > > Can cpu_to_X() be

Re: [PATCH 05/14] block: drop extra error propagation for bdrv_set_backing_hd

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > bdrv_set_backing_hd now returns status, let's use it. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 04/14] blockdev: fix drive_backup_prepare() missed error

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:20 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > We leak local_err and don't report failure to the caller. It's > definitely wrong, let's fix. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 08/14] blockjob: return status from block_job_set_speed()

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:24 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > Better to return status together with setting errp. It allows to avoid > error propagation in the caller. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 02/14] block: use return status of bdrv_append()

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:18 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > Now bdrv_append returns status and we can drop all the local_err things > around it. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 3/3] block: enable long IO requests report by default

2020-09-17 Thread Max Reitz
On 10.08.20 12:14, Denis V. Lunev wrote: > Latency threshold is set to 10 seconds following guest request timeout > on legacy storage controller. > > Signed-off-by: Denis V. Lunev > CC: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Kevin Wolf > CC: Max Reitz > --- > blockdev.c |

Re: [PATCH v2 2/3] virtio-blk: undo destructive iov_discard_*() operations

2020-09-17 Thread Li Qiang
Stefan Hajnoczi 于2020年9月17日周四 下午5:47写道: > > Fuzzing discovered that virtqueue_unmap_sg() is being called on modified > req->in/out_sg iovecs. This means dma_memory_map() and > dma_memory_unmap() calls do not have matching memory addresses. > > Fuzzing discovered that non-RAM addresses trigger a

Re: [PATCH 11/14] block/qcow2-bitmap: return startus from qcow2_store_persistent_dirty_bitmaps

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:27 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > It's better to return status together with setting errp. It makes > possible to avoid error propagation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy [...] > -void qcow2_store_persistent_dirty_bitmaps(BlockDriverState

RE: [PATCH 1/2] file-posix: Correctly read max_segments of SG nodes

2020-09-17 Thread Dmitry Fomichev
> -Original Message- > From: Maxim Levitsky > Sent: Thursday, September 17, 2020 9:24 AM > To: Max Reitz ; Dmitry Fomichev > ; Kevin Wolf ; Paolo > Bonzini ; Fam Zheng ; Philippe > Mathieu-Daudé > Cc: Alistair Francis ; Damien Le Moal > ; qemu-block@nongnu.org; qemu- > de...@nongnu.org >

Re: [PATCH 10/14] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface

2020-09-17 Thread Alberto Garcia
On Wed 09 Sep 2020 08:59:26 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > -/* qcow2_load_dirty_bitmaps() > - * Return value is a hint for caller: true means that the Qcow2 header was > - * updated. (false doesn't mean that the header should be updated by the > - * caller, it just means that