Re: [PATCH v3 8/8] parallels: Replace qemu_co_mutex_lock by WITH_QEMU_LOCK_GUARD

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: Replace the way we use mutex in parallels_co_check() for simplier and less error prone code. Signed-off-by: Alexander Ivanov --- v2: Fix an incorrect usage of WITH_QEMU_LOCK_GUARD. v3: Fix commit message. block/parallels.c | 26 --

Re: [PATCH v3 7/8] parallels: Move statistic collection to a separate function

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: We will add more and more checks so we need a better code structure in parallels_co_check. Let each check performs in a separate loop in a separate helper. Signed-off-by: Alexander Ivanov Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vla

Re: [PATCH v3 6/8] parallels: Move check of leaks to a separate function

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: We will add more and more checks so we need a better code structure in parallels_co_check. Let each check performs in a separate loop in a separate helper. Signed-off-by: Alexander Ivanov --- v2: No change. v3: Fix commit message. block/parallels.c |

Re: [RFC v3 5/8] block: add BlockRAMRegistrar

2022-08-17 Thread Stefan Hajnoczi
On Thu, Jul 14, 2022 at 11:30:11AM +0200, Hanna Reitz wrote: > On 08.07.22 06:17, Stefan Hajnoczi wrote: > > Emulated devices and other BlockBackend users wishing to take advantage > > of blk_register_buf() all have the same repetitive job: register > > RAMBlocks with the BlockBackend using RAMBloc

Re: [RFC v3 4/8] block: add BDRV_REQ_REGISTERED_BUF request flag

2022-08-17 Thread Stefan Hajnoczi
On Thu, Jul 14, 2022 at 10:54:12AM +0200, Hanna Reitz wrote: > On 08.07.22 06:17, Stefan Hajnoczi wrote: > > Block drivers may optimize I/O requests accessing buffers previously > > registered with bdrv_register_buf(). Checking whether all elements of a > > request's QEMUIOVector are within previou

Re: [PATCH v3 5/8] parallels: Move check of cluster outside image to a separate function

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: We will add more and more checks so we need a better code structure in parallels_co_check. Let each check performs in a separate loop in a separate helper. Signed-off-by: Alexander Ivanov Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vl

Re: [PATCH v3 4/8] parallels: Move check of unclean image to a separate function

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: We will add more and more checks so we need a better code structure in parallels_co_check. Let each check performs in a separate loop in a separate helper. Signed-off-by: Alexander Ivanov Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vla

Re: [PATCH v3 3/8] parallels: Use generic infrastructure for BAT writing in parallels_co_check()

2022-08-17 Thread Denis V. Lunev
On 17.08.2022 21:48, Vladimir Sementsov-Ogievskiy wrote: On 8/15/22 12:02, Alexander Ivanov wrote: BAT is written in the context of conventional operations over the image inside bdrv_co_flush() when it calls parallels_co_flush_to_os() callback. Thus we should not modify BAT array directly, but c

[PULL 01/12] virtio-scsi: fix race in virtio_scsi_dataplane_start()

2022-08-17 Thread Michael S. Tsirkin
From: Stefan Hajnoczi As soon as virtio_scsi_data_plane_start() attaches host notifiers the IOThread may start virtqueue processing. There is a race between IOThread virtqueue processing and virtio_scsi_data_plane_start() because it only assigns s->dataplane_started after attaching host notifiers

Re: [PATCH v3 1/8] parallels: Out of image offset in BAT leads to image inflation

2022-08-17 Thread Denis V. Lunev
On 17.08.2022 21:43, Vladimir Sementsov-Ogievskiy wrote: On 8/17/22 22:27, Denis V. Lunev wrote: On 17.08.2022 21:13, Vladimir Sementsov-Ogievskiy wrote: On 8/15/22 12:02, Alexander Ivanov wrote: data_end field in BDRVParallelsState is set to the biggest offset present in BAT. If this offset i

Re: [PATCH v3 3/8] parallels: Use generic infrastructure for BAT writing in parallels_co_check()

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: BAT is written in the context of conventional operations over the image inside bdrv_co_flush() when it calls parallels_co_flush_to_os() callback. Thus we should not modify BAT array directly, but call parallels_set_bat_entry() helper and bdrv_co_flush() f

Re: [PATCH v3 1/8] parallels: Out of image offset in BAT leads to image inflation

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/17/22 22:27, Denis V. Lunev wrote: On 17.08.2022 21:13, Vladimir Sementsov-Ogievskiy wrote: On 8/15/22 12:02, Alexander Ivanov wrote: data_end field in BDRVParallelsState is set to the biggest offset present in BAT. If this offset is outside of the image, any further write will create the

Re: [PATCH v3 1/8] parallels: Out of image offset in BAT leads to image inflation

2022-08-17 Thread Denis V. Lunev
On 17.08.2022 21:13, Vladimir Sementsov-Ogievskiy wrote: On 8/15/22 12:02, Alexander Ivanov wrote: data_end field in BDRVParallelsState is set to the biggest offset present in BAT. If this offset is outside of the image, any further write will create the cluster at this offset and/or the image

Re: [PATCH v3 2/8] parallels: create parallels_set_bat_entry_helper() to assign BAT value

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: This helper will be reused in next patches during parallels_co_check rework to simplify its code. Signed-off-by: Alexander Ivanov Reviewed-by: Vladimir Sementsov-Ogievskiy --- v2: A new patch - a part of a splitted patch. v3: Fix commit message.

Re: [PATCH v3 1/8] parallels: Out of image offset in BAT leads to image inflation

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/15/22 12:02, Alexander Ivanov wrote: data_end field in BDRVParallelsState is set to the biggest offset present in BAT. If this offset is outside of the image, any further write will create the cluster at this offset and/or the image will be truncated to this offset on close. This is definite

Re: [PATCH v10 18/21] job.c: enable job lock/unlock and remove Aiocontext locks

2022-08-17 Thread Vladimir Sementsov-Ogievskiy
On 8/16/22 15:52, Emanuele Giuseppe Esposito wrote:   } @@ -501,8 +481,12 @@ void job_unref_locked(Job *job)> assert(!job->txn);     if (job->driver->free) { +    AioContext *aio_context = job->aio_context;   job_unlock(); +    /* FIXME: aiocontext lock is r

[PULL 01/10] virtio-scsi: fix race in virtio_scsi_dataplane_start()

2022-08-17 Thread Michael S. Tsirkin
From: Stefan Hajnoczi As soon as virtio_scsi_data_plane_start() attaches host notifiers the IOThread may start virtqueue processing. There is a race between IOThread virtqueue processing and virtio_scsi_data_plane_start() because it only assigns s->dataplane_started after attaching host notifiers

[PATCH 09/10] iotests: Refactor tests of parallels images checks (131)

2022-08-17 Thread Alexander Ivanov
Replace hardcoded numbers by variables. Signed-off-by: Alexander Ivanov --- tests/qemu-iotests/131 | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/qemu-iotests/131 b/tests/qemu-iotests/131 index a847692b4c..601546c84c 100755 --- a/tests/q

[PATCH 08/10] iotests: Add test for BAT entries duplication check

2022-08-17 Thread Alexander Ivanov
Fill the image with a pattern and write another pattern in the second cluster. Corrupt the image and check if the pattern changes. Repair the image and check the patterns on guest and host sides. Signed-off-by: Alexander Ivanov --- tests/qemu-iotests/tests/parallels-checks | 31 +

[PATCH 07/10] iotests: Add leak check test for parallels format

2022-08-17 Thread Alexander Ivanov
Write a pattern to the last cluster, extend the image by 1 claster, repair and check that the last cluster still has the same pattern. Signed-off-by: Alexander Ivanov --- tests/qemu-iotests/tests/parallels-checks | 27 +++ tests/qemu-iotests/tests/parallels-checks.out | 22 ++

[PATCH 03/10] parallels: Create parallels_handle_leak() to truncate excess clusters

2022-08-17 Thread Alexander Ivanov
This helper will be reused in the next patch for duplications check. Signed-off-by: Alexander Ivanov --- block/parallels.c | 83 ++- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index ce04a4da71.

[PATCH 02/10] parallels: Incorrect data end calculation in parallels_open

2022-08-17 Thread Alexander Ivanov
The BDRVParallelsState structure contains data_end field that is measured in sectors. In parallels_open() initially this field is set by data_off field from parallels image header. According to the parallels format documentation, data_off field contains an offset, in sectors, from the start of the

[PATCH 06/10] iotests: Add out-of-image check test for parallels format

2022-08-17 Thread Alexander Ivanov
Fill the image with a pattern to generate entries in the BAT, set the first BAT entry outside the image, try to read the corrupted image, repair and check for zeroes in the first cluster. Signed-off-by: Alexander Ivanov --- tests/qemu-iotests/tests/parallels-checks | 78 +++

[PATCH 10/10] iotests: Fix cluster size in parallels images tests (131)

2022-08-17 Thread Alexander Ivanov
In this test cluster size is 64k, but modern tools generate images with cluster size 1M. Calculate cluster size using track field from image header. Signed-off-by: Alexander Ivanov --- tests/qemu-iotests/131 | 5 - tests/qemu-iotests/131.out | 44 +++---

[PATCH 04/10] parallels: Add checking and repairing duplicate offsets in BAT

2022-08-17 Thread Alexander Ivanov
Cluster offsets must be unique among all BAT entries. Find duplicate offsets in the BAT. If a duplicated offset is found fix it by copying the content of the relevant cluster to a new allocated cluster and set the new cluster offset to the duplicated entry. Add host_cluster_index() helper to dedu

[PATCH 05/10] parallels: Use highest_offset() helper in leak check

2022-08-17 Thread Alexander Ivanov
Deduplicate code by using highest_offset() helper. Signed-off-by: Alexander Ivanov --- block/parallels.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index bd129f44fa..93d21804f2 100644 --- a/block/parallels.c +++ b/block/paral

[PATCH 01/10] parallels: Incorrect condition in out-of-image check

2022-08-17 Thread Alexander Ivanov
All the offsets in the BAT must be at least one cluster away from the end of the data area. Fix the check condition for correct check. Signed-off-by: Alexander Ivanov --- block/parallels.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/parallels.c b/block/parallels.c i

[PATCH 00/10] parallels: Add duplication check, refactor, fix bugs

2022-08-17 Thread Alexander Ivanov
This patchset depends on the patchset [PATCH v3 0/8] parallels: Refactor the code of images checks and fix a bug Fix an incorrect condition in out-of-image check and incorrect data end calculation in parallels_open(). Add parallels_handle_leak() and highest_offset() helpers. Add checking and rep

Re: [PATCH v10 14/21] jobs: protect job.aio_context with BQL and job_mutex

2022-08-17 Thread Emanuele Giuseppe Esposito
Am 17/08/2022 um 10:04 schrieb Emanuele Giuseppe Esposito: >>> +/* protect against read in job_do_yield_locked */ >>> +JOB_LOCK_GUARD(); >>> +/* ensure the coroutine is quiescent while the AioContext is changed */ >>> +assert(job->pause_count > 0); >> job->pause_count only shows

Re: [PATCH v10 18/21] job.c: enable job lock/unlock and remove Aiocontext locks

2022-08-17 Thread Emanuele Giuseppe Esposito
Am 05/08/2022 um 15:01 schrieb Kevin Wolf: > Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben: >> Change the job_{lock/unlock} and macros to use job_mutex. >> >> Now that they are not nop anymore, remove the aiocontext >> to avoid deadlocks. > > Okay, so this is the big bad pat

Re: [PATCH v10 13/21] job: detect change of aiocontext within job coroutine

2022-08-17 Thread Emanuele Giuseppe Esposito
Am 17/08/2022 um 10:34 schrieb Kevin Wolf: > Am 16.08.2022 um 17:09 hat Emanuele Giuseppe Esposito geschrieben: >> >> >> Am 05/08/2022 um 10:37 schrieb Kevin Wolf: >>> Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben: From: Paolo Bonzini We want to make sure acce

Re: [RFC] hw/block/m25p80: implement Octal SPI commands

2022-08-17 Thread Francisco Iglesias
Hi Anton, On [2022 Aug 13] Sat 12:00:03, Anton Kochkov wrote: > * Implement Octal SPI commands based on Micron MT35X series > * Fix Micron 0x2C-based ID handling (incompatible with Numonyx) > * Fix Micron configuration registers handling Would it be ok for you to split the patch up into 3 patches

Re: [PATCH v10 11/21] jobs: group together API calls under the same job lock

2022-08-17 Thread Kevin Wolf
Am 17.08.2022 um 11:35 hat Emanuele Giuseppe Esposito geschrieben: > > > Am 17/08/2022 um 10:46 schrieb Kevin Wolf: > @@ -475,13 +477,15 @@ void *block_job_create(const char *job_id, const > BlockJobDriver *driver, > job->ready_notifier.notify = block_job_event_ready; >

Re: [PATCH v10 11/21] jobs: group together API calls under the same job lock

2022-08-17 Thread Emanuele Giuseppe Esposito
Am 17/08/2022 um 10:46 schrieb Kevin Wolf: @@ -475,13 +477,15 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, job->ready_notifier.notify = block_job_event_ready; job->idle_notifier.notify = block_job_on_idle; -notifier_li

[PATCH 2/2] block: add missed block_acct_setup with new block device init procedure

2022-08-17 Thread Denis V. Lunev
Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from the first glance, but it has changed things a lot. 'libvirt' uses it to detect that it should follow new initialization way and this changes things considerably. With this procedure followed, blockdev_init() is not called anym

[PATCH v2] block: add missed block_acct_setup with new block device init procedure

2022-08-17 Thread Denis V. Lunev
Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from the first glance, but it has changed things a lot. 'libvirt' uses it to detect that it should follow new initialization way and this changes things considerably. With this procedure followed, blockdev_init() is not called anym

[PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()

2022-08-17 Thread Denis V. Lunev
We would have one more place for block_acct_setup() calling, which should not corrupt original value. Signed-off-by: Denis V. Lunev CC: Peter Krempa CC: Markus Armbruster CC: John Snow CC: Kevin Wolf CC: Hanna Reitz CC: Vladimir Sementsov-Ogievskiy --- block/accounting.c | 24 +

Re: [PATCH v10 11/21] jobs: group together API calls under the same job lock

2022-08-17 Thread Kevin Wolf
Am 16.08.2022 um 16:54 hat Emanuele Giuseppe Esposito geschrieben: > Am 04/08/2022 um 19:10 schrieb Kevin Wolf: > > Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben: > >> Now that the API offers also _locked() functions, take advantage > >> of it and give also the caller control to

[PATCH 1/2] block: use bdrv_is_sg() helper instead of raw bs->sg reading

2022-08-17 Thread Denis V. Lunev
I believe that if the helper exists, it must be used always for reading of the value. It breaks expectations in the other case. Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Hanna Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Ronnie Sahlberg CC: Paolo Bonzini CC: Peter Lieven CC: Vladimir

[PATCH 2/2] block: make serializing requests functions 'void'

2022-08-17 Thread Denis V. Lunev
Return codes of the following functions are never used in the code: * bdrv_wait_serialising_requests_locked * bdrv_wait_serialising_requests * bdrv_make_request_serialising Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Hanna Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Ronnie Sahlberg CC:

[PATCH for 7.2] minor fixups in block code

2022-08-17 Thread Denis V. Lunev
These 2 patches are just minor improvements to make code a bit better. Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Hanna Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Ronnie Sahlberg CC: Paolo Bonzini CC: Peter Lieven CC: Vladimir Sementsov-Ogievskiy

Re: [PATCH v10 13/21] job: detect change of aiocontext within job coroutine

2022-08-17 Thread Kevin Wolf
Am 16.08.2022 um 17:09 hat Emanuele Giuseppe Esposito geschrieben: > > > Am 05/08/2022 um 10:37 schrieb Kevin Wolf: > > Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben: > >> From: Paolo Bonzini > >> > >> We want to make sure access of job->aio_context is always done > >> under

Re: [PATCH v10 14/21] jobs: protect job.aio_context with BQL and job_mutex

2022-08-17 Thread Emanuele Giuseppe Esposito
Am 05/08/2022 um 11:12 schrieb Kevin Wolf: > Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben: >> In order to make it thread safe, implement a "fake rwlock", >> where we allow reads under BQL *or* job_mutex held, but >> writes only under BQL *and* job_mutex. > > Oh, so the "or