Re: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.

>+ virBufferAsprintf(opt, "gluster+%s://", >+ virDomainDiskProtocolTransportTypeToString(disk->hosts->transport)); >+ >+ /* if transport type is not unix, specify server:port */ >+ if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { >+ if (strstr(disk->hosts->name, ":")) { Wouldn't it be better to check for ':' and check for absence of "." (and vice versa in the else) so that if someone specified a.b.c:d or a:b:c:d:e.f we can throw error rite away, instead of qemu erroring out later in time ? Its a very primtive check but can help to catch user input error early enuf. >+ /* if IPv6 addr, use square brackets to enclose it */ >+ virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name, disk->hosts->port); >+ } else { >+ virBufferAsprintf(opt, "%s:%s", disk->hosts->name, disk->hosts->port); >+ } >+ } >+ >+ /* append source path to gluster disk image */ >+ virBufferAsprintf(opt, "/%s", disk->src); >+ >+ /* if transport type is unix, server name is path to unix socket, ignore port */ >+ if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { >+ virBufferAsprintf(opt, "?socket=%s", disk->hosts->name); >+ } This can be clubbed as part of else clause to the above if condn (if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that ways we avoid an un-necessary check of transport here. It also means that disk->src needs to be pulled inside the if & else clauses, which I feel should be ok.