Re: [PATCH] hw/block: m25p80: Fix fast read for SST flashes

2020-12-03 Thread Bin Meng
Hi Francisco, On Thu, Dec 3, 2020 at 4:38 PM Francisco Iglesias wrote: > > Hi Bin and Alistair, > > On [2020 Dec 02] Wed 11:40:11, Alistair Francis wrote: > > On Sun, Nov 29, 2020 at 6:55 PM Bin Meng wrote: > > > > > > From: Bin Meng > > > > > > SST flashes require a dummy byte after the

Re: [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-03 Thread Li Qiang
Philippe Mathieu-Daudé 于2020年12月3日周四 下午8:38写道: > > On 12/3/20 1:02 PM, Li Qiang wrote: > > Philippe Mathieu-Daudé 于2020年12月3日周四 下午7:37写道: > >> > >> Hi Li, > >> > >> On 12/3/20 12:21 PM, Li Qiang wrote: > >>> Philippe Mathieu-Daudé 于2020年12月2日周三 上午3:13写道: > > cdb_len can not be zero...

[PATCH 2/4] block/io: bdrv_refresh_limits(): use ERRP_GUARD

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
This simplifies following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/io.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/io.c b/block/io.c index ec5e152bb7..3e91074c9f 100644 --- a/block/io.c +++ b/block/io.c @@ -135,10 +135,10 @@ static void

[PATCH 3/4] block/io: bdrv_check_byte_request(): drop bdrv_is_inserted()

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
Move bdrv_is_inserted() calls into callers. We are going to make bdrv_check_byte_request() a clean thing. bdrv_is_inserted() is not about checking the request, it's about checking the bs. So, it should be separate. With this patch we probably change error path for some failure scenarios. But

[PATCH 4/4] block: introduce BDRV_MAX_LENGTH

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
We are going to modify block layer to work with 64bit requests. And first step is moving to int64_t type for both offset and bytes arguments in all block request related functions. It's mostly safe (when widening signed or unsigned int to int64_t), but switching from uint64_t is questionable.

[PATCH 1/4] block/file-posix: fix workaround in raw_do_pwrite_zeroes()

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
We should not set overlap_bytes: 1. Don't worry: it is calculated by bdrv_mark_request_serialising() and will be equal to or greater than bytes anyway. 2. If the request was already aligned up to some greater alignment, than we may break things: we reduce overlap_bytes, and further

[PATCH 0/4] block: prepare for 64bit

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
Hi all! This is a preparation series for v4 of "[PATCH v3 00/17] 64bit block-layer". The whole thing is in 04, and 01-03 are small preparations. Vladimir Sementsov-Ogievskiy (4): block/file-posix: fix workaround in raw_do_pwrite_zeroes() block/io: bdrv_refresh_limits(): use ERRP_GUARD

Re: [PATCH v12 14/14] block: apply COR-filter to block-stream jobs

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
02.12.2020 21:18, Andrey Shinkevich wrote: On 27.10.2020 21:24, Andrey Shinkevich wrote: On 27.10.2020 20:57, Vladimir Sementsov-Ogievskiy wrote: 27.10.2020 20:48, Andrey Shinkevich wrote: On 27.10.2020 19:13, Vladimir Sementsov-Ogievskiy wrote: 22.10.2020 21:13, Andrey Shinkevich wrote:

Re: [PATCH v13 09/10] stream: skip filters when writing backing file name to QCOW2 header

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
02.12.2020 21:31, Andrey Shinkevich wrote: Avoid writing a filter JSON file name and a filter format name to QCOW2 image when the backing file is being changed after the block stream job. It can occur due to a concurrent commit job on the same backing chain. A user is still able to assign the

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Eduardo Habkost
On Thu, Dec 03, 2020 at 07:10:37PM +0100, Paolo Bonzini wrote: > On 03/12/20 18:52, Eduardo Habkost wrote: > > On Thu, Dec 03, 2020 at 05:50:46PM +0100, Paolo Bonzini wrote: > > > On 03/12/20 16:15, Kevin Wolf wrote: > > > > I don't think this is an intermediate state like Eduardo wants to have. >

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Paolo Bonzini
On 03/12/20 18:52, Eduardo Habkost wrote: On Thu, Dec 03, 2020 at 05:50:46PM +0100, Paolo Bonzini wrote: On 03/12/20 16:15, Kevin Wolf wrote: I don't think this is an intermediate state like Eduardo wants to have. Creating the object, then setting properties, then realize [1] will fail after

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Paolo Bonzini
On 03/12/20 18:43, Kevin Wolf wrote: I think I'd want to do step 2 and 3 combined, because converting user-creatable objects to oc->configure means manually writing the configure function that will be generated from QAPI in step 3. Writing code just to throw it away isn't my favourite pastime.

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Eduardo Habkost
On Thu, Dec 03, 2020 at 05:50:46PM +0100, Paolo Bonzini wrote: > On 03/12/20 16:15, Kevin Wolf wrote: > > I don't think this is an intermediate state like Eduardo wants to have. > > Creating the object, then setting properties, then realize [1] will fail > > after your change. But keeping it

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Kevin Wolf
Am 03.12.2020 um 17:50 hat Paolo Bonzini geschrieben: > On 03/12/20 16:15, Kevin Wolf wrote: > > I don't think this is an intermediate state like Eduardo wants to have. > > Creating the object, then setting properties, then realize [1] will fail > > after your change. But keeping it working was

[PATCH 3/3] block: Fix deadlock in bdrv_co_yield_to_drain()

2020-12-03 Thread Kevin Wolf
If bdrv_co_yield_to_drain() is called for draining a block node that runs in a different AioContext, it keeps that AioContext locked while it yields and schedules a BH in the AioContext to do the actual drain. As long as executing the BH is the very next thing the event loop of the node's

[PATCH 1/3] block: Simplify qmp_block_resize() error paths

2020-12-03 Thread Kevin Wolf
The only thing that happens after the 'out:' label is blk_unref(blk). However, blk = NULL in all of the error cases, so instead of jumping to 'out:', we can just return directly. Cc: qemu-sta...@nongnu.org Signed-off-by: Kevin Wolf --- blockdev.c | 7 +++ 1 file changed, 3 insertions(+), 4

[PATCH 0/3] block: Fix block_resize deadlock with iothreads

2020-12-03 Thread Kevin Wolf
The conversion of qmp_block_resize() to coroutines exposed a preexisting locking bug in the drain implementation that can cause deadlocks. As it happens, fixing this bug reveals in turn that the locking in qmp_block_resize() itself is incomplete, too. Kevin Wolf (3): block: Simplify

[PATCH 2/3] block: Fix locking in qmp_block_resize()

2020-12-03 Thread Kevin Wolf
The drain functions assume that we hold the AioContext lock of the drained block node. Make sure to actually take the lock. Cc: qemu-sta...@nongnu.org Fixes: eb94b81a94bce112e6b206df846c1551aaf6cab6 Signed-off-by: Kevin Wolf --- blockdev.c | 5 - 1 file changed, 4 insertions(+), 1

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Paolo Bonzini
On 03/12/20 16:15, Kevin Wolf wrote: I don't think this is an intermediate state like Eduardo wants to have. Creating the object, then setting properties, then realize [1] will fail after your change. But keeping it working was the whole point of the exercise. With the sample code, you must

Re: [PATCH] qemu-nbd: Fix a memleak in nbd_client_thread()

2020-12-03 Thread Eric Blake
On 12/1/20 8:02 PM, Alex Chen wrote: > On 2020/12/2 4:15, Eric Blake wrote: >> While the patch looks correct, we have a lot of duplication. Simpler >> might be a solution with only one exit label altogether: >> > > Thanks for your review, I will modify the patch and send patch v2 according >

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Kevin Wolf
Am 03.12.2020 um 12:11 hat Paolo Bonzini geschrieben: > On 02/12/20 18:35, Kevin Wolf wrote: > > > Could we have an intermediate state that doesn't require any > > > duplication and thus have no separate code paths that could > > > diverge? > > > > The one requirement we have for an intermediate

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Eduardo Habkost
On Thu, Dec 03, 2020 at 07:46:29AM +0100, Gerd Hoffmann wrote: > Hi, > > > It would be much nicer to do the wrapper the other way round, i.e. > > setting properties before the device is realized would update a > > configuration struct and realize would then call .create() with that > > struct.

[PATCH v2] qemu-nbd: Fix a memleak in nbd_client_thread()

2020-12-03 Thread Alex Chen
When the qio_channel_socket_connect_sync() fails we should goto 'out_socket' label to free the 'sioc' instead of goto 'out' label. In addition, there's a lot of redundant code in the successful branch and the error branch, optimize it. Reported-by: Euler Robot Signed-off-by: Alex Chen

Re: [PATCH for-5.2 05/10] vhost-user-blk-test: close fork child file descriptors

2020-12-03 Thread Stefan Hajnoczi
On Tue, Nov 24, 2020 at 08:08:26PM +0800, Coiby Xu wrote: > Hi Stefan, > > On Wed, Nov 11, 2020 at 12:43:26PM +, Stefan Hajnoczi wrote: > > Do not leave stdin and stdout open after fork. stdout is the > > tap-driver.pl pipe. If we keep the pipe open then tap-driver.pl will not > > detect that

Re: [PATCH v2 0/4] vhost-user: avoid g_return_val_if() in get/set_config()

2020-12-03 Thread Stefano Garzarella
On Wed, Dec 02, 2020 at 03:26:07PM +, Stefan Hajnoczi wrote: v2: * Print errors [Marc-André] Markus Armbruster pointed out that g_return_val_if() is meant for programming errors. It must not be used for input validation since it can be compiled out. Use explicit if statements instead.

Re: [PATCH 0/5] SCSI: fix transfer limits for SCSI passthrough

2020-12-03 Thread Paolo Bonzini
On 03/12/20 13:49, Maxim Levitsky wrote: On Wed, 2020-11-04 at 19:32 +0200, Maxim Levitsky wrote: This patch series attempts to provide a solution to the problem of the transfer limits of the raw file driver (host_device/file-posix), some of which I already tried to fix in the past. I included

Re: [PATCH 0/5] SCSI: fix transfer limits for SCSI passthrough

2020-12-03 Thread Maxim Levitsky
On Wed, 2020-11-04 at 19:32 +0200, Maxim Levitsky wrote: > This patch series attempts to provide a solution to the problem of the > transfer > limits of the raw file driver (host_device/file-posix), some of which I > already tried to fix in the past. > > I included 2 patches from Tom Yan which

Re: [PATCH v13 08/10] copy-on-read: skip non-guest reads if no copy needed

2020-12-03 Thread Vladimir Sementsov-Ogievskiy
02.12.2020 21:30, Andrey Shinkevich wrote: If the flag BDRV_REQ_PREFETCH was set, skip idling read/write operations in COR-driver. It can be taken into account for the COR-algorithms optimization. That check is being made during the block stream job by the moment. Add the BDRV_REQ_PREFETCH flag

Re: [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes

2020-12-03 Thread Francisco Iglesias
Hello Bin, On [2020 Dec 02] Wed 22:30:37, Bin Meng wrote: > From: Xuzhou Cheng > > Auto Address Increment (AAI) Word-Program is a special command of > SST flashes. AAI-WP allows multiple bytes of data to be programmed > without re-issuing the next sequential address location. > >

Re: [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-03 Thread Philippe Mathieu-Daudé
On 12/3/20 1:02 PM, Li Qiang wrote: > Philippe Mathieu-Daudé 于2020年12月3日周四 下午7:37写道: >> >> Hi Li, >> >> On 12/3/20 12:21 PM, Li Qiang wrote: >>> Philippe Mathieu-Daudé 于2020年12月2日周三 上午3:13写道: cdb_len can not be zero... (or less than 6) here, else we have a out-of-bound read first

Re: [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-03 Thread Li Qiang
Philippe Mathieu-Daudé 于2020年12月3日周四 下午7:37写道: > > Hi Li, > > On 12/3/20 12:21 PM, Li Qiang wrote: > > Philippe Mathieu-Daudé 于2020年12月2日周三 上午3:13写道: > >> > >> cdb_len can not be zero... (or less than 6) here, else we have a > >> out-of-bound read first in scsi_cdb_length(): > >> > >> 71 int

Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation

2020-12-03 Thread Stefan Hajnoczi
On Thu, Dec 03, 2020 at 02:26:08PM +0400, Marc-André Lureau wrote: > On Thu, Dec 3, 2020 at 1:52 PM Stefan Hajnoczi wrote: > > > On Wed, Dec 02, 2020 at 07:50:51PM +0400, Marc-André Lureau wrote: > > > On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi > > wrote: > > > > > > > Do not validate input

Re: [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-03 Thread Philippe Mathieu-Daudé
Hi Li, On 12/3/20 12:21 PM, Li Qiang wrote: > Philippe Mathieu-Daudé 于2020年12月2日周三 上午3:13写道: >> >> cdb_len can not be zero... (or less than 6) here, else we have a >> out-of-bound read first in scsi_cdb_length(): >> >> 71 int scsi_cdb_length(uint8_t *buf) >> 72 { >> 73 int cdb_len; >> 74

Re: [PATCH 1/2] nvme: updated shared header for copy command

2020-12-03 Thread Stefan Hajnoczi
On Tue, Nov 24, 2020 at 08:14:17AM +0100, Klaus Jensen wrote: > From: Klaus Jensen > > Add new data structures and types for the Simple Copy command. > > Signed-off-by: Klaus Jensen > Cc: Stefan Hajnoczi > Cc: Fam Zheng > --- > include/block/nvme.h | 45

Re: [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-03 Thread Li Qiang
Philippe Mathieu-Daudé 于2020年12月2日周三 上午3:13写道: > > cdb_len can not be zero... (or less than 6) here, else we have a > out-of-bound read first in scsi_cdb_length(): > > 71 int scsi_cdb_length(uint8_t *buf) > 72 { > 73 int cdb_len; > 74 > 75 switch (buf[0] >> 5) { Hi Philippe, Here I

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-03 Thread Paolo Bonzini
On 02/12/20 18:35, Kevin Wolf wrote: Could we have an intermediate state that doesn't require any duplication and thus have no separate code paths that could diverge? The one requirement we have for an intermediate state is that it supports both interfaces: The well-know create/set

Re: [PATCH v2 1/4] tests/qtest/fuzz-test: Quit test_lp1878642 once done

2020-12-03 Thread Li Qiang
Philippe Mathieu-Daudé 于2020年12月2日周三 上午3:11写道: > > Missed in fd250172842 ("qtest: add a reproducer for LP#1878642"). > > Reviewed-by: Thomas Huth > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Li Qiang > --- > tests/qtest/fuzz-test.c | 1 + > 1 file changed, 1 insertion(+) > > diff

Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation

2020-12-03 Thread Marc-André Lureau
On Thu, Dec 3, 2020 at 1:52 PM Stefan Hajnoczi wrote: > On Wed, Dec 02, 2020 at 07:50:51PM +0400, Marc-André Lureau wrote: > > On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi > wrote: > > > > > Do not validate input with g_return_val_if(). This API is intended for > > > checking programming

Re: [PATCH] scsi: allow user to set werror as report

2020-12-03 Thread Philippe Mathieu-Daudé
On 12/3/20 3:55 AM, Zihao Chang wrote: > Ping? This is a fix patch which has been reviewed, whose tree should it go > via? The change itself is in-between 'Block layer' and 'SCSI' subsystems, so either Paolo or Kevin (Cc'ing qemu-block@). > > Thanks > Zihao > > On 2020/11/3 22:03, Zihao Chang

Re: [PATCH v2 3/4] contrib/vhost-user-input: avoid g_return_val_if() input validation

2020-12-03 Thread Stefan Hajnoczi
On Wed, Dec 02, 2020 at 07:51:49PM +0400, Marc-André Lureau wrote: > On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi wrote: > > > Do not validate input with g_return_val_if(). This API is intended for > > checking programming errors and is compiled out with -DG_DISABLE_CHECKS. > > > > Use an

Re: [PATCH v2 2/4] contrib/vhost-user-gpu: avoid g_return_val_if() input validation

2020-12-03 Thread Stefan Hajnoczi
On Wed, Dec 02, 2020 at 07:50:51PM +0400, Marc-André Lureau wrote: > On Wed, Dec 2, 2020 at 7:27 PM Stefan Hajnoczi wrote: > > > Do not validate input with g_return_val_if(). This API is intended for > > checking programming errors and is compiled out with -DG_DISABLE_CHECKS. > > > > Use an

RE: [PATCH] block: Don't inactivate bs if it is aleady inactive

2020-12-03 Thread Tuguoyi
> -Original Message- > From: Vladimir Sementsov-Ogievskiy [mailto:vsement...@virtuozzo.com] > Sent: Saturday, November 28, 2020 4:48 PM > To: tuguoyi (Cloud) ; Kevin Wolf ; > Max Reitz ; qemu-block@nongnu.org > Cc: qemu-de...@nongnu.org; wangyong (Cloud) > Subject: Re: [PATCH] block:

Re: [PATCH] hw/block: m25p80: Fix fast read for SST flashes

2020-12-03 Thread Francisco Iglesias
Hi Bin and Alistair, On [2020 Dec 02] Wed 11:40:11, Alistair Francis wrote: > On Sun, Nov 29, 2020 at 6:55 PM Bin Meng wrote: > > > > From: Bin Meng > > > > SST flashes require a dummy byte after the address bits. > > > > Signed-off-by: Bin Meng > > I couldn't find a datasheet that says