On Fri, Jul 22, 2016 at 01:50:29PM +0530, Prasanna Kumar Kalever wrote:
> This patch adds support for initialization of gluster backend
> with multiple servers which acts as gluster volfile servers for
> the gluster storage backend.
>
> This will help in achieving high availability of gluster backend
> connectivity via libgfapi i.e. when the first volfile server fails,
> then other servers specified are used as volfile server to re-establish
> the backend gluster connectivity.
>
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@xxxxxxxxxx>
> ---
> src/storage/storage_backend_gluster.c | 100 ++++++++++++++++++++++++----------
> 1 file changed, 71 insertions(+), 29 deletions(-)
>
> diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
> index eda060d..ab61619 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
> @@ -32,6 +32,8 @@
> #include "virstring.h"
> #include "viruri.h"
>
> +#define QEMU_DEFAULT_GLUSTER_PORT 24007
> +
> #define VIR_FROM_THIS VIR_FROM_STORAGE
>
> VIR_LOG_INIT("storage.storage_backend_gluster");
> @@ -573,20 +575,18 @@ static int
> virStorageFileBackendGlusterInit(virStorageSourcePtr src)
> {
> virStorageFileBackendGlusterPrivPtr priv = NULL;
> - virStorageNetHostDefPtr host = &(src->hosts[0]);
> - const char *hostname;
> + const char *transport;
> + bool is_unix = false;
> int port = 0;
> + size_t i = 0;
>
> - if (src->nhosts != 1) {
> + if (src->nhosts < 1) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Expected exactly 1 host for the gluster volume"));
> + _("Expected atleast 1 host for the gluster volume"));
s/atleast/at least/
> return -1;
> }
>
> - hostname = host->name;
> -
> - VIR_DEBUG("initializing gluster storage file %p (gluster://%s:%s/%s%s)[%u:%u]",
> - src, hostname, host->port ? host->port : "0",
> + VIR_DEBUG("Initializing gluster volume=%s path=%s with uid=%u gid=%u",
> NULLSTR(src->volume), src->path,
> (unsigned int)src->drv->uid, (unsigned int)src->drv->gid);
>
> @@ -600,35 +600,77 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
> if (VIR_ALLOC(priv) < 0)
> return -1;
>
> - if (host->port &&
> - virStrToLong_i(host->port, NULL, 10, &port) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("failed to parse port number '%s'"),
> - host->port);
> - goto error;
> - }
> -
> - if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX)
> - hostname = host->socket;
> -
> if (!(priv->vol = glfs_new(src->volume))) {
> virReportOOMError();
> goto error;
> }
>
> - if (glfs_set_volfile_server(priv->vol,
> - virStorageNetHostTransportTypeToString(host->transport),
> - hostname, port) < 0) {
> - virReportSystemError(errno,
> - _("failed to set gluster volfile server '%s'"),
> - hostname);
> - goto error;
> + for (i = 0; i < src->nhosts; i++) {
> +
> + if ((src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP) ||
> + (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX)) {
> + transport = virStorageNetHostTransportTypeToString(src->hosts[i].transport);
> + } else {
> + virReportSystemError(errno,
> + _("gluster only supports tcp|unix not '%s'"),
> + virStorageNetHostTransportTypeToString(src->hosts[i].transport));
> + goto error;
> + }
> +
> +
> + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) {
> + is_unix = true;
> + } else if (!src->hosts[i].port) {
> + port = QEMU_DEFAULT_GLUSTER_PORT;
> + } else {
> + if (virStrToLong_i(src->hosts[i].port, NULL, 10, &port) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to parse port number '%s'"),
> + src->hosts[i].port);
> + goto error;
> + }
Indentation is screwed up here.
> + }
> + if (glfs_set_volfile_server(priv->vol, transport,
> + is_unix ? (src->hosts[i].socket) : (src->hosts[i].name),
The () used here serve no useful purpose.
> + is_unix ? 0 : port) < 0) {
> + if (is_unix) {
> + virReportSystemError(errno,
> + _("failed to set gluster volfile server with "
> + "socket '%s'"), src->hosts[i].socket);
> + } else {
> + virReportSystemError(errno,
> + _("failed to set gluster volfile server with "
> + "host '%s' and port '%s'"), src->hosts[i].name,
> + src->hosts[i].port);
> + }
> + goto error;
> + } else {
> + if (is_unix) {
> + VIR_DEBUG("Successfully set volfile server with socket:'%s' ",
> + src->hosts[i].socket);
> + } else {
> + VIR_DEBUG("Successfully set volfile server with host:'%s' "
> + "port:'%d' transport:'%s'", src->hosts[i].name,
> + port, transport);
> + }
> + }
> + is_unix = false;
> }
>
> if (glfs_init(priv->vol) < 0) {
> - virReportSystemError(errno,
> - _("failed to initialize gluster connection to "
> - "server: '%s'"), hostname);
> + virReportSystemError(errno,
> + _("failed to initialize gluster connection"
> + " for volume %s, path %s with"),
> + NULLSTR(src->volume), src->path);
> + for (i = 0; i < src->nhosts; i++) {
> + if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
> + virReportSystemError(errno, _("host: %s and port %s"),
> + src->hosts[i].name, src->hosts[i].port);
> + } else {
> + virReportSystemError(errno, _("socket %s"),
> + src->hosts[i].socket);
> + }
> + }
This code looks really messed up - any codepath should only call the
virReportXXXXXXError functions *once*, certainly never report
errors in a loop.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list