[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH 10/15] gluster: Drop assumptions on
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH 10/15] gluster: Drop assumptions on SocketTransport names |
Date: |
Fri, 03 Mar 2017 08:31:54 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Niels de Vos <address@hidden> 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 artificial. Use the
>> appropriate literal strings instead.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> block/gluster.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/gluster.c b/block/gluster.c
>> index 56b4abe..7236d59 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -412,8 +412,7 @@ static struct glfs
>> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
>>
>> for (server = gconf->server; server; server = server->next) {
>> if (server->value->type == GLUSTER_TRANSPORT_UNIX) {
>> - ret = glfs_set_volfile_server(glfs,
>> -
>> GlusterTransport_lookup[server->value->type],
>> + ret = glfs_set_volfile_server(glfs, "unix",
>> server->value->u.q_unix.path, 0);
>> } else {
>> if (parse_uint_full(server->value->u.tcp.port, &port, 10) < 0 ||
>> @@ -423,8 +422,7 @@ static struct glfs
>> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
>> errno = EINVAL;
>> goto out;
>> }
>> - ret = glfs_set_volfile_server(glfs,
>> -
>> GlusterTransport_lookup[server->value->type],
>> + ret = glfs_set_volfile_server(glfs, "tcp",
>> server->value->u.tcp.host,
>> (int)port);
>> }
>> --
>> 2.7.4
>
> Instead of the strings for "unix" and "tcp", I would have liked
> #define's. Unfortunately it seems that these are not available in public
> headers :-/
Symbols from glfs.h would be ideal, yes. But well-known string literals
aren't too bad.
> If this is easier to understand, I don't have any objections.
It's easier to read, but that's not my chief reason. My chief reason is
that the values glfs_set_volfile_server() accepts are glfs's business,
while the names of the QAPI enumeration constants are QMP's business.
Coupling the two is inappropriate.
Decoupling them enables the renaming of the enumeration constant in
PATCH 14.
> Reviewed-by: Niels de Vos <address@hidden>
Thanks!
- [Qemu-devel] [PATCH 00/15] block: A bunch of fixes for Sheepdog and Gluster, Markus Armbruster, 2017/03/02
- [Qemu-devel] [PATCH 11/15] gluster: Don't duplicate qapi-util.c's qapi_enum_parse(), Markus Armbruster, 2017/03/02
- [Qemu-devel] [PATCH 03/15] sheepdog: Fix error handling sd_create(), Markus Armbruster, 2017/03/02
- [Qemu-devel] [PATCH 10/15] gluster: Drop assumptions on SocketTransport names, Markus Armbruster, 2017/03/02
- [Qemu-devel] [PATCH 04/15] sheepdog: Mark sd_snapshot_delete() lossage FIXME, Markus Armbruster, 2017/03/02
- [Qemu-devel] [PATCH 01/15] sheepdog: Defuse time bomb in sd_open() error handling, Markus Armbruster, 2017/03/02
- [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create(), Markus Armbruster, 2017/03/02