Re: [Qemu-devel] [PATCH v17 0/4][WIP] block/gluster: add support for multiple gluster servers

[Jeff Cody]

On Wed, Jun 15, 2016 at 01:55:43PM +0530, Prasanna Kumar Kalever wrote:
> This version of patches are rebased on master branch.
> 
> Prasanna Kumar Kalever (4):
>   block/gluster: rename [server, volname, image] -> [host, volume, path]
>   block/gluster: code cleanup
>   block/gluster: using new qapi schema
>   block/gluster: add support for multiple gluster servers
>

I think the main criticism with this series revolves around the interface,
and the overloading of the server hosts fields when using tcp and unix
sockets, etc.  The idea of using flat unions for the API was floated.

Eric, does this criticism still stand, from libvirt's perspective?  Or are
you comfortable enough with the current interface that I can go ahead and
take this series in through my tree?


> v1:
> multiple host addresses but common port number and transport type
> pattern: URI syntax with query (?) delimitor
> syntax:
>     file=gluster[+transport-type]://host1:24007/testvol/a.img\
>          ?server=host2&server=host3
> 
> v2:
> multiple host addresses each have their own port number, but all use
>                                                          common transport type
> pattern: URI syntax  with query (?) delimiter
> syntax:
>     file=gluster[+transport-type]://[host[:port]]/testvol/a.img\
>          [?server=host1[:port]\
>           &server=host2[:port]]
> 
> v3:
> multiple host addresses each have their own port number and transport type
> pattern: changed to json
> syntax:
>     'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
>            "path":"/path/a.qcow2","server":
>          [{"host":"1.2.3.4","port":"24007","transport":"tcp"},
>           {"host":"4.5.6.7","port":"24008","transport":"rdma"}] } }'
> 
> v4, v5:
> address comments from "Eric Blake" <address@hidden>
> renamed:
> 'backup-volfile-servers' -> 'volfile-servers'
> 
> v6:
> address comments from Peter Krempa <address@hidden>
> renamed:
>  'volname'    ->  'volume'
>  'image-path' ->  'path'
>  'server'     ->  'host'
> 
> v7:
> fix for v6 (initialize num_servers to 1 and other typos)
> 
> v8:
> split patch set v7 into series of 3 as per Peter Krempa <address@hidden>
> review comments
> 
> v9:
> reorder the series of patches addressing "Eric Blake" <address@hidden>
> review comments
> 
> v10:
> fix mem-leak as per Peter Krempa <address@hidden> review comments
> 
> v11:
> using qapi-types* defined structures as per "Eric Blake" <address@hidden>
> review comments.
> 
> v12:
> fix crash caused in qapi_free_BlockdevOptionsGluster
> 
> v13:
> address comments from "Jeff Cody" <address@hidden>
> 
> v14:
> address comments from "Eric Blake" <address@hidden>
> split patch 3/3 into two
> rename input option and variable from 'servers' to 'server'
> 
> v15:
> patch 1/4 changed the commit message as per Eric's comment
> patch 2/4 are unchanged
> patch 3/4 addressed Jeff's comments
> patch 4/4 concentrates on unix transport related help info,
> rename 'parse_transport_option()' to 'qapi_enum_parse()',
> address memory leaks and other comments given by Jeff and Eric
> 
> v16:
> In patch 4/4 fixed segfault on glfs_init() error case, as per Jeff's comments
> other patches in this series remain unchanged
> 
> v17:
> rebase of v16 on latest master
> 
>  block/gluster.c      | 484 
> ++++++++++++++++++++++++++++++++++++++-------------
>  qapi/block-core.json |  64 ++++++-
>  2 files changed, 419 insertions(+), 129 deletions(-)
> 
> -- 
> 2.5.5
>