Re: [PATCH v2 3/3] qapi: deprecate drive-backup

2021-05-14 Thread John Snow
On 5/6/21 5:57 AM, Kashyap Chamarthy wrote: TODO: We also need to deprecate drive-backup transaction action.. But union members in QAPI doesn't support 'deprecated' feature. I tried to dig a bit, but failed :/ Markus, could you please help with it? At least by advice? Oho, I see. OK, I'm not

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
we want to get from shres here, after possible call to block_copy_task_shrink(), as task->bytes may be reduced. Ah right, I missed that. So I guess if we want the caller to protect co-shared-resource, get_from_shres stays where it is, and put_ instead can still go into task_end (with a

Re: [PATCH v3 06/33] util/async: aio_co_schedule(): support reschedule in same ctx

2021-05-14 Thread Paolo Bonzini
On 14/05/21 19:27, Roman Kagan wrote: AFAICS your patch has basically the same effect as Vladimir's patch "util/async: aio_co_enter(): do aio_co_schedule in general case" (https://lore.kernel.org/qemu-devel/20210408140827.332915-4-vsement...@virtuozzo.com/). That one was found to break e.g.

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Vladimir Sementsov-Ogievskiy via
14.05.2021 20:28, Emanuele Giuseppe Esposito wrote: On 14/05/2021 17:30, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:32, Emanuele Giuseppe Esposito wrote: On 14/05/2021 16:26, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021

Re: [PATCH v2 2/3] docs/interop/bitmaps: use blockdev-backup

2021-05-14 Thread John Snow
On 5/5/21 9:58 AM, Vladimir Sementsov-Ogievskiy wrote: We are going to deprecate drive-backup, so use modern interface here. In examples where target image creation is shown, show blockdev-add as well. If target creation omitted, omit blockdev-add as well. Signed-off-by: Vladimir

Re: [PATCH v2 1/3] docs/block-replication: use blockdev-backup

2021-05-14 Thread John Snow
On 5/5/21 9:58 AM, Vladimir Sementsov-Ogievskiy wrote: We are going to deprecate drive-backup, so don't mention it here. Moreover, blockdev-backup seems more correct in the context. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow --- docs/block-replication.txt | 4

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-05-14 Thread John Snow
On 5/14/21 4:16 AM, Emanuele Giuseppe Esposito wrote: On 13/05/2021 20:47, John Snow wrote: On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. Signed-off-by: Emanuele Giuseppe

Re: [PATCH] fdc: check drive block device before usage (CVE-2021-20196)

2021-05-14 Thread John Snow
On 5/14/21 3:23 PM, Thomas Huth wrote: On 23/01/2021 11.03, P J P wrote: From: Prasad J Pandit While processing ioport command in 'fdctrl_write_dor', device controller may select a drive which is not initialised with a block device. This may result in a NULL pointer dereference. Add checks to

Re: [PATCH] fdc: check drive block device before usage (CVE-2021-20196)

2021-05-14 Thread Thomas Huth
On 23/01/2021 11.03, P J P wrote: From: Prasad J Pandit While processing ioport command in 'fdctrl_write_dor', device controller may select a drive which is not initialised with a block device. This may result in a NULL pointer dereference. Add checks to avoid it. Fixes: CVE-2021-20196

Re: [PATCH 07/10] iotests: use subprocess.run where possible

2021-05-14 Thread John Snow
On 5/12/21 5:46 PM, John Snow wrote: pylint 2.8.x adds warnings whenever we use Popen calls without using 'with', so it's desirable to convert synchronous calls to run() invocations where applicable. (Though, this trades one pylint warning for another due to a pylint bug, which I've silenced

Re: [PATCH 03/10] python/machine: use subprocess.run instead of subprocess.Popen

2021-05-14 Thread John Snow
On 5/14/21 10:08 AM, Wainer dos Santos Moschetta wrote: Now it might throw a CalledProcessError given that `check=True`. Shouldn't it capture the exception and (possible) re-throw as an QEMUMachineError? I lied to you again. The existing callers all check for failure explicitly, so in the

Re: [PATCH 05/10] python/machine: Disable pylint warning for open() in _pre_launch

2021-05-14 Thread John Snow
On 5/14/21 10:42 AM, Wainer dos Santos Moschetta wrote: Hi, On 5/12/21 6:46 PM, John Snow wrote: Shift the open() call later so that the pylint pragma applies *only* to that one open() call. Add a note that suggests why this is safe: the resource is unconditionally cleaned up in

Re: [PATCH 03/10] python/machine: use subprocess.run instead of subprocess.Popen

2021-05-14 Thread John Snow
On 5/14/21 10:08 AM, Wainer dos Santos Moschetta wrote: Hi, On 5/12/21 6:46 PM, John Snow wrote: use run() instead of Popen() -- to assert to pylint that we are not forgetting to close a long-running program. Signed-off-by: John Snow ---   python/qemu/machine.py | 15 +--   1 file

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Paolo Bonzini
Il ven 14 mag 2021, 16:10 Emanuele Giuseppe Esposito ha scritto: > > I'm not sure I like it since callers may still need coarser grained > > locks to protect their own state or synchronize access to multiple > > items of data. Also, some callers may not need thread-safety. > > > > Can the caller

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
On 14/05/2021 17:30, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:32, Emanuele Giuseppe Esposito wrote: On 14/05/2021 16:26, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at

[PULL 17/19] test-write-threshold: drop extra tests

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy Testing set/get of one 64bit variable doesn't seem necessary. We have a lot of such variables. Also remaining tests do test set/get anyway. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id:

[PULL 16/19] block/write-threshold: drop extra APIs

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy bdrv_write_threshold_exceeded() is unused. bdrv_write_threshold_is_set() is used only to double check the value of bs->write_threshold_offset in tests. No real sense in it (both tests do check real value with help of bdrv_write_threshold_get()) Signed-off-by:

[PULL 15/19] test-write-threshold: rewrite test_threshold_(not_)trigger tests

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy These tests use bdrv_write_threshold_exceeded() API, which is used only for test (since pre-previous commit). Better is testing real API, which is used in block.c as well. So, let's call bdrv_write_threshold_check_write(), and check is

[PULL 09/19] qemu-iotests: fix case of SOCK_DIR already in the environment

2021-05-14 Thread Max Reitz
From: Paolo Bonzini Due to a typo, in this case the SOCK_DIR was not being created. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-6-pbonz...@redhat.com> Message-Id:

Re: [PATCH v3 06/33] util/async: aio_co_schedule(): support reschedule in same ctx

2021-05-14 Thread Roman Kagan
On Thu, May 13, 2021 at 11:04:37PM +0200, Paolo Bonzini wrote: > On 12/05/21 09:15, Vladimir Sementsov-Ogievskiy wrote: > > > > > > > > > > I don't understand.  Why doesn't aio_co_enter go through the ctx != > > > qemu_get_current_aio_context() branch and just do aio_co_schedule? > > > That was

[PULL 14/19] block: drop write notifiers

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy They are unused now. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-3-vsement...@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- include/block/block_int.h | 12

[PULL 12/19] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-14 Thread Max Reitz
From: Emanuele Giuseppe Esposito pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. Modify all subprocess.Popen call to use the 'with' statement, except the one in __init__ of QemuIoInteractive class, since it is assigned to a class

[PULL 05/14] block/export: improve vu_blk_sect_range_ok()

2021-05-14 Thread Kevin Wolf
From: Stefan Hajnoczi The checks in vu_blk_sect_range_ok() assume VIRTIO_BLK_SECTOR_SIZE is equal to BDRV_SECTOR_SIZE. This is true, but let's add a QEMU_BUILD_BUG_ON() to make it explicit. We might as well check that the request buffer size is a multiple of VIRTIO_BLK_SECTOR_SIZE while we're

[PULL 10/19] Document qemu-img options data_file and data_file_raw

2021-05-14 Thread Max Reitz
From: Connor Kuehl The contents of this patch were initially developed and posted by Han Han[1], however, it appears the original patch was not applied. Since then, the relevant documentation has been moved and adapted to a new format. I've taken most of the original wording and tweaked it

[PULL 03/19] monitor: hmp_qemu_io: acquire aio contex, fix crash

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy Max reported the following bug: $ ./qemu-img create -f raw src.img 1G $ ./qemu-img create -f raw dst.img 1G $ (echo ' {"execute":"qmp_capabilities"} {"execute":"blockdev-mirror", "arguments":{"job-id":"mirror", "device":"source",

[PULL 03/14] block: Fix Transaction leak in bdrv_reopen_multiple()

2021-05-14 Thread Kevin Wolf
Like other error paths, this one needs to call tran_finalize() and clean up the BlockReopenQueue, too. Fixes: CID 1452772 Fixes: 72373e40fbc7e4218061a8211384db362d3e7348 Signed-off-by: Kevin Wolf Message-Id: <20210503110555.24001-3-kw...@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy

[PULL 07/19] qemu-iotests: move command line and environment handling from TestRunner to TestEnv

2021-05-14 Thread Max Reitz
From: Paolo Bonzini In the next patch, "check" will learn how to execute a test script without going through TestRunner. To enable this, keep only the text output and subprocess handling in the TestRunner; move into TestEnv the logic to prepare for running a subprocess. Reviewed-by: Vladimir

[PULL 08/19] qemu-iotests: let "check" spawn an arbitrary test command

2021-05-14 Thread Max Reitz
From: Paolo Bonzini Right now there is no easy way for "check" to print a reproducer command. Because such a reproducer command line would be huge, we can instead teach check to start a command of our choice. This can be for example a Python unit test with arguments to only run a specific

[PULL 13/19] block/write-threshold: don't use write notifiers

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's better special-case write-threshold and drop write notifiers at all. (Actually, write-threshold is special-cased anyway, as the only user of

[PULL 04/19] mirror: stop cancelling in-flight requests on non-force cancel in READY

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy If mirror is READY than cancel operation is not discarding the whole result of the operation, but instead it's a documented way get a point-in-time snapshot of source disk. So, we should not cancel any requests if mirror is READ and force=false. Let's fix that

[PULL 01/19] iotests/231: Update expected deprecation message

2021-05-14 Thread Max Reitz
From: Connor Kuehl The deprecation message in the expected output has technically been wrong since the wrong version of a patch was applied to it. Because of this, the test fails. Correct the expected output so that it passes. Signed-off-by: Connor Kuehl Reviewed-by: Max Reitz Reviewed-by:

[PULL 05/19] qemu-iotests: do not buffer the test output

2021-05-14 Thread Max Reitz
From: Paolo Bonzini Instead of buffering the test output into a StringIO, patch it on the fly by wrapping sys.stdout's write method. This can be done unconditionally, even if using -d, which makes execute_unittest a bit simpler. Signed-off-by: Paolo Bonzini Reviewed-by: Vladimir

[PULL 02/19] block/rbd: Add an escape-aware strchr helper

2021-05-14 Thread Max Reitz
From: Connor Kuehl Sometimes the parser needs to further split a token it has collected from the token input stream. Right now, it does a cursory check to see if the relevant characters appear in the token to determine if it should break it down further. However, qemu_rbd_next_tok() will escape

[PULL 00/19] Block patches

2021-05-14 Thread Max Reitz
The following changes since commit 96662996eda78c48aa4e76d8615c7eb72d80: Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20210513a' into staging (2021-05-14 12:03:47 +0100) are available in the Git repository at: https://github.com/XanClic/qemu.git

[PULL 19/19] write-threshold: deal with includes

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy "qemu/typedefs.h" is enough for include/block/write-threshold.h header with forward declaration of BlockDriverState. Also drop extra includes from block/write-threshold.c and tests/unit/test-write-threshold.c Signed-off-by: Vladimir Sementsov-Ogievskiy

[PULL 18/19] test-write-threshold: drop extra TestStruct structure

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy We don't need this extra logic: it doesn't make code simpler. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-8-vsement...@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz ---

[PULL 11/19] block/copy-on-read: use bdrv_drop_filter() and drop s->active

2021-05-14 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy Now, after huge update of block graph permission update algorithm, we don't need this workaround with active state of the filter. Drop it and use new smart bdrv_drop_filter() function. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id:

[PULL 06/19] qemu-iotests: allow passing unittest.main arguments to the test scripts

2021-05-14 Thread Max Reitz
From: Paolo Bonzini Python test scripts that use unittest consist of multiple tests. unittest.main allows selecting which tests to run, but currently this is not possible because the iotests wrapper ignores sys.argv. unittest.main command line options also allow the user to pick the desired

Re: [PATCH v3 3/4] migrate-bitmaps-test: Fix pylint warnings

2021-05-14 Thread Vladimir Sementsov-Ogievskiy
14.05.2021 18:43, Max Reitz wrote: There are a couple of things pylint takes issue with: - The "time" import is unused - The import order (iotests should come last) - get_bitmap_hash() doesn't use @self and so should be a function - Semicolons at the end of some lines - Parentheses after "if" -

[PULL 08/14] vhost-user-blk-test: test discard/write zeroes invalid inputs

2021-05-14 Thread Kevin Wolf
From: Stefan Hajnoczi Exercise input validation code paths in block/export/vhost-user-blk-server.c. Signed-off-by: Stefan Hajnoczi Message-Id: <20210309094106.196911-5-stefa...@redhat.com> Signed-off-by: Kevin Wolf Message-Id: <20210322092327.150720-4-stefa...@redhat.com> Signed-off-by: Kevin

[PULL 14/14] vhost-user-blk: Check that num-queues is supported by backend

2021-05-14 Thread Kevin Wolf
Creating a device with a number of queues that isn't supported by the backend is pointless, the device won't work properly and the error messages are rather confusing. Just fail to create the device if num-queues is higher than what the backend supports. Since the relationship between num-queues

[PULL 13/14] virtio: Fail if iommu_platform is requested, but unsupported

2021-05-14 Thread Kevin Wolf
Commit 2943b53f6 (' virtio: force VIRTIO_F_IOMMU_PLATFORM') made sure that vhost can't just reject VIRTIO_F_IOMMU_PLATFORM when it was requested. However, just adding it back to the negotiated flags isn't right either because it promises support to the guest that the device actually doesn't

[PULL 04/14] qapi: spelling fix (addtional)

2021-05-14 Thread Kevin Wolf
From: Michael Tokarev Fixes: 3d0d3c30ae3a259bff176f85a3efa2d0816695af Signed-off-by: Michael Tokarev Message-Id: <20210508093315.393274-1-...@msgid.tls.msk.ru> Signed-off-by: Kevin Wolf --- qapi/qom.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qapi/qom.json

[PULL 10/14] vhost-user-blk: Don't reconnect during initialisation

2021-05-14 Thread Kevin Wolf
This is a partial revert of commits 77542d43149 and bc79c87bcde. Usually, an error during initialisation means that the configuration was wrong. Reconnecting won't make the error go away, but just turn the error condition into an endless loop. Avoid this and return errors again. Additionally,

[PULL 02/14] block: Fix Transaction leak in bdrv_root_attach_child()

2021-05-14 Thread Kevin Wolf
The error path needs to call tran_finalize(), too. Fixes: CID 1452773 Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1 Signed-off-by: Kevin Wolf Message-Id: <20210503110555.24001-2-kw...@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf ---

[PULL 09/14] vhost-user-blk: Make sure to set Error on realize failure

2021-05-14 Thread Kevin Wolf
We have to set errp before jumping to virtio_err, otherwise the caller (virtio_device_realize()) will take this as success and crash when it later tries to access things that we've already freed in the error path. Fixes: 77542d431491788d1e8e79d93ce10172ef207775 Signed-off-by: Kevin Wolf

[PULL 11/14] vhost-user-blk: Improve error reporting in realize

2021-05-14 Thread Kevin Wolf
Now that vhost_user_blk_connect() is not called from an event handler any more, but directly from vhost_user_blk_device_realize(), we can actually make use of Error again instead of calling error_report() in the inner function and setting a more generic and therefore less useful error message in

[PULL 12/14] vhost-user-blk: Get more feature flags from vhost device

2021-05-14 Thread Kevin Wolf
VIRTIO_F_RING_PACKED and VIRTIO_F_IOMMU_PLATFORM need to be supported by the vhost device, otherwise advertising it to the guest doesn't result in a working configuration. They are currently not supported by the vhost-user-blk export in QEMU. Fixes:

[PULL 06/14] test: new qTest case to test the vhost-user-blk-server

2021-05-14 Thread Kevin Wolf
From: Coiby Xu This test case has the same tests as tests/virtio-blk-test.c except for tests have block_resize. Since the vhost-user-blk export only serves one client one time, two exports are started by qemu-storage-daemon for the hotplug test. Suggested-by: Thomas Huth Signed-off-by: Coiby

[PULL 07/14] tests/qtest: add multi-queue test case to vhost-user-blk-test

2021-05-14 Thread Kevin Wolf
From: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi Message-Id: <20210309094106.196911-4-stefa...@redhat.com> Signed-off-by: Kevin Wolf Message-Id: <20210322092327.150720-3-stefa...@redhat.com> Signed-off-by: Kevin Wolf --- tests/qtest/vhost-user-blk-test.c | 81

[PULL 09/16] virtio-blk: Configure all host notifiers in a single MR transaction

2021-05-14 Thread Michael S. Tsirkin
From: Greg Kurz This allows the virtio-blk-pci device to batch the setup of all its host notifiers. This significantly improves boot time of VMs with a high number of vCPUs, e.g. from 3m26.186s down to 0m58.023s for a pseries machine with 384 vCPUs. Note that memory_region_transaction_commit()

[PULL 08/16] virtio-blk: Fix rollback path in virtio_blk_data_plane_start()

2021-05-14 Thread Michael S. Tsirkin
From: Greg Kurz When dataplane multiqueue support was added in QEMU 2.7, the path that would rollback guest notifiers assignment in case of error simply got dropped. Later on, when Error was added to blk_set_aio_context() in QEMU 4.1, another error path was introduced, but it ommits to rollback

[PULL 00/14] Block layer patches

2021-05-14 Thread Kevin Wolf
The following changes since commit 96662996eda78c48aa4e76d8615c7eb72d80: Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20210513a' into staging (2021-05-14 12:03:47 +0100) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for

[PULL 01/14] qcow2: set bdi->is_dirty

2021-05-14 Thread Kevin Wolf
From: Vladimir Sementsov-Ogievskiy Set bdi->is_dirty, so that qemu-img info could show dirty flag. After this commit the following check will show '"dirty-flag": true': ./build/qemu-img create -f qcow2 -o lazy_refcounts=on x 1M ./build/qemu-io x qemu-io> write 0 1M After "write" command

Re: [PATCH v2 0/6] vhost-user-blk: Error handling fixes during initialistion

2021-05-14 Thread Kevin Wolf
Am 14.05.2021 um 14:20 hat Michael S. Tsirkin geschrieben: > On Thu, Apr 29, 2021 at 07:13:10PM +0200, Kevin Wolf wrote: > > vhost-user-blk neglects for several properties to check whether the > > configured value is even compatible with the backend. This results > > sometimes in crashes because

Re: [ANNOUNCE] libblkio v0.1.0 preview release

2021-05-14 Thread Kevin Wolf
Am 13.05.2021 um 11:47 hat Stefan Hajnoczi geschrieben: > On Thu, May 06, 2021 at 12:33:24PM +0200, Kevin Wolf wrote: > > Am 06.05.2021 um 10:46 hat Stefan Hajnoczi geschrieben: > > > What do you think about this: > > > > > > The blkio instance states are: > > > > > > created -> attached ->

[PULL 04/16] virtio-blk: Constify VirtIOFeature feature_sizes[]

2021-05-14 Thread Michael S. Tsirkin
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210511104157.2880306-3-phi...@redhat.com> --- hw/block/virtio-blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index d28979efb8..f139cd7cc9

[PATCH v3 0/4] iotests/297: Cover tests/

2021-05-14 Thread Max Reitz
v1: https://lists.nongnu.org/archive/html/qemu-block/2021-03/msg01471.html v2: https://lists.nongnu.org/archive/html/qemu-block/2021-05/msg00492.html Hi, When reviewing Vladimir’s new addition to tests/, I noticed that 297 so far does not cover named tests. That isn’t so good. This series

[PATCH v3 4/4] iotests/297: Cover tests/

2021-05-14 Thread Max Reitz
297 so far does not check the named tests, which reside in the tests/ directory (i.e. full path tests/qemu-iotests/tests). Fix it. Thanks to the previous two commits, all named tests pass its scrutiny, so we do not have to add anything to SKIP_FILES. Signed-off-by: Max Reitz Reviewed-by:

[PATCH v3 3/4] migrate-bitmaps-test: Fix pylint warnings

2021-05-14 Thread Max Reitz
There are a couple of things pylint takes issue with: - The "time" import is unused - The import order (iotests should come last) - get_bitmap_hash() doesn't use @self and so should be a function - Semicolons at the end of some lines - Parentheses after "if" - Some lines are too long (80

Re: [PATCH v4 0/6] Allow changing bs->file on reopen

2021-05-14 Thread Vladimir Sementsov-Ogievskiy
Hi Alberto! What are your plans for v5? I'm now finishing a new series which makes backup-top filter public, and I want to base it on your series (otherwise I can't add a test). 17.03.2021 20:15, Alberto Garcia wrote: Based-on: <20210317143529.615584-1-vsement...@virtuozzo.com> Hello, this

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Vladimir Sementsov-Ogievskiy via
14.05.2021 17:32, Emanuele Giuseppe Esposito wrote: On 14/05/2021 16:26, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at 10:59:40AM +0200, Emanuele Giuseppe Esposito wrote:

[PATCH v3 1/4] iotests/297: Drop 169 and 199 from the skip list

2021-05-14 Thread Max Reitz
169 and 199 have been renamed and moved to tests/ (commit a44be0334be: "iotests: rename and move 169 and 199 tests"), so we can drop them from the skip list. Signed-off-by: Max Reitz Reviewed-by: Willian Rampazzo Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf ---

[PATCH v3 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings

2021-05-14 Thread Max Reitz
pylint complains that discards1_sha256 and all_discards_sha256 are first set in non-__init__ methods. These variables are not really class-variables anyway, so let them instead be returned by start_postcopy(), thus silencing pylint. Suggested-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Max

Re: [PATCH v2 0/4] iotests/297: Cover tests/

2021-05-14 Thread Max Reitz
On 14.05.21 13:02, Max Reitz wrote: On 12.05.21 19:43, Max Reitz wrote: v1: https://lists.nongnu.org/archive/html/qemu-block/2021-03/msg01471.html Hi, When reviewing Vladimir’s new addition to tests/, I noticed that 297 so far does not cover named tests.  That isn’t so good. This series

Re: [PATCH 05/10] python/machine: Disable pylint warning for open() in _pre_launch

2021-05-14 Thread Wainer dos Santos Moschetta
Hi, On 5/12/21 6:46 PM, John Snow wrote: Shift the open() call later so that the pylint pragma applies *only* to that one open() call. Add a note that suggests why this is safe: the resource is unconditionally cleaned up in _post_shutdown(). You can also put it in a pylint disable/enable

Re: [PATCH v2] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-14 Thread Emanuele Giuseppe Esposito
On 14/05/2021 16:09, Max Reitz wrote: On 12.05.21 19:04, Max Reitz wrote: On 10.05.21 21:04, Emanuele Giuseppe Esposito wrote: pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. Modify all subprocess.Popen call to use the 'with'

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
On 14/05/2021 16:26, Vladimir Sementsov-Ogievskiy wrote: 14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at 10:59:40AM +0200, Emanuele Giuseppe Esposito wrote: co-shared-resource is currently not thread-safe, as also

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Vladimir Sementsov-Ogievskiy via
14.05.2021 17:10, Emanuele Giuseppe Esposito wrote: On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at 10:59:40AM +0200, Emanuele Giuseppe Esposito wrote: co-shared-resource is currently not thread-safe, as also reported in co-shared-resource.h. Add a QemuMutex because

Re: [PATCH 5/6] co-shared-resource: protect with a mutex

2021-05-14 Thread Emanuele Giuseppe Esposito
On 12/05/2021 17:44, Stefan Hajnoczi wrote: On Mon, May 10, 2021 at 10:59:40AM +0200, Emanuele Giuseppe Esposito wrote: co-shared-resource is currently not thread-safe, as also reported in co-shared-resource.h. Add a QemuMutex because co_try_get_from_shres can also be invoked from

Re: [PATCH v2] qemu-iotests: fix pylint 2.8 consider-using-with error

2021-05-14 Thread Max Reitz
On 12.05.21 19:04, Max Reitz wrote: On 10.05.21 21:04, Emanuele Giuseppe Esposito wrote: pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. Modify all subprocess.Popen call to use the 'with' statement, except the one in __init__ of

Re: [PATCH 03/10] python/machine: use subprocess.run instead of subprocess.Popen

2021-05-14 Thread Wainer dos Santos Moschetta
Hi, On 5/12/21 6:46 PM, John Snow wrote: use run() instead of Popen() -- to assert to pylint that we are not forgetting to close a long-running program. Signed-off-by: John Snow --- python/qemu/machine.py | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git

Re: [PATCH v2 0/6] vhost-user-blk: Error handling fixes during initialistion

2021-05-14 Thread Michael S. Tsirkin
On Thu, Apr 29, 2021 at 07:13:10PM +0200, Kevin Wolf wrote: > vhost-user-blk neglects for several properties to check whether the > configured value is even compatible with the backend. This results > sometimes in crashes because of buggy error handling code, and sometimes > in devices that are

Re: [PATCH v2 0/4] iotests/297: Cover tests/

2021-05-14 Thread Max Reitz
On 12.05.21 19:43, Max Reitz wrote: v1: https://lists.nongnu.org/archive/html/qemu-block/2021-03/msg01471.html Hi, When reviewing Vladimir’s new addition to tests/, I noticed that 297 so far does not cover named tests. That isn’t so good. This series makes it cover them, and because tests/

Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

2021-05-14 Thread Emanuele Giuseppe Esposito
On 13/05/2021 20:47, John Snow wrote: On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. Signed-off-by: Emanuele Giuseppe Esposito ---   python/qemu/machine.py    | 2 +-  

Re: [PATCH v3 01/15] python: qemu: add timer parameter for qmp.accept socket

2021-05-14 Thread Emanuele Giuseppe Esposito
On 13/05/2021 19:54, John Snow wrote: On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: Add a new _qmp_timer field to the QEMUMachine class. The default timer is 15 sec, as per the default in the qmp accept() function. Fine enough for now. What's the exact need for this change,