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
>
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
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
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
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
>
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
>
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
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
>
> -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
> -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 ;
> -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
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
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
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,
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
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
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
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,
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
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
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
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
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
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)
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 ++-
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
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
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
---
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:
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
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
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
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,
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
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
>
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
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
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
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
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
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
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
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
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
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
@@
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
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
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
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 ()
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
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.
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
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
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.
>
>
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
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
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
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()
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
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
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
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
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
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
>
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
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
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
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
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
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
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
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 |
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
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
> -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
>
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
76 matches
Mail list logo