Re: [PULL 00/13] Block layer patches

2020-01-28 Thread Peter Maydell
On Mon, 27 Jan 2020 at 17:56, Kevin Wolf wrote: > > The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c: > > Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into > staging (2020-01-27 13:02:36 +) > > are available in the Git repository at: > >

Re: [PATCH v2 2/2] doc: Use @code rather than @var for options

2020-01-28 Thread Peter Maydell
On Tue, 28 Jan 2020 at 07:39, David Edmondson wrote: > > Eric Blake writes: > > > On 1/24/20 4:34 AM, David Edmondson wrote: > >> Texinfo defines @var for metasyntactic variables and such terms are > >> shown in upper-case or italics in the output of makeinfo. When > >> considering an option to

Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

2020-01-28 Thread Markus Armbruster
Peter Krempa writes: > On Mon, Jan 27, 2020 at 14:39:02 -0500, John Snow wrote: >> >> >> On 1/27/20 5:36 AM, Maxim Levitsky wrote: >> > This patch series is bunch of cleanups >> > to the hmp monitor code. >> > >> > This series only touched blockdev related hmp handlers. >> > >> > No

[PATCH 1/2] mirror: Store MirrorOp.co for debuggability

2020-01-28 Thread Kevin Wolf
If a coroutine is launched, but the coroutine pointer isn't stored anywhere, debugging any problems inside the coroutine is quite hard. Let's store the coroutine pointer of a mirror operation in MirrorOp to have it available in the debugger. Signed-off-by: Kevin Wolf --- block/mirror.c | 2 ++

[PATCH 0/2] mirror: Fix hang (operation waiting for itself)

2020-01-28 Thread Kevin Wolf
Kevin Wolf (2): mirror: Store MirrorOp.co for debuggability mirror: Don't let an operation wait for itself block/mirror.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) -- 2.20.1

[PATCH 2/2] mirror: Don't let an operation wait for itself

2020-01-28 Thread Kevin Wolf
mirror_wait_for_free_in_flight_slot() just picks a random operation to wait for. However, when mirror_co_read() waits for free slots, its MirrorOp is already in s->ops_in_flight, so if not enough slots are immediately available, an operation can end up waiting for itself to complete, which results

Re: [PATCH] iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)

2020-01-28 Thread Philippe Mathieu-Daudé
Hi guys, (Cc'ing Jon) On 1/23/20 5:59 PM, Kevin Wolf wrote: Am 23.01.2020 um 13:44 hat Felipe Franciosi geschrieben: When querying an iSCSI server for the provisioning status of blocks (via GET LBA STATUS), Qemu only validates that the response descriptor zero's LBA matches the one requested.

Re: [PATCH] iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)

2020-01-28 Thread Kevin Wolf
Am 28.01.2020 um 13:30 hat Philippe Mathieu-Daudé geschrieben: > Hi guys, > > (Cc'ing Jon) > > On 1/23/20 5:59 PM, Kevin Wolf wrote: > > Am 23.01.2020 um 13:44 hat Felipe Franciosi geschrieben: > > > When querying an iSCSI server for the provisioning status of blocks (via > > > GET LBA STATUS),

Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

2020-01-28 Thread Ján Tomko
On Mon, Jan 27, 2020 at 04:01:31PM -0500, John Snow wrote: On 1/27/20 3:43 PM, Peter Krempa wrote: On Mon, Jan 27, 2020 at 14:39:02 -0500, John Snow wrote: On 1/27/20 5:36 AM, Maxim Levitsky wrote: This patch series is bunch of cleanups to the hmp monitor code. This series only touched

Re: [PATCH 2/2] mirror: Don't let an operation wait for itself

2020-01-28 Thread Eric Blake
On 1/28/20 9:17 AM, Kevin Wolf wrote: mirror_wait_for_free_in_flight_slot() just picks a random operation to wait for. However, when mirror_co_read() waits for free slots, its MirrorOp is already in s->ops_in_flight, so if not enough slots are immediately available, an operation can end up

Re: [PATCH 01/13] qcrypto: add generic infrastructure for crypto options amendment

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:38PM +0200, Maxim Levitsky wrote: > This will be used first to implement luks keyslot management. > > block_crypto_amend_opts_init will be used to convert > qemu-img cmdline to QCryptoBlockAmendOptions > > Signed-off-by: Maxim Levitsky > --- > block/crypto.c

Re: [PATCH 06/13] block/crypto: implement the encryption key management

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:43PM +0200, Maxim Levitsky wrote: > This implements the encryption key management using the generic code in > qcrypto layer and exposes it to the user via qemu-img > > This code adds another 'write_func' because the initialization > write_func works directly on the

Re: [PATCH 07/13] qcow2: extend qemu-img amend interface with crypto options

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:44PM +0200, Maxim Levitsky wrote: > Now that we have all the infrastructure in place, > wire it in the qcow2 driver and expose this to the user. > > Signed-off-by: Maxim Levitsky > --- > block/qcow2.c | 101 +++--- > 1

Re: [PATCH 11/13] block/crypto: implement blockdev-amend

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:48PM +0200, Maxim Levitsky wrote: > Signed-off-by: Maxim Levitsky > --- > block/crypto.c | 70 > qapi/block-core.json | 14 - > 2 files changed, 64 insertions(+), 20 deletions(-) > > diff --git

[PATCH v2 4/4] monitor: Move qmp_query_qmp_schema to qmp-cmds-control.c

2020-01-28 Thread Kevin Wolf
monitor/misc.c contains code that works only in the system emulator, so it can't be linked to tools like a storage daemon. In order to make schema introspection available for tools, move the function to monitor/qmp-cmds-control.c, which can be linked into the storage daemon. Signed-off-by: Kevin

Re: [PATCH 1/2] mirror: Store MirrorOp.co for debuggability

2020-01-28 Thread Eric Blake
On 1/28/20 9:17 AM, Kevin Wolf wrote: If a coroutine is launched, but the coroutine pointer isn't stored anywhere, debugging any problems inside the coroutine is quite hard. Let's store the coroutine pointer of a mirror operation in MirrorOp to have it available in the debugger. Signed-off-by:

Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

2020-01-28 Thread Dr. David Alan Gilbert
* John Snow (js...@redhat.com) wrote: > > > On 1/27/20 3:43 PM, Peter Krempa wrote: > > On Mon, Jan 27, 2020 at 14:39:02 -0500, John Snow wrote: > >> > >> > >> On 1/27/20 5:36 AM, Maxim Levitsky wrote: > >>> This patch series is bunch of cleanups > >>> to the hmp monitor code. > >>> > >>> This

Re: [PATCH 04/13] block: amend: separate amend and create options for qemu-img

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:41PM +0200, Maxim Levitsky wrote: > Some options are only useful for creation > (or hard to be amended, like cluster size for qcow2), while some other > options are only useful for amend, like upcoming keyslot management > options for luks > > Since currently only

Re: [PATCH 08/13] iotests: filter few more luks specific create options

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:45PM +0200, Maxim Levitsky wrote: > This allows more tests to be able to have same output on both qcow2 luks > encrypted images > and raw luks images > > Signed-off-by: Maxim Levitsky > --- > tests/qemu-iotests/087.out | 6 +++--- >

[PATCH v2 3/4] monitor: Create monitor/qmp-cmds-control.c

2020-01-28 Thread Kevin Wolf
Move all of the QMP commands handlers to implement the 'control' module (qapi/control.json) that can be shared between the system emulator and tools such as a storage daemon to a new file monitor/qmp-cmds-control.c. Signed-off-by: Kevin Wolf --- monitor/misc.c | 110

Re: [PATCH v3 04/13] monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert (It's easier to compare if you keep the function order the same) > --- > block/monitor/block-hmp-cmds.c | 97 +- > blockdev.c

[PATCH v2 0/4] monitor: Refactoring in preparation for qemu-storage-daemon

2020-01-28 Thread Kevin Wolf
This series creates a QAPI module 'control' that can be used from tools outside the system emulator and moves some monitor initialisation code from vl.c to monitor code. It is split from the series to introduce qemu-storage-daemon because these refactorings make sense on their own as cleanups.

Re: [PATCH v3 03/13] monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > These days device-hotplug.c only contains the hmp_drive_add > In the next patch, rest of hmp_drive* functions will be moved > there. > > Also change the license of that file to GPL2+ since most > of the code that will be moved there is under that

Re: [PATCH 02/13] qcrypto-luks: implement encryption key management

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 21, 2020 at 03:13:01PM +0200, Maxim Levitsky wrote: > On Tue, 2020-01-21 at 08:54 +0100, Markus Armbruster wrote: > > > > > > +## > > > +# @LUKSKeyslotUpdate: > > > +# > > > +# @keyslot: If specified, will update only keyslot with this > > > index > > > +# > > > +#

Re: [PATCH 12/13] block/qcow2: implement blockdev-amend

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:49PM +0200, Maxim Levitsky wrote: > Currently the implementation only supports amending the encryption > options, unlike the qemu-img version > > Signed-off-by: Maxim Levitsky > --- > block/qcow2.c| 39 +++ >

Re: [PATCH 02/13] qcrypto-luks: implement encryption key management

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 09:33:39PM +0200, Maxim Levitsky wrote: > Next few patches will expose that functionality > to the user. > > Signed-off-by: Maxim Levitsky > --- > crypto/block-luks.c | 374 +++- > qapi/crypto.json| 50 +- > 2 files

Re: [PATCH 02/13] qcrypto-luks: implement encryption key management

2020-01-28 Thread Daniel P . Berrangé
On Tue, Jan 28, 2020 at 05:11:16PM +, Daniel P. Berrangé wrote: > On Tue, Jan 21, 2020 at 03:13:01PM +0200, Maxim Levitsky wrote: > > On Tue, 2020-01-21 at 08:54 +0100, Markus Armbruster wrote: > > > > > > > > > > +## > > > > +# @LUKSKeyslotUpdate: > > > > +# > > > > +# @keyslot: If

[PATCH v2 2/4] qapi: Create module 'control'

2020-01-28 Thread Kevin Wolf
misc.json contains definitions that are related to the system emulator, so it can't be used for other tools like the storage daemon. This patch moves basic functionality that is shared between all tools (and mostly related to the monitor itself) into a new control.json, which could be used in

[PATCH v2 1/4] monitor: Move monitor option parsing to monitor/monitor.c

2020-01-28 Thread Kevin Wolf
Both the system emulators and tools with QMP support (specifically, the planned storage daemon) will need to parse monitor options, so move that code to monitor/monitor.c, which can be linked into binaries that aren't a system emulator. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster

Re: [PATCH v3 08/13] monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Yes, I think that's OK; I can imagine nbd might want to move on it's own somewhere since it's not really core block code; copying in Eric. Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c

Re: [PATCH v3 09/13] monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 138 + > include/block/block-hmp-commands.h | 9 ++ > include/monitor/hmp.h | 6 -- >

Re: [PATCH v3 07/13] monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 47 ++ > include/block/block-hmp-commands.h | 4 +++ > include/monitor/hmp.h | 3 -- >

Re: [PATCH v3 12/13] add 'error' prefix to vreport

2020-01-28 Thread Maxim Levitsky
On Mon, 2020-01-27 at 12:36 +0200, Maxim Levitsky wrote: > This changes most of qemu's error messages, > but it feels like the right thing to do. > > This is WIP patch, since I updated most of iotests but not all of them, > and will be updated if this patch is accepeted in the review. > Also few

Re: [PATCH v3 04/13] monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c

2020-01-28 Thread Maxim Levitsky
On Tue, 2020-01-28 at 17:51 +, Dr. David Alan Gilbert wrote: > * Maxim Levitsky (mlevi...@redhat.com) wrote: > > Signed-off-by: Maxim Levitsky > > Reviewed-by: Dr. David Alan Gilbert > > (It's easier to compare if you keep the function order the same) Sorry about that, next time I will do

Re: [PATCH v3 13/13] monitor/hmp: Prefer to use hmp_handle_error for error reporting in block hmp commands

2020-01-28 Thread Maxim Levitsky
On Mon, 2020-01-27 at 12:36 +0200, Maxim Levitsky wrote: > This way they all will be prefixed with 'Error:' which some parsers > (e.g libvirt) need > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1719169 > > Signed-off-by: Maxim Levitsky And I probably will keep that patch since it

Re: [PATCH v3 11/13] monitor: Move hmp_drive_add_node to block-hmp-cmds.c

2020-01-28 Thread Maxim Levitsky
On Tue, 2020-01-28 at 19:03 +, Dr. David Alan Gilbert wrote: > * Maxim Levitsky (mlevi...@redhat.com) wrote: > > Signed-off-by: Maxim Levitsky > > Looks OK to me, I'm not clear on the name for 'bdrv_set_monitor_owned' > I'd want a block person to OK that, but: The name inspiration came

Re: [PATCH v3 10/13] monitor/hmp: move hmp_info_block* to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 388 + > include/block/block-hmp-commands.h | 4 + > include/monitor/hmp.h | 4 - >

Re: [PATCH v3 05/13] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 64 ++ > include/block/block-hmp-commands.h | 3 ++ > include/monitor/hmp.h | 2 - >

Re: [PATCH v3 06/13] monitor/hmp: move hmp_block_job* to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 52 ++ > include/block/block-hmp-commands.h | 6 > include/monitor/hmp.h | 5 --- >

Re: [PATCH v3 03/13] monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c

2020-01-28 Thread Maxim Levitsky
On Tue, 2020-01-28 at 16:56 +, Dr. David Alan Gilbert wrote: > * Maxim Levitsky (mlevi...@redhat.com) wrote: > > These days device-hotplug.c only contains the hmp_drive_add > > In the next patch, rest of hmp_drive* functions will be moved > > there. > > > > Also change the license of that

Re: [PATCH v3 08/13] monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c

2020-01-28 Thread Eric Blake
On 1/28/20 12:56 PM, Dr. David Alan Gilbert wrote: * Maxim Levitsky (mlevi...@redhat.com) wrote: Signed-off-by: Maxim Levitsky Yes, I think that's OK; I can imagine nbd might want to move on it's own somewhere since it's not really core block code; copying in Eric. I think that

Re: [PATCH v3 11/13] monitor: Move hmp_drive_add_node to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Looks OK to me, I'm not clear on the name for 'bdrv_set_monitor_owned' I'd want a block person to OK that, but: Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 30

Re: [PATCH v3 06/13] monitor/hmp: move hmp_block_job* to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 52 ++ > include/block/block-hmp-commands.h | 6 > include/monitor/hmp.h | 5 --- >

Re: [PATCH v3 09/13] monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > Signed-off-by: Maxim Levitsky Reviewed-by: Dr. David Alan Gilbert > --- > block/monitor/block-hmp-cmds.c | 138 + > include/block/block-hmp-commands.h | 9 ++ > include/monitor/hmp.h | 6 -- >

Re: [PATCH 9/9] monitor/hmp: Prefer to use hmp_handle_error for error reporting in block hmp commands

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote: > On Mon, 2020-01-27 at 14:44 +0100, Markus Armbruster wrote: > > Maxim Levitsky writes: > > > > > On Wed, 2019-11-27 at 09:38 +0100, Markus Armbruster wrote: > > > > Title is too long. blockdev-hmp-cmds.c will become > > > >

Re: [PATCH v2 1/7] block/block-copy: specialcase first copy_range request

2020-01-28 Thread Andrey Shinkevich
On 27/11/2019 21:08, Vladimir Sementsov-Ogievskiy wrote: > In block_copy_do_copy we fallback to read+write if copy_range failed. > In this case copy_size is larger than defined for buffered IO, and > there is corresponding commit. Still, backup copies data cluster by > cluster, and most of

Re: [PATCH v2 1/4] monitor: Move monitor option parsing to monitor/monitor.c

2020-01-28 Thread Markus Armbruster
Kevin Wolf writes: > Both the system emulators and tools with QMP support (specifically, the > planned storage daemon) will need to parse monitor options, so move that > code to monitor/monitor.c, which can be linked into binaries that aren't > a system emulator. > > Signed-off-by: Kevin Wolf >