Re: [Qemu-block] [PATCH RFC] file-posix: make lock_fd read-only

2017-10-10 Thread Eric Blake
On 10/10/2017 02:30 PM, Denis V. Lunev wrote: > On 10/10/2017 09:50 PM, Eric Blake wrote: >> On 10/10/2017 08:42 AM, Vladimir Sementsov-Ogievskiy wrote: >>> We do not reopen lock_fd on bdrv_reopen which leads to problems on >>> reopen image RO. So, lets make lock_fd be always RO. >>> This is

Re: [Qemu-block] [PATCH RFC] file-posix: make lock_fd read-only

2017-10-10 Thread Denis V. Lunev
On 10/10/2017 09:50 PM, Eric Blake wrote: > On 10/10/2017 08:42 AM, Vladimir Sementsov-Ogievskiy wrote: >> We do not reopen lock_fd on bdrv_reopen which leads to problems on >> reopen image RO. So, lets make lock_fd be always RO. >> This is correct, because qemu_lock_fd always called with

Re: [Qemu-block] [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status()

2017-10-10 Thread John Snow
On 10/10/2017 03:00 PM, Eric Blake wrote: > On 10/10/2017 09:43 AM, Eric Blake wrote: > --- v5: use second label for cleaner exit logic [John], use local_pnum >> @@ -1811,16 +1811,19 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,

Re: [Qemu-block] [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status()

2017-10-10 Thread Eric Blake
On 10/10/2017 09:43 AM, Eric Blake wrote: >>> --- >>> v5: use second label for cleaner exit logic [John], use local_pnum > >>> @@ -1811,16 +1811,19 @@ static int64_t coroutine_fn >>> bdrv_co_get_block_status(BlockDriverState *bs, >>> int64_t total_sectors; >>> int64_t n; >>>

Re: [Qemu-block] [PATCH RFC] file-posix: make lock_fd read-only

2017-10-10 Thread Eric Blake
On 10/10/2017 08:42 AM, Vladimir Sementsov-Ogievskiy wrote: > We do not reopen lock_fd on bdrv_reopen which leads to problems on > reopen image RO. So, lets make lock_fd be always RO. > This is correct, because qemu_lock_fd always called with exclusive=false > on lock_fd. How is that correct?

Re: [Qemu-block] [PATCH v5 07/23] block: Convert bdrv_get_block_status() to bytes

2017-10-10 Thread Eric Blake
On 10/10/2017 09:46 AM, Kevin Wolf wrote: > Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: >> We are gradually moving away from sector-based interfaces, towards >> byte-based. In the common case, allocation is unlikely to ever use >> values that are not naturally sector-aligned, but it is

Re: [Qemu-block] [PATCH v2 10/10] nbd: Minimal structured read for client

2017-10-10 Thread Paolo Bonzini
On 10/10/2017 16:55, Vladimir Sementsov-Ogievskiy wrote: > Hmm, would it be simpler just pass a function pointer, which should be > called on each loop iteration? > So, we will return to one common func nbd_co_receive_reply, but with two > additional parameters: func and opaque? Function pointers

Re: [Qemu-block] [PATCH v2 10/10] nbd: Minimal structured read for client

2017-10-10 Thread Vladimir Sementsov-Ogievskiy
10.10.2017 11:02, Paolo Bonzini wrote: On 09/10/2017 19:27, Vladimir Sementsov-Ogievskiy wrote: +int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured, + _ret, qiov, payload); + +if (ret < 0) { +s->quit = true; +} else { +

Re: [Qemu-block] [PATCH v5 07/23] block: Convert bdrv_get_block_status() to bytes

2017-10-10 Thread Kevin Wolf
Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: > We are gradually moving away from sector-based interfaces, towards > byte-based. In the common case, allocation is unlikely to ever use > values that are not naturally sector-aligned, but it is possible > that byte-based values will let us be

Re: [Qemu-block] [PATCH v5 04/23] qcow2: Switch is_zero_sectors() to byte-based

2017-10-10 Thread Eric Blake
On 10/10/2017 09:15 AM, Kevin Wolf wrote: > Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: >> We are gradually converting to byte-based interfaces, as they are >> easier to reason about than sector-based. Convert another internal >> function (no semantic change), and rename it to is_zero() in

Re: [Qemu-block] [PATCH v5 00/23] make bdrv_get_block_status byte-based

2017-10-10 Thread Eric Blake
On 10/10/2017 07:58 AM, Kevin Wolf wrote: > Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: >> There are patches floating around to add NBD_CMD_BLOCK_STATUS, >> but NBD wants to report status on byte granularity (even if the >> reporting will probably be naturally aligned to sectors or even >>

Re: [Qemu-block] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status()

2017-10-10 Thread Eric Blake
On 10/10/2017 08:59 AM, Kevin Wolf wrote: > Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: >> Not all callers care about which BDS owns the mapping for a given >> range of the file. This patch merely simplifies the callers by >> consolidating the logic in the common call point, while

Re: [Qemu-block] [PATCH v2 10/10] nbd: Minimal structured read for client

2017-10-10 Thread Vladimir Sementsov-Ogievskiy
09.10.2017 20:27, Vladimir Sementsov-Ogievskiy wrote: Minimal implementation: for structured error only error_report error message. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/nbd.h | 6 + block/nbd-client.c | 358

Re: [Qemu-block] [PATCH v5 04/23] qcow2: Switch is_zero_sectors() to byte-based

2017-10-10 Thread Kevin Wolf
Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Convert another internal > function (no semantic change), and rename it to is_zero() in the > process. > > Signed-off-by: Eric Blake

Re: [Qemu-block] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status()

2017-10-10 Thread Kevin Wolf
Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: > Not all callers care about which BDS owns the mapping for a given > range of the file. This patch merely simplifies the callers by > consolidating the logic in the common call point, while guaranteeing > a non-NULL file to all the driver

[Qemu-block] [PATCH RFC] file-posix: make lock_fd read-only

2017-10-10 Thread Vladimir Sementsov-Ogievskiy
We do not reopen lock_fd on bdrv_reopen which leads to problems on reopen image RO. So, lets make lock_fd be always RO. This is correct, because qemu_lock_fd always called with exclusive=false on lock_fd. Signed-off-by: Vladimir Sementsov-Ogievskiy --- Hi all! We've

Re: [Qemu-block] [PATCH v5 00/23] make bdrv_get_block_status byte-based

2017-10-10 Thread Kevin Wolf
Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: > There are patches floating around to add NBD_CMD_BLOCK_STATUS, > but NBD wants to report status on byte granularity (even if the > reporting will probably be naturally aligned to sectors or even > much higher levels). I've therefore started the

Re: [Qemu-block] [PATCH 15/18] block/mirror: Add active mirroring

2017-10-10 Thread Kevin Wolf
Am 18.09.2017 um 18:26 hat Max Reitz geschrieben: > On 2017-09-18 12:06, Stefan Hajnoczi wrote: > > On Sat, Sep 16, 2017 at 03:58:01PM +0200, Max Reitz wrote: > >> On 2017-09-14 17:57, Stefan Hajnoczi wrote: > >>> On Wed, Sep 13, 2017 at 08:19:07PM +0200, Max Reitz wrote: > This patch

Re: [Qemu-block] [PATCH 13/18] block/mirror: Keep write perm for pending writes

2017-10-10 Thread Kevin Wolf
Am 13.09.2017 um 20:19 hat Max Reitz geschrieben: > The owner of the mirror BDS might retire its write permission; but there > may still be pending mirror operations so the mirror BDS cannot > necessarily retire its write permission for its child then. > > Signed-off-by: Max Reitz

Re: [Qemu-block] [PATCH 10/18] block/mirror: Make source the file child

2017-10-10 Thread Kevin Wolf
Am 13.09.2017 um 20:19 hat Max Reitz geschrieben: > Regarding the source BDS, the mirror BDS is arguably a filter node. > Therefore, the source BDS should be its "file" child. > > Signed-off-by: Max Reitz TODO: Justification why this doesn't break things like

Re: [Qemu-block] [PATCH 08/18] block/mirror: Use source as a BdrvChild

2017-10-10 Thread Kevin Wolf
Am 13.09.2017 um 20:19 hat Max Reitz geschrieben: > With this, the mirror_top_bs is no longer just a technically required > node in the BDS graph but actually represents the block job operation. > > Signed-off-by: Max Reitz > --- > block/mirror.c | 18 ++ > 1

Re: [Qemu-block] [PATCH 05/18] block/mirror: Convert to coroutines

2017-10-10 Thread Kevin Wolf
Am 13.09.2017 um 20:18 hat Max Reitz geschrieben: > In order to talk to the source BDS (and maybe in the future to the > target BDS as well) directly, we need to convert our existing AIO > requests into coroutine I/O requests. > > Signed-off-by: Max Reitz Please follow

Re: [Qemu-block] [PATCH v2 04/10] nbd-server: refactor simple reply sending

2017-10-10 Thread Paolo Bonzini
On 10/10/2017 10:40, Daniel P. Berrange wrote: > The I/O channels code does not make guarantees wrt concurrent usage of > threads or coroutines. It is the callers responsibility to avoid any > concurrent usage for all APIs. With coroutines you are at least avoiding > the danger of corrupting

Re: [Qemu-block] [PATCH v2 04/10] nbd-server: refactor simple reply sending

2017-10-10 Thread Daniel P. Berrange
On Mon, Oct 09, 2017 at 02:18:10PM -0500, Eric Blake wrote: > [adding Dan for a qio question - search for your name below] > > On 10/09/2017 12:27 PM, Vladimir Sementsov-Ogievskiy wrote: > > Get rid of calculating structure fields offsets by hand and set_cork, > > rename nbd_co_send_reply to

Re: [Qemu-block] [PATCH 02/18] block: BDS deletion during bdrv_drain_recurse

2017-10-10 Thread Kevin Wolf
Am 13.09.2017 um 20:18 hat Max Reitz geschrieben: > Drainined a BDS child may lead to both the original BDS and/or its other > children being deleted (e.g. if the original BDS represents a block > job). We should prepare for this in both bdrv_drain_recurse() and > bdrv_drained_begin() by

Re: [Qemu-block] [PATCH v2 10/10] nbd: Minimal structured read for client

2017-10-10 Thread Paolo Bonzini
On 09/10/2017 19:27, Vladimir Sementsov-Ogievskiy wrote: > > +int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured, > + _ret, qiov, payload); > + > +if (ret < 0) { > +s->quit = true; > +} else { > +/* For assert at

Re: [Qemu-block] [Qemu-devel] Patch to add helpful tracing output for driver authors in NVMe emulation

2017-10-10 Thread Kevin Wolf
Am 10.10.2017 um 08:58 hat Markus Armbruster geschrieben: > Doug Gale writes: > > > I used exclamations as a concise way of indicating that the driver did > > something nonsensical, or horribly invalid, like something likely to > > cause a memory corruption, trying to start

Re: [Qemu-block] [Qemu-devel] Patch to add helpful tracing output for driver authors in NVMe emulation

2017-10-10 Thread Markus Armbruster
Doug Gale writes: > I used exclamations as a concise way of indicating that the driver did > something nonsensical, or horribly invalid, like something likely to > cause a memory corruption, trying to start the controller with a > nonsense configuration, providing invalid PRDs