| Message ID | 1468947453-5433-4-git-send-email-prasanna.kalever@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Prasanna Kumar Kalever <prasanna.kalever@redhat.com> writes: > gluster volfile server fetch happens through unix and/or tcp, it doesn't > support volfile fetch over rdma, the rdma code may actually mislead, > to make sure things do not break, for now we fallback to tcp when requested > for rdma with a warning. > > If you are wondering how this worked all these days, its the gluster libgfapi > code which handles anything other than unix transport as socket/tcp, sad but > true. > > Also gluster doesn't support ipv6 addresses, removing the ipv6 related > comments/docs section > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > --- > block/gluster.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index 40ee852..8a54ad4 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -12,6 +12,7 @@ > #include "block/block_int.h" > #include "qapi/error.h" > #include "qemu/uri.h" > +#include "qemu/error-report.h" > > #define GLUSTER_OPT_FILENAME "filename" > #define GLUSTER_OPT_DEBUG "debug" > @@ -134,12 +135,10 @@ static int parse_volume_options(GlusterConf *gconf, char *path) > * > * 'transport' specifies the transport type used to connect to gluster > * management daemon (glusterd). Valid transport types are > - * tcp, unix and rdma. If a transport type isn't specified, then tcp > - * type is assumed. > + * tcp, unix. If a transport type isn't specified, then tcp type is assumed. > * > * 'host' specifies the host where the volume file specification for > - * the given volume resides. This can be either hostname, ipv4 address > - * or ipv6 address. ipv6 address needs to be within square brackets [ ]. > + * the given volume resides. This can be either hostname, ipv4 address. "either hostname or ipv4 address" Could be touched up on commit, or in a cleanup patch on top. > * If transport type is 'unix', then 'host' field should not be specified. > * The 'socket' field needs to be populated with the path to unix domain > * socket. > @@ -158,11 +157,8 @@ static int parse_volume_options(GlusterConf *gconf, char *path) > * file=gluster://1.2.3.4/testvol/a.img > * file=gluster+tcp://1.2.3.4/testvol/a.img > * file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img > - * file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img > - * file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img > * file=gluster+tcp://host.domain.com:24007/testvol/dir/a.img > * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket > - * file=gluster+rdma://1.2.3.4:24007/testvol/a.img > */ > static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) > { > @@ -185,7 +181,9 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) > gconf->transport = g_strdup("unix"); > is_unix = true; > } else if (!strcmp(uri->scheme, "gluster+rdma")) { > - gconf->transport = g_strdup("rdma"); > + gconf->transport = g_strdup("tcp"); > + error_report("Warning: rdma feature is not supported falling " > + "back to tcp"); "not supported, falling back" Could be touched up on commit, or in a cleanup patch on top. > } else { > ret = -EINVAL; > goto out; > @@ -1048,6 +1046,12 @@ static BlockDriver bdrv_gluster_unix = { > .create_opts = &qemu_gluster_create_opts, > }; > > +/* rdma is deprecated (actually never supported for volfile fetch) > + * lets maintain for the protocol compatibility, to make sure things > + * won't break immediately for now gluster+rdma will fall back to gluster+tcp > + * protocol with Warning > + * TODO: remove gluster+rdma interface support > + */ > static BlockDriver bdrv_gluster_rdma = { > .format_name = "gluster", > .protocol_name = "gluster+rdma", This comment could use some polish, but we can do that on top. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 07/19/2016 10:57 AM, Prasanna Kumar Kalever wrote: Grammar suggestions, can be squashed in by the maintainer (in addition to what Markus pointed out): > gluster volfile server fetch happens through unix and/or tcp, it doesn't > support volfile fetch over rdma, the rdma code may actually mislead, s/rdma, the/rdma. The/ > to make sure things do not break, for now we fallback to tcp when requested s/to make/so to make/ > for rdma with a warning. s/rdma with/rdma, with/ > > If you are wondering how this worked all these days, its the gluster libgfapi > code which handles anything other than unix transport as socket/tcp, sad but > true. > > Also gluster doesn't support ipv6 addresses, removing the ipv6 related > comments/docs section > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > --- > block/gluster.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index 40ee852..8a54ad4 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -12,6 +12,7 @@ > #include "block/block_int.h" > #include "qapi/error.h" > #include "qemu/uri.h" > +#include "qemu/error-report.h" > > #define GLUSTER_OPT_FILENAME "filename" > #define GLUSTER_OPT_DEBUG "debug" > @@ -134,12 +135,10 @@ static int parse_volume_options(GlusterConf *gconf, char *path) > * > * 'transport' specifies the transport type used to connect to gluster > * management daemon (glusterd). Valid transport types are > - * tcp, unix and rdma. If a transport type isn't specified, then tcp > - * type is assumed. > + * tcp, unix. If a transport type isn't specified, then tcp type is assumed. s/tcp, unix/tcp or unix/ > * > * 'host' specifies the host where the volume file specification for > - * the given volume resides. This can be either hostname, ipv4 address > - * or ipv6 address. ipv6 address needs to be within square brackets [ ]. > + * the given volume resides. This can be either hostname, ipv4 address. s/either hostname, ipv4 address/either a hostname or an ipv4 address/ > @@ -1048,6 +1046,12 @@ static BlockDriver bdrv_gluster_unix = { > .create_opts = &qemu_gluster_create_opts, > }; > > +/* rdma is deprecated (actually never supported for volfile fetch) > + * lets maintain for the protocol compatibility, to make sure things s/fetch) lets maintain for/fetch). Let's maintain it for/ > + * won't break immediately for now gluster+rdma will fall back to gluster+tcp s/immediately for now gluster+rdma/immediately. For now, gluster+rdma/ > + * protocol with Warning s/Warning/a warning/ > + * TODO: remove gluster+rdma interface support > + */ > static BlockDriver bdrv_gluster_rdma = { > .format_name = "gluster", > .protocol_name = "gluster+rdma", > Reviewed-by: Eric Blake <eblake@redhat.com>
On Tue, Jul 19, 2016 at 10:27:31PM +0530, Prasanna Kumar Kalever wrote: > gluster volfile server fetch happens through unix and/or tcp, it doesn't > support volfile fetch over rdma, the rdma code may actually mislead, > to make sure things do not break, for now we fallback to tcp when requested > for rdma with a warning. > > If you are wondering how this worked all these days, its the gluster libgfapi > code which handles anything other than unix transport as socket/tcp, sad but > true. > > Also gluster doesn't support ipv6 addresses, removing the ipv6 related > comments/docs section > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > --- > block/gluster.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index 40ee852..8a54ad4 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -12,6 +12,7 @@ > #include "block/block_int.h" > #include "qapi/error.h" > #include "qemu/uri.h" > +#include "qemu/error-report.h" > > #define GLUSTER_OPT_FILENAME "filename" > #define GLUSTER_OPT_DEBUG "debug" > @@ -134,12 +135,10 @@ static int parse_volume_options(GlusterConf *gconf, char *path) > * > * 'transport' specifies the transport type used to connect to gluster > * management daemon (glusterd). Valid transport types are > - * tcp, unix and rdma. If a transport type isn't specified, then tcp > - * type is assumed. > + * tcp, unix. If a transport type isn't specified, then tcp type is assumed. > * > * 'host' specifies the host where the volume file specification for > - * the given volume resides. This can be either hostname, ipv4 address > - * or ipv6 address. ipv6 address needs to be within square brackets [ ]. > + * the given volume resides. This can be either hostname, ipv4 address. > * If transport type is 'unix', then 'host' field should not be specified. > * The 'socket' field needs to be populated with the path to unix domain > * socket. > @@ -158,11 +157,8 @@ static int parse_volume_options(GlusterConf *gconf, char *path) > * file=gluster://1.2.3.4/testvol/a.img > * file=gluster+tcp://1.2.3.4/testvol/a.img > * file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img > - * file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img > - * file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img > * file=gluster+tcp://host.domain.com:24007/testvol/dir/a.img > * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket > - * file=gluster+rdma://1.2.3.4:24007/testvol/a.img > */ > static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) > { > @@ -185,7 +181,9 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) > gconf->transport = g_strdup("unix"); > is_unix = true; > } else if (!strcmp(uri->scheme, "gluster+rdma")) { > - gconf->transport = g_strdup("rdma"); > + gconf->transport = g_strdup("tcp"); > + error_report("Warning: rdma feature is not supported falling " > + "back to tcp"); > } else { > ret = -EINVAL; > goto out; > @@ -1048,6 +1046,12 @@ static BlockDriver bdrv_gluster_unix = { > .create_opts = &qemu_gluster_create_opts, > }; > > +/* rdma is deprecated (actually never supported for volfile fetch) > + * lets maintain for the protocol compatibility, to make sure things > + * won't break immediately for now gluster+rdma will fall back to gluster+tcp > + * protocol with Warning > + * TODO: remove gluster+rdma interface support > + */ > static BlockDriver bdrv_gluster_rdma = { > .format_name = "gluster", > .protocol_name = "gluster+rdma", > -- > 2.7.4 > > Modulo Markus's and Eric's comments: Reviewed-by: Jeff Cody <jcody@redhat.com> (I'll make the comment changes on commit)
diff --git a/block/gluster.c b/block/gluster.c index 40ee852..8a54ad4 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -12,6 +12,7 @@ #include "block/block_int.h" #include "qapi/error.h" #include "qemu/uri.h" +#include "qemu/error-report.h" #define GLUSTER_OPT_FILENAME "filename" #define GLUSTER_OPT_DEBUG "debug" @@ -134,12 +135,10 @@ static int parse_volume_options(GlusterConf *gconf, char *path) * * 'transport' specifies the transport type used to connect to gluster * management daemon (glusterd). Valid transport types are - * tcp, unix and rdma. If a transport type isn't specified, then tcp - * type is assumed. + * tcp, unix. If a transport type isn't specified, then tcp type is assumed. * * 'host' specifies the host where the volume file specification for - * the given volume resides. This can be either hostname, ipv4 address - * or ipv6 address. ipv6 address needs to be within square brackets [ ]. + * the given volume resides. This can be either hostname, ipv4 address. * If transport type is 'unix', then 'host' field should not be specified. * The 'socket' field needs to be populated with the path to unix domain * socket. @@ -158,11 +157,8 @@ static int parse_volume_options(GlusterConf *gconf, char *path) * file=gluster://1.2.3.4/testvol/a.img * file=gluster+tcp://1.2.3.4/testvol/a.img * file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img - * file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img - * file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img * file=gluster+tcp://host.domain.com:24007/testvol/dir/a.img * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket - * file=gluster+rdma://1.2.3.4:24007/testvol/a.img */ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) { @@ -185,7 +181,9 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) gconf->transport = g_strdup("unix"); is_unix = true; } else if (!strcmp(uri->scheme, "gluster+rdma")) { - gconf->transport = g_strdup("rdma"); + gconf->transport = g_strdup("tcp"); + error_report("Warning: rdma feature is not supported falling " + "back to tcp"); } else { ret = -EINVAL; goto out; @@ -1048,6 +1046,12 @@ static BlockDriver bdrv_gluster_unix = { .create_opts = &qemu_gluster_create_opts, }; +/* rdma is deprecated (actually never supported for volfile fetch) + * lets maintain for the protocol compatibility, to make sure things + * won't break immediately for now gluster+rdma will fall back to gluster+tcp + * protocol with Warning + * TODO: remove gluster+rdma interface support + */ static BlockDriver bdrv_gluster_rdma = { .format_name = "gluster", .protocol_name = "gluster+rdma",
gluster volfile server fetch happens through unix and/or tcp, it doesn't support volfile fetch over rdma, the rdma code may actually mislead, to make sure things do not break, for now we fallback to tcp when requested for rdma with a warning. If you are wondering how this worked all these days, its the gluster libgfapi code which handles anything other than unix transport as socket/tcp, sad but true. Also gluster doesn't support ipv6 addresses, removing the ipv6 related comments/docs section Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- block/gluster.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)