Re: [Qemu-block] [Qemu-devel] [PATCH 12/15] gluster: Plug memory leaks in qemu_gluster_parse_json()

2017-03-02 Thread Markus Armbruster
Niels de Vos writes: > On Thu, Mar 02, 2017 at 10:44:03PM +0100, Markus Armbruster wrote: >> To reproduce, run >> >> $ valgrind qemu-system-x86_64 --nodefaults -S --drive >> driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx >> >> Signed-off-by: Markus

Re: [Qemu-block] [Qemu-devel] [PATCH 10/15] gluster: Drop assumptions on SocketTransport names

2017-03-02 Thread Markus Armbruster
Niels de Vos writes: > On Thu, Mar 02, 2017 at 10:44:01PM +0100, Markus Armbruster wrote: >> qemu_gluster_glfs_init() passes the names of QAPI enumeration type >> SocketTransport to glfs_set_volfile_server(). Works, because they >> were chosen to match. But the coupling is

Re: [Qemu-block] [PATCH 12/15] gluster: Plug memory leaks in qemu_gluster_parse_json()

2017-03-02 Thread Niels de Vos
On Thu, Mar 02, 2017 at 10:44:03PM +0100, Markus Armbruster wrote: > To reproduce, run > > $ valgrind qemu-system-x86_64 --nodefaults -S --drive > driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx > > Signed-off-by: Markus Armbruster > --- > block/gluster.c |

Re: [Qemu-block] [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation

2017-03-02 Thread Greg Kurz
On Fri, 3 Mar 2017 10:11:09 +0800 Stefan Hajnoczi wrote: > On Wed, Mar 01, 2017 at 10:27:48PM +0100, Greg Kurz wrote: > > On Wed, 1 Mar 2017 11:50:23 + > > Stefan Hajnoczi wrote: > > > > > v3: > > > * Added Patch 2 to fix invalid test parameters

Re: [Qemu-block] [PATCH 10/15] gluster: Drop assumptions on SocketTransport names

2017-03-02 Thread Niels de Vos
On Thu, Mar 02, 2017 at 10:44:01PM +0100, Markus Armbruster wrote: > qemu_gluster_glfs_init() passes the names of QAPI enumeration type > SocketTransport to glfs_set_volfile_server(). Works, because they > were chosen to match. But the coupling is artificial. Use the > appropriate literal

Re: [Qemu-block] [PATCH 11/15] gluster: Don't duplicate qapi-util.c's qapi_enum_parse()

2017-03-02 Thread Niels de Vos
On Thu, Mar 02, 2017 at 10:44:02PM +0100, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster > --- > block/gluster.c | 30 +- > 1 file changed, 9 insertions(+), 21 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index

Re: [Qemu-block] [Qemu-devel] [PATCH 00/15] block: A bunch of fixes for Sheepdog and Gluster

2017-03-02 Thread Markus Armbruster
Eric Blake writes: > On 03/02/2017 03:43 PM, Markus Armbruster wrote: >> Bad error handling, memory leaks, and lack of blockdev-add support. > > How hard are we trying to get blockdev-add working in 2.9? Or is this > series 2.10 material now? Definitely not 2.10: seven

Re: [Qemu-block] [Qemu-devel] [PATCH 02/15] sheepdog: Fix error handling in sd_snapshot_delete()

2017-03-02 Thread Markus Armbruster
Eric Blake writes: > On 03/02/2017 03:43 PM, Markus Armbruster wrote: >> As a bdrv_snapshot_delete() method, sd_snapshot_delete() must set an >> error and return negative errno on failure. It sometimes returns -1, >> and sometimes neglects to set an error. It also prints

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Markus Armbruster
Philippe Mathieu-Daudé writes: > On 03/02/2017 08:32 PM, Eric Blake wrote: >> On 03/02/2017 03:43 PM, Markus Armbruster wrote: >>> sd_parse_uri() truncates long VDI names silently. Reject them >>> instead. >>> >>> Signed-off-by: Markus Armbruster >>> --- >>>

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Markus Armbruster
Eric Blake writes: > On 03/02/2017 03:43 PM, Markus Armbruster wrote: >> sd_parse_uri() truncates long VDI names silently. Reject them >> instead. >> >> Signed-off-by: Markus Armbruster >> --- >> block/sheepdog.c | 4 +++- >> 1 file changed, 3

Re: [Qemu-block] [Qemu-devel] [PATCH 01/15] sheepdog: Defuse time bomb in sd_open() error handling

2017-03-02 Thread Markus Armbruster
Eric Blake writes: > On 03/02/2017 03:43 PM, Markus Armbruster wrote: >> When qemu_opts_absorb_qdict() fails, sd_open() closes stdin, because >> sd->fd is still zero. Fortunately, qemu_opts_absorb_qdict() can't >> fail, because: >> >> 1. it only fails when qemu_opt_parse()

Re: [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation

2017-03-02 Thread Stefan Hajnoczi
On Wed, Mar 01, 2017 at 11:50:23AM +, Stefan Hajnoczi wrote: > v3: > * Added Patch 2 to fix invalid test parameters > * Switched to nicer max < avg check [Berto] > v2: > * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto] > * Rephrased warning about guest hangs and errors [Berto] > *

Re: [Qemu-block] [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation

2017-03-02 Thread Stefan Hajnoczi
On Wed, Mar 01, 2017 at 10:27:48PM +0100, Greg Kurz wrote: > On Wed, 1 Mar 2017 11:50:23 + > Stefan Hajnoczi wrote: > > > v3: > > * Added Patch 2 to fix invalid test parameters > > * Switched to nicer max < avg check [Berto] > > v2: > > * Fixed s/bps/iops/ copy-paste

Re: [Qemu-block] [Qemu-devel] [PATCH RFC] block: Tolerate existing writers on read only BdrvChild

2017-03-02 Thread Fam Zheng
On Thu, 03/02 15:23, Kevin Wolf wrote: > Am 02.03.2017 um 12:21 hat Fam Zheng geschrieben: > > On Wed, 03/01 17:22, Kevin Wolf wrote: > > > Am 01.03.2017 um 17:10 hat Fam Zheng geschrieben: > > > > On Wed, 03/01 16:16, Kevin Wolf wrote: > > > > > > I'm not sure about this because: 1) this is

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Philippe Mathieu-Daudé
On 03/02/2017 08:32 PM, Eric Blake wrote: On 03/02/2017 03:43 PM, Markus Armbruster wrote: sd_parse_uri() truncates long VDI names silently. Reject them instead. Signed-off-by: Markus Armbruster --- block/sheepdog.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Philippe Mathieu-Daudé
On 03/02/2017 06:43 PM, Markus Armbruster wrote: sd_parse_uri() truncates long VDI names silently. Reject them instead. Signed-off-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé --- block/sheepdog.c | 4 +++- 1 file changed, 3

Re: [Qemu-block] [Qemu-devel] [PATCH 03/15] sheepdog: Fix error handling sd_create()

2017-03-02 Thread Philippe Mathieu-Daudé
On 03/02/2017 06:43 PM, Markus Armbruster wrote: As a bdrv_create() method, sd_create() must set an error and return negative errno on failure. It prints the error instead of setting it when connect_to_sdog() fails. Fix that. While there, return the value of connect_to_sdog() like we do

Re: [Qemu-block] [Qemu-devel] [PATCH 00/15] block: A bunch of fixes for Sheepdog and Gluster

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > Bad error handling, memory leaks, and lack of blockdev-add support. How hard are we trying to get blockdev-add working in 2.9? Or is this series 2.10 material now? > > Markus Armbruster (15): > sheepdog: Defuse time bomb in sd_open() error

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > sd_parse_uri() truncates long VDI names silently. Reject them > instead. > > Signed-off-by: Markus Armbruster > --- > block/sheepdog.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git

Re: [Qemu-block] [Qemu-devel] [PATCH 05/15] sheepdog: Fix snapshot ID parsing in _open(), _create, _goto()

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > sd_parse_uri() and sd_snapshot_goto() screw up error checking after > strtoul(), and truncate long tag names silently. Fix by replacing > those parts by new sd_parse_snapid_or_tag(), which checks more > carefully. At least we've fixed checkpatch

Re: [Qemu-block] [Qemu-devel] [PATCH 04/15] sheepdog: Mark sd_snapshot_delete() lossage FIXME

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > sd_snapshot_delete() should delete the snapshot whose ID matches > @snapshot_id and whose name matches @name. But that's not what it > does. If @snapshot_id is a valid ID, it deletes the snapshot with > that ID, else it deletes the snapshot with

Re: [Qemu-block] [Qemu-devel] [PATCH 03/15] sheepdog: Fix error handling sd_create()

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > As a bdrv_create() method, sd_create() must set an error and return > negative errno on failure. It prints the error instead of setting it > when connect_to_sdog() fails. Fix that. > > While there, return the value of connect_to_sdog() like we

Re: [Qemu-block] [Qemu-devel] [PATCH 02/15] sheepdog: Fix error handling in sd_snapshot_delete()

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > As a bdrv_snapshot_delete() method, sd_snapshot_delete() must set an > error and return negative errno on failure. It sometimes returns -1, > and sometimes neglects to set an error. It also prints error messages > with error_report(). Fix all

Re: [Qemu-block] [Qemu-devel] [PATCH 01/15] sheepdog: Defuse time bomb in sd_open() error handling

2017-03-02 Thread Eric Blake
On 03/02/2017 03:43 PM, Markus Armbruster wrote: > When qemu_opts_absorb_qdict() fails, sd_open() closes stdin, because > sd->fd is still zero. Fortunately, qemu_opts_absorb_qdict() can't > fail, because: > > 1. it only fails when qemu_opt_parse() fails, and > 2. the only member of

[Qemu-block] [PATCH 13/15] qapi-schema: Rename GlusterServer to SocketAddressFlat

2017-03-02 Thread Markus Armbruster
As its documentation says, it's not specific to Gluster. Rename it, as I'm going to use it for something else. Signed-off-by: Markus Armbruster --- block/gluster.c | 38 +++--- qapi-schema.json | 38

[Qemu-block] [PATCH 09/15] sheepdog: Implement bdrv_parse_filename()

2017-03-02 Thread Markus Armbruster
This permits configuration with driver-specific options in addition to pseudo-filename parsed as URI. For instance, --drive driver=sheepdog,host=fido,vdi=dolly instead of --drive driver=sheepdog,file=sheepdog://fido/dolly It's also a first step towards supporting blockdev-add.

[Qemu-block] [PATCH 11/15] gluster: Don't duplicate qapi-util.c's qapi_enum_parse()

2017-03-02 Thread Markus Armbruster
Signed-off-by: Markus Armbruster --- block/gluster.c | 30 +- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/block/gluster.c b/block/gluster.c index 7236d59..6fbcf9e 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -12,6 +12,7

[Qemu-block] [PATCH 08/15] sheepdog: Use SocketAddress and socket_connect()

2017-03-02 Thread Markus Armbruster
sd_parse_uri() builds a string from host and port parts for inet_connect(). inet_connect() parses it into host, port and options. Whether this gets exactly the same host, port and no options for all inputs is not obvious. Cut out the string middleman and build a SocketAddress for

[Qemu-block] [PATCH 01/15] sheepdog: Defuse time bomb in sd_open() error handling

2017-03-02 Thread Markus Armbruster
When qemu_opts_absorb_qdict() fails, sd_open() closes stdin, because sd->fd is still zero. Fortunately, qemu_opts_absorb_qdict() can't fail, because: 1. it only fails when qemu_opt_parse() fails, and 2. the only member of runtime_opts.desc[] is a QEMU_OPT_STRING, and 3. qemu_opt_parse() can't

[Qemu-block] [PATCH 05/15] sheepdog: Fix snapshot ID parsing in _open(), _create, _goto()

2017-03-02 Thread Markus Armbruster
sd_parse_uri() and sd_snapshot_goto() screw up error checking after strtoul(), and truncate long tag names silently. Fix by replacing those parts by new sd_parse_snapid_or_tag(), which checks more carefully. sd_snapshot_delete() also parses snapshot IDs, but is currently too broken for me to

[Qemu-block] [PATCH 00/15] block: A bunch of fixes for Sheepdog and Gluster

2017-03-02 Thread Markus Armbruster
Bad error handling, memory leaks, and lack of blockdev-add support. Markus Armbruster (15): sheepdog: Defuse time bomb in sd_open() error handling sheepdog: Fix error handling in sd_snapshot_delete() sheepdog: Fix error handling sd_create() sheepdog: Mark sd_snapshot_delete() lossage

[Qemu-block] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Markus Armbruster
sd_parse_uri() truncates long VDI names silently. Reject them instead. Signed-off-by: Markus Armbruster --- block/sheepdog.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index deb110e..72a52a6 100644 ---

[Qemu-block] [PATCH 12/15] gluster: Plug memory leaks in qemu_gluster_parse_json()

2017-03-02 Thread Markus Armbruster
To reproduce, run $ valgrind qemu-system-x86_64 --nodefaults -S --drive driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx Signed-off-by: Markus Armbruster --- block/gluster.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-)

[Qemu-block] [PATCH 04/15] sheepdog: Mark sd_snapshot_delete() lossage FIXME

2017-03-02 Thread Markus Armbruster
sd_snapshot_delete() should delete the snapshot whose ID matches @snapshot_id and whose name matches @name. But that's not what it does. If @snapshot_id is a valid ID, it deletes the snapshot with that ID, else it deletes the snapshot with that name. It doesn't use @name at all. Add suitable

Re: [Qemu-block] [PATCH RFC] block: Tolerate existing writers on read only BdrvChild

2017-03-02 Thread Fam Zheng
On Wed, 03/01 17:22, Kevin Wolf wrote: > Am 01.03.2017 um 17:10 hat Fam Zheng geschrieben: > > On Wed, 03/01 16:16, Kevin Wolf wrote: > > > > I'm not sure about this because: 1) this is intrusive from a user PoV, > > > > many > > > > scripts and upper layer tools will stop working; > > > > > >

Re: [Qemu-block] [Qemu-devel] [PULL 00/46] Block layer patches

2017-03-02 Thread Peter Maydell
On 28 February 2017 at 20:35, Kevin Wolf wrote: > The following changes since commit 9514f2648ca05b38e852b490a12b8cd98d5808c1: > > Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging > (2017-02-28 17:39:49 +) > > are available in the git repository