All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v9 0/4] GlusterFS support in QEMU - v9
@ 2012-09-24  9:10 Bharata B Rao
  2012-09-24  9:10 ` [Qemu-devel] [PATCH v9 1/4] aio: Fix qemu_aio_wait() to maintain correct walking_handlers count Bharata B Rao
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Bharata B Rao @ 2012-09-24  9:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	Daniel Veillard, Blue Swirl, Avi Kivity, Paolo Bonzini

Hi,

This is v9 of the patchset to support GlusterFS backend from QEMU.

Changes in v9
-------------
- Drop all inet_parse related patches from the patchset.
- Include generic URI parsing code from libxml2 and libvirt in QEMU and
  use that in gluster block backend instead of private URI parsing
  implementation within gluster.

Previous versions can be found here:

v8 - http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg04007.html
v7 - http://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02532.html
v6 - http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg01536.html
v5 - http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg01023.html
v4 - http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg00147.html
v3 - http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg03322.html
v2 - http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02718.html
v1 - http://lists.nongnu.org/archive/html/qemu-devel/2012-06/msg01745.html

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v9 1/4] aio: Fix qemu_aio_wait() to maintain correct walking_handlers count
  2012-09-24  9:10 [Qemu-devel] [PATCH v9 0/4] GlusterFS support in QEMU - v9 Bharata B Rao
@ 2012-09-24  9:10 ` Bharata B Rao
  2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library Bharata B Rao
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Bharata B Rao @ 2012-09-24  9:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	Daniel Veillard, Blue Swirl, Avi Kivity, Paolo Bonzini

aio: Fix qemu_aio_wait() to maintain correct walking_handlers count

From: Paolo Bonzini <pbonzini@redhat.com>

Fix qemu_aio_wait() to ensure that registered aio handlers don't get
deleted when they are still active. This is ensured by maintaning the
right count of walking_handlers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---

 aio.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)


diff --git a/aio.c b/aio.c
index 0a9eb10..99b8b72 100644
--- a/aio.c
+++ b/aio.c
@@ -119,7 +119,7 @@ bool qemu_aio_wait(void)
         return true;
     }
 
-    walking_handlers = 1;
+    walking_handlers++;
 
     FD_ZERO(&rdfds);
     FD_ZERO(&wrfds);
@@ -147,7 +147,7 @@ bool qemu_aio_wait(void)
         }
     }
 
-    walking_handlers = 0;
+    walking_handlers--;
 
     /* No AIO operations?  Get us out of here */
     if (!busy) {
@@ -159,7 +159,7 @@ bool qemu_aio_wait(void)
 
     /* if we have any readable fds, dispatch event */
     if (ret > 0) {
-        walking_handlers = 1;
+        walking_handlers++;
 
         /* we have to walk very carefully in case
          * qemu_aio_set_fd_handler is called while we're walking */
@@ -187,7 +187,7 @@ bool qemu_aio_wait(void)
             }
         }
 
-        walking_handlers = 0;
+        walking_handlers--;
     }
 
     return true;

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-09-24  9:10 [Qemu-devel] [PATCH v9 0/4] GlusterFS support in QEMU - v9 Bharata B Rao
  2012-09-24  9:10 ` [Qemu-devel] [PATCH v9 1/4] aio: Fix qemu_aio_wait() to maintain correct walking_handlers count Bharata B Rao
@ 2012-09-24  9:12 ` Bharata B Rao
  2012-09-24 10:27   ` Richard W.M. Jones
  2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 3/4] configure: Add a config option for GlusterFS as block backend Bharata B Rao
  2012-09-24  9:13 ` [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU " Bharata B Rao
  3 siblings, 1 reply; 26+ messages in thread
From: Bharata B Rao @ 2012-09-24  9:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	Daniel Veillard, Blue Swirl, Avi Kivity, Paolo Bonzini

qemu: URI parsing library

From: Paolo Bonzini <pbonzini@redhat.com>

Add a new URI parsing library to QEMU. The code has been borrowed from
libxml2 and libvirt.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---

 Makefile.objs |    2 
 uri.c         | 2249 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 uri.h         |  113 +++
 3 files changed, 2363 insertions(+), 1 deletions(-)
 create mode 100644 uri.c
 create mode 100644 uri.h


diff --git a/Makefile.objs b/Makefile.objs
index 4412757..7c1c682 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -42,7 +42,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o
-block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o
+block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o uri.o
 block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/uri.c b/uri.c
new file mode 100644
index 0000000..dd922de
--- /dev/null
+++ b/uri.c
@@ -0,0 +1,2249 @@
+/**
+ * uri.c: set of generic URI related routines
+ *
+ * Reference: RFCs 3986, 2732 and 2373
+ *
+ * Copyright (C) 1998-2003 Daniel Veillard.  All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
+ * DANIEL VEILLARD BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Except as contained in this notice, the name of Daniel Veillard shall not
+ * be used in advertising or otherwise to promote the sale, use or other
+ * dealings in this Software without prior written authorization from him.
+ *
+ * daniel@veillard.com
+ *
+ **
+ *
+ * Copyright (C) 2007, 2009-2010 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Authors:
+ *    Richard W.M. Jones <rjones@redhat.com>
+ *
+ */
+
+#include <glib.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "uri.h"
+
+static void uri_clean(URI *uri);
+
+/*
+ * Old rule from 2396 used in legacy handling code
+ * alpha    = lowalpha | upalpha
+ */
+#define IS_ALPHA(x) (IS_LOWALPHA(x) || IS_UPALPHA(x))
+
+
+/*
+ * lowalpha = "a" | "b" | "c" | "d" | "e" | "f" | "g" | "h" | "i" | "j" |
+ *            "k" | "l" | "m" | "n" | "o" | "p" | "q" | "r" | "s" | "t" |
+ *            "u" | "v" | "w" | "x" | "y" | "z"
+ */
+
+#define IS_LOWALPHA(x) (((x) >= 'a') && ((x) <= 'z'))
+
+/*
+ * upalpha = "A" | "B" | "C" | "D" | "E" | "F" | "G" | "H" | "I" | "J" |
+ *           "K" | "L" | "M" | "N" | "O" | "P" | "Q" | "R" | "S" | "T" |
+ *           "U" | "V" | "W" | "X" | "Y" | "Z"
+ */
+#define IS_UPALPHA(x) (((x) >= 'A') && ((x) <= 'Z'))
+
+#ifdef IS_DIGIT
+#undef IS_DIGIT
+#endif
+/*
+ * digit = "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9"
+ */
+#define IS_DIGIT(x) (((x) >= '0') && ((x) <= '9'))
+
+/*
+ * alphanum = alpha | digit
+ */
+
+#define IS_ALPHANUM(x) (IS_ALPHA(x) || IS_DIGIT(x))
+
+/*
+ * mark = "-" | "_" | "." | "!" | "~" | "*" | "'" | "(" | ")"
+ */
+
+#define IS_MARK(x) (((x) == '-') || ((x) == '_') || ((x) == '.') ||     \
+    ((x) == '!') || ((x) == '~') || ((x) == '*') || ((x) == '\'') ||    \
+    ((x) == '(') || ((x) == ')'))
+
+/*
+ * unwise = "{" | "}" | "|" | "\" | "^" | "`"
+ */
+
+#define IS_UNWISE(p)                                                    \
+      (((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) ||         \
+       ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) ||        \
+       ((*(p) == ']')) || ((*(p) == '`')))
+/*
+ * reserved = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" | "$" | "," |
+ *            "[" | "]"
+ */
+
+#define IS_RESERVED(x) (((x) == ';') || ((x) == '/') || ((x) == '?') || \
+        ((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') || \
+        ((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') || \
+        ((x) == ']'))
+
+/*
+ * unreserved = alphanum | mark
+ */
+
+#define IS_UNRESERVED(x) (IS_ALPHANUM(x) || IS_MARK(x))
+
+/*
+ * Skip to next pointer char, handle escaped sequences
+ */
+
+#define NEXT(p) ((*p == '%')? p += 3 : p++)
+
+/*
+ * Productions from the spec.
+ *
+ *    authority     = server | reg_name
+ *    reg_name      = 1*( unreserved | escaped | "$" | "," |
+ *                        ";" | ":" | "@" | "&" | "=" | "+" )
+ *
+ * path          = [ abs_path | opaque_part ]
+ */
+
+
+/************************************************************************
+ *									*
+ *                         RFC 3986 parser				*
+ *									*
+ ************************************************************************/
+
+#define ISA_DIGIT(p) ((*(p) >= '0') && (*(p) <= '9'))
+#define ISA_ALPHA(p) (((*(p) >= 'a') && (*(p) <= 'z')) ||		\
+                      ((*(p) >= 'A') && (*(p) <= 'Z')))
+#define ISA_HEXDIG(p)							\
+       (ISA_DIGIT(p) || ((*(p) >= 'a') && (*(p) <= 'f')) ||		\
+        ((*(p) >= 'A') && (*(p) <= 'F')))
+
+/*
+ *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
+ *                     / "*" / "+" / "," / ";" / "="
+ */
+#define ISA_SUB_DELIM(p)						\
+      (((*(p) == '!')) || ((*(p) == '$')) || ((*(p) == '&')) ||		\
+       ((*(p) == '(')) || ((*(p) == ')')) || ((*(p) == '*')) ||		\
+       ((*(p) == '+')) || ((*(p) == ',')) || ((*(p) == ';')) ||		\
+       ((*(p) == '=')) || ((*(p) == '\'')))
+
+/*
+ *    gen-delims    = ":" / "/" / "?" / "#" / "[" / "]" / "@"
+ */
+#define ISA_GEN_DELIM(p)						\
+      (((*(p) == ':')) || ((*(p) == '/')) || ((*(p) == '?')) ||         \
+       ((*(p) == '#')) || ((*(p) == '[')) || ((*(p) == ']')) ||         \
+       ((*(p) == '@')))
+
+/*
+ *    reserved      = gen-delims / sub-delims
+ */
+#define ISA_RESERVED(p) (ISA_GEN_DELIM(p) || (ISA_SUB_DELIM(p)))
+
+/*
+ *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
+ */
+#define ISA_UNRESERVED(p)						\
+      ((ISA_ALPHA(p)) || (ISA_DIGIT(p)) || ((*(p) == '-')) ||		\
+       ((*(p) == '.')) || ((*(p) == '_')) || ((*(p) == '~')))
+
+/*
+ *    pct-encoded   = "%" HEXDIG HEXDIG
+ */
+#define ISA_PCT_ENCODED(p)						\
+     ((*(p) == '%') && (ISA_HEXDIG(p + 1)) && (ISA_HEXDIG(p + 2)))
+
+/*
+ *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
+ */
+#define ISA_PCHAR(p)							\
+     (ISA_UNRESERVED(p) || ISA_PCT_ENCODED(p) || ISA_SUB_DELIM(p) ||	\
+      ((*(p) == ':')) || ((*(p) == '@')))
+
+/**
+ * rfc3986_parse_scheme:
+ * @uri:  pointer to an URI structure
+ * @str:  pointer to the string to analyze
+ *
+ * Parse an URI scheme
+ *
+ * ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_scheme(URI *uri, const char **str) {
+    const char *cur;
+
+    if (str == NULL)
+	return(-1);
+
+    cur = *str;
+    if (!ISA_ALPHA(cur))
+	return(2);
+    cur++;
+    while (ISA_ALPHA(cur) || ISA_DIGIT(cur) ||
+           (*cur == '+') || (*cur == '-') || (*cur == '.')) cur++;
+    if (uri != NULL) {
+	if (uri->scheme != NULL) g_free(uri->scheme);
+	uri->scheme = g_strndup(*str, cur - *str);
+    }
+    *str = cur;
+    return(0);
+}
+
+/**
+ * rfc3986_parse_fragment:
+ * @uri:  pointer to an URI structure
+ * @str:  pointer to the string to analyze
+ *
+ * Parse the query part of an URI
+ *
+ * fragment      = *( pchar / "/" / "?" )
+ * NOTE: the strict syntax as defined by 3986 does not allow '[' and ']'
+ *       in the fragment identifier but this is used very broadly for
+ *       xpointer scheme selection, so we are allowing it here to not break
+ *       for example all the DocBook processing chains.
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_fragment(URI *uri, const char **str)
+{
+    const char *cur;
+
+    if (str == NULL)
+        return (-1);
+
+    cur = *str;
+
+    while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
+           (*cur == '[') || (*cur == ']') ||
+           ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur))))
+        NEXT(cur);
+    if (uri != NULL) {
+        if (uri->fragment != NULL)
+            g_free(uri->fragment);
+	if (uri->cleanup & 2)
+	    uri->fragment = g_strndup(*str, cur - *str);
+	else
+	    uri->fragment = uri_string_unescape(*str, cur - *str, NULL);
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_query:
+ * @uri:  pointer to an URI structure
+ * @str:  pointer to the string to analyze
+ *
+ * Parse the query part of an URI
+ *
+ * query = *uric
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_query(URI *uri, const char **str)
+{
+    const char *cur;
+
+    if (str == NULL)
+        return (-1);
+
+    cur = *str;
+
+    while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
+           ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur))))
+        NEXT(cur);
+    if (uri != NULL) {
+	if (uri->query != NULL)
+	    g_free (uri->query);
+	uri->query = g_strndup (*str, cur - *str);
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_port:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse a port  part and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * port          = *DIGIT
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_port(URI *uri, const char **str)
+{
+    const char *cur = *str;
+
+    if (ISA_DIGIT(cur)) {
+	if (uri != NULL)
+	    uri->port = 0;
+	while (ISA_DIGIT(cur)) {
+	    if (uri != NULL)
+		uri->port = uri->port * 10 + (*cur - '0');
+	    cur++;
+	}
+	*str = cur;
+	return(0);
+    }
+    return(1);
+}
+
+/**
+ * rfc3986_parse_user_info:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an user informations part and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * userinfo      = *( unreserved / pct-encoded / sub-delims / ":" )
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_user_info(URI *uri, const char **str)
+{
+    const char *cur;
+
+    cur = *str;
+    while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) ||
+           ISA_SUB_DELIM(cur) || (*cur == ':'))
+	NEXT(cur);
+    if (*cur == '@') {
+	if (uri != NULL) {
+	    if (uri->user != NULL) g_free(uri->user);
+	    if (uri->cleanup & 2)
+		uri->user = g_strndup(*str, cur - *str);
+	    else
+		uri->user = uri_string_unescape(*str, cur - *str, NULL);
+	}
+	*str = cur;
+	return(0);
+    }
+    return(1);
+}
+
+/**
+ * rfc3986_parse_dec_octet:
+ * @str:  the string to analyze
+ *
+ *    dec-octet     = DIGIT                 ; 0-9
+ *                  / %x31-39 DIGIT         ; 10-99
+ *                  / "1" 2DIGIT            ; 100-199
+ *                  / "2" %x30-34 DIGIT     ; 200-249
+ *                  / "25" %x30-35          ; 250-255
+ *
+ * Skip a dec-octet.
+ *
+ * Returns 0 if found and skipped, 1 otherwise
+ */
+static int
+rfc3986_parse_dec_octet(const char **str) {
+    const char *cur = *str;
+
+    if (!(ISA_DIGIT(cur)))
+        return(1);
+    if (!ISA_DIGIT(cur+1))
+	cur++;
+    else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur+2)))
+	cur += 2;
+    else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2)))
+	cur += 3;
+    else if ((*cur == '2') && (*(cur + 1) >= '0') &&
+	     (*(cur + 1) <= '4') && (ISA_DIGIT(cur + 2)))
+	cur += 3;
+    else if ((*cur == '2') && (*(cur + 1) == '5') &&
+	     (*(cur + 2) >= '0') && (*(cur + 1) <= '5'))
+	cur += 3;
+    else
+        return(1);
+    *str = cur;
+    return(0);
+}
+/**
+ * rfc3986_parse_host:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an host part and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * host          = IP-literal / IPv4address / reg-name
+ * IP-literal    = "[" ( IPv6address / IPvFuture  ) "]"
+ * IPv4address   = dec-octet "." dec-octet "." dec-octet "." dec-octet
+ * reg-name      = *( unreserved / pct-encoded / sub-delims )
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_host(URI *uri, const char **str)
+{
+    const char *cur = *str;
+    const char *host;
+
+    host = cur;
+    /*
+     * IPv6 and future adressing scheme are enclosed between brackets
+     */
+    if (*cur == '[') {
+        cur++;
+	while ((*cur != ']') && (*cur != 0))
+	    cur++;
+	if (*cur != ']')
+	    return(1);
+	cur++;
+	goto found;
+    }
+    /*
+     * try to parse an IPv4
+     */
+    if (ISA_DIGIT(cur)) {
+        if (rfc3986_parse_dec_octet(&cur) != 0)
+	    goto not_ipv4;
+	if (*cur != '.')
+	    goto not_ipv4;
+	cur++;
+        if (rfc3986_parse_dec_octet(&cur) != 0)
+	    goto not_ipv4;
+	if (*cur != '.')
+	    goto not_ipv4;
+        if (rfc3986_parse_dec_octet(&cur) != 0)
+	    goto not_ipv4;
+	if (*cur != '.')
+	    goto not_ipv4;
+        if (rfc3986_parse_dec_octet(&cur) != 0)
+	    goto not_ipv4;
+	goto found;
+not_ipv4:
+        cur = *str;
+    }
+    /*
+     * then this should be a hostname which can be empty
+     */
+    while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) || ISA_SUB_DELIM(cur))
+        NEXT(cur);
+found:
+    if (uri != NULL) {
+	if (uri->authority != NULL) g_free(uri->authority);
+	uri->authority = NULL;
+	if (uri->server != NULL) g_free(uri->server);
+	if (cur != host) {
+	    if (uri->cleanup & 2)
+		uri->server = g_strndup(host, cur - host);
+	    else
+		uri->server = uri_string_unescape(host, cur - host, NULL);
+	} else
+	    uri->server = NULL;
+    }
+    *str = cur;
+    return(0);
+}
+
+/**
+ * rfc3986_parse_authority:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an authority part and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * authority     = [ userinfo "@" ] host [ ":" port ]
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_authority(URI *uri, const char **str)
+{
+    const char *cur;
+    int ret;
+
+    cur = *str;
+    /*
+     * try to parse an userinfo and check for the trailing @
+     */
+    ret = rfc3986_parse_user_info(uri, &cur);
+    if ((ret != 0) || (*cur != '@'))
+        cur = *str;
+    else
+        cur++;
+    ret = rfc3986_parse_host(uri, &cur);
+    if (ret != 0) return(ret);
+    if (*cur == ':') {
+        cur++;
+        ret = rfc3986_parse_port(uri, &cur);
+	if (ret != 0) return(ret);
+    }
+    *str = cur;
+    return(0);
+}
+
+/**
+ * rfc3986_parse_segment:
+ * @str:  the string to analyze
+ * @forbid: an optional forbidden character
+ * @empty: allow an empty segment
+ *
+ * Parse a segment and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * segment       = *pchar
+ * segment-nz    = 1*pchar
+ * segment-nz-nc = 1*( unreserved / pct-encoded / sub-delims / "@" )
+ *               ; non-zero-length segment without any colon ":"
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_segment(const char **str, char forbid, int empty)
+{
+    const char *cur;
+
+    cur = *str;
+    if (!ISA_PCHAR(cur)) {
+        if (empty)
+	    return(0);
+	return(1);
+    }
+    while (ISA_PCHAR(cur) && (*cur != forbid))
+        NEXT(cur);
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_path_ab_empty:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an path absolute or empty and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * path-abempty  = *( "/" segment )
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_path_ab_empty(URI *uri, const char **str)
+{
+    const char *cur;
+    int ret;
+
+    cur = *str;
+
+    while (*cur == '/') {
+        cur++;
+	ret = rfc3986_parse_segment(&cur, 0, 1);
+	if (ret != 0) return(ret);
+    }
+    if (uri != NULL) {
+	if (uri->path != NULL) g_free(uri->path);
+        if (*str != cur) {
+            if (uri->cleanup & 2)
+                uri->path = g_strndup(*str, cur - *str);
+            else
+                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+        } else {
+            uri->path = NULL;
+        }
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_path_absolute:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an path absolute and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * path-absolute = "/" [ segment-nz *( "/" segment ) ]
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_path_absolute(URI *uri, const char **str)
+{
+    const char *cur;
+    int ret;
+
+    cur = *str;
+
+    if (*cur != '/')
+        return(1);
+    cur++;
+    ret = rfc3986_parse_segment(&cur, 0, 0);
+    if (ret == 0) {
+	while (*cur == '/') {
+	    cur++;
+	    ret = rfc3986_parse_segment(&cur, 0, 1);
+	    if (ret != 0) return(ret);
+	}
+    }
+    if (uri != NULL) {
+	if (uri->path != NULL) g_free(uri->path);
+        if (cur != *str) {
+            if (uri->cleanup & 2)
+                uri->path = g_strndup(*str, cur - *str);
+            else
+                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+        } else {
+            uri->path = NULL;
+        }
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_path_rootless:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an path without root and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * path-rootless = segment-nz *( "/" segment )
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_path_rootless(URI *uri, const char **str)
+{
+    const char *cur;
+    int ret;
+
+    cur = *str;
+
+    ret = rfc3986_parse_segment(&cur, 0, 0);
+    if (ret != 0) return(ret);
+    while (*cur == '/') {
+        cur++;
+	ret = rfc3986_parse_segment(&cur, 0, 1);
+	if (ret != 0) return(ret);
+    }
+    if (uri != NULL) {
+	if (uri->path != NULL) g_free(uri->path);
+        if (cur != *str) {
+            if (uri->cleanup & 2)
+                uri->path = g_strndup(*str, cur - *str);
+            else
+                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+        } else {
+            uri->path = NULL;
+        }
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_path_no_scheme:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an path which is not a scheme and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * path-noscheme = segment-nz-nc *( "/" segment )
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_path_no_scheme(URI *uri, const char **str)
+{
+    const char *cur;
+    int ret;
+
+    cur = *str;
+
+    ret = rfc3986_parse_segment(&cur, ':', 0);
+    if (ret != 0) return(ret);
+    while (*cur == '/') {
+        cur++;
+	ret = rfc3986_parse_segment(&cur, 0, 1);
+	if (ret != 0) return(ret);
+    }
+    if (uri != NULL) {
+	if (uri->path != NULL) g_free(uri->path);
+        if (cur != *str) {
+            if (uri->cleanup & 2)
+                uri->path = g_strndup(*str, cur - *str);
+            else
+                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+        } else {
+            uri->path = NULL;
+        }
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_hier_part:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an hierarchical part and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * hier-part     = "//" authority path-abempty
+ *                / path-absolute
+ *                / path-rootless
+ *                / path-empty
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_hier_part(URI *uri, const char **str)
+{
+    const char *cur;
+    int ret;
+
+    cur = *str;
+
+    if ((*cur == '/') && (*(cur + 1) == '/')) {
+        cur += 2;
+	ret = rfc3986_parse_authority(uri, &cur);
+	if (ret != 0) return(ret);
+	ret = rfc3986_parse_path_ab_empty(uri, &cur);
+	if (ret != 0) return(ret);
+	*str = cur;
+	return(0);
+    } else if (*cur == '/') {
+        ret = rfc3986_parse_path_absolute(uri, &cur);
+	if (ret != 0) return(ret);
+    } else if (ISA_PCHAR(cur)) {
+        ret = rfc3986_parse_path_rootless(uri, &cur);
+	if (ret != 0) return(ret);
+    } else {
+	/* path-empty is effectively empty */
+	if (uri != NULL) {
+	    if (uri->path != NULL) g_free(uri->path);
+	    uri->path = NULL;
+	}
+    }
+    *str = cur;
+    return (0);
+}
+
+/**
+ * rfc3986_parse_relative_ref:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an URI string and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * relative-ref  = relative-part [ "?" query ] [ "#" fragment ]
+ * relative-part = "//" authority path-abempty
+ *               / path-absolute
+ *               / path-noscheme
+ *               / path-empty
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_relative_ref(URI *uri, const char *str) {
+    int ret;
+
+    if ((*str == '/') && (*(str + 1) == '/')) {
+        str += 2;
+	ret = rfc3986_parse_authority(uri, &str);
+	if (ret != 0) return(ret);
+	ret = rfc3986_parse_path_ab_empty(uri, &str);
+	if (ret != 0) return(ret);
+    } else if (*str == '/') {
+	ret = rfc3986_parse_path_absolute(uri, &str);
+	if (ret != 0) return(ret);
+    } else if (ISA_PCHAR(str)) {
+        ret = rfc3986_parse_path_no_scheme(uri, &str);
+	if (ret != 0) return(ret);
+    } else {
+	/* path-empty is effectively empty */
+	if (uri != NULL) {
+	    if (uri->path != NULL) g_free(uri->path);
+	    uri->path = NULL;
+	}
+    }
+
+    if (*str == '?') {
+	str++;
+	ret = rfc3986_parse_query(uri, &str);
+	if (ret != 0) return(ret);
+    }
+    if (*str == '#') {
+	str++;
+	ret = rfc3986_parse_fragment(uri, &str);
+	if (ret != 0) return(ret);
+    }
+    if (*str != 0) {
+	uri_clean(uri);
+	return(1);
+    }
+    return(0);
+}
+
+
+/**
+ * rfc3986_parse:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an URI string and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * scheme ":" hier-part [ "?" query ] [ "#" fragment ]
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse(URI *uri, const char *str) {
+    int ret;
+
+    ret = rfc3986_parse_scheme(uri, &str);
+    if (ret != 0) return(ret);
+    if (*str != ':') {
+	return(1);
+    }
+    str++;
+    ret = rfc3986_parse_hier_part(uri, &str);
+    if (ret != 0) return(ret);
+    if (*str == '?') {
+	str++;
+	ret = rfc3986_parse_query(uri, &str);
+	if (ret != 0) return(ret);
+    }
+    if (*str == '#') {
+	str++;
+	ret = rfc3986_parse_fragment(uri, &str);
+	if (ret != 0) return(ret);
+    }
+    if (*str != 0) {
+	uri_clean(uri);
+	return(1);
+    }
+    return(0);
+}
+
+/**
+ * rfc3986_parse_uri_reference:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an URI reference string and fills in the appropriate fields
+ * of the @uri structure
+ *
+ * URI-reference = URI / relative-ref
+ *
+ * Returns 0 or the error code
+ */
+static int
+rfc3986_parse_uri_reference(URI *uri, const char *str) {
+    int ret;
+
+    if (str == NULL)
+	return(-1);
+    uri_clean(uri);
+
+    /*
+     * Try first to parse absolute refs, then fallback to relative if
+     * it fails.
+     */
+    ret = rfc3986_parse(uri, str);
+    if (ret != 0) {
+	uri_clean(uri);
+        ret = rfc3986_parse_relative_ref(uri, str);
+	if (ret != 0) {
+	    uri_clean(uri);
+	    return(ret);
+	}
+    }
+    return(0);
+}
+
+/**
+ * uri_parse:
+ * @str:  the URI string to analyze
+ *
+ * Parse an URI based on RFC 3986
+ *
+ * URI-reference = [ absoluteURI | relativeURI ] [ "#" fragment ]
+ *
+ * Returns a newly built URI or NULL in case of error
+ */
+URI *
+uri_parse(const char *str) {
+    URI *uri;
+    int ret;
+
+    if (str == NULL)
+	return(NULL);
+    uri = uri_new();
+    if (uri != NULL) {
+	ret = rfc3986_parse_uri_reference(uri, str);
+        if (ret) {
+	    uri_free(uri);
+	    return(NULL);
+	}
+    }
+    return(uri);
+}
+
+/**
+ * uri_parse_into:
+ * @uri:  pointer to an URI structure
+ * @str:  the string to analyze
+ *
+ * Parse an URI reference string based on RFC 3986 and fills in the
+ * appropriate fields of the @uri structure
+ *
+ * URI-reference = URI / relative-ref
+ *
+ * Returns 0 or the error code
+ */
+int
+uri_parse_into(URI *uri, const char *str) {
+    return(rfc3986_parse_uri_reference(uri, str));
+}
+
+/**
+ * uri_parse_raw:
+ * @str:  the URI string to analyze
+ * @raw:  if 1 unescaping of URI pieces are disabled
+ *
+ * Parse an URI but allows to keep intact the original fragments.
+ *
+ * URI-reference = URI / relative-ref
+ *
+ * Returns a newly built URI or NULL in case of error
+ */
+URI *
+uri_parse_raw(const char *str, int raw) {
+    URI *uri;
+    int ret;
+
+    if (str == NULL)
+	return(NULL);
+    uri = uri_new();
+    if (uri != NULL) {
+        if (raw) {
+	    uri->cleanup |= 2;
+	}
+	ret = uri_parse_into(uri, str);
+        if (ret) {
+	    uri_free(uri);
+	    return(NULL);
+	}
+    }
+    return(uri);
+}
+
+/************************************************************************
+ *									*
+ *			Generic URI structure functions			*
+ *									*
+ ************************************************************************/
+
+/**
+ * uri_new:
+ *
+ * Simply creates an empty URI
+ *
+ * Returns the new structure or NULL in case of error
+ */
+URI *
+uri_new(void) {
+    URI *ret;
+
+    ret = (URI *) g_malloc(sizeof(URI));
+    memset(ret, 0, sizeof(URI));
+    return(ret);
+}
+
+/**
+ * realloc2n:
+ *
+ * Function to handle properly a reallocation when saving an URI
+ * Also imposes some limit on the length of an URI string output
+ */
+static char *
+realloc2n(char *ret, int *max) {
+    char *temp;
+    int tmp;
+
+    tmp = *max * 2;
+    temp = g_realloc(ret, (tmp + 1));
+    *max = tmp;
+    return(temp);
+}
+
+/**
+ * uri_to_string:
+ * @uri:  pointer to an URI
+ *
+ * Save the URI as an escaped string
+ *
+ * Returns a new string (to be deallocated by caller)
+ */
+char *
+uri_to_string(URI *uri) {
+    char *ret = NULL;
+    char *temp;
+    const char *p;
+    int len;
+    int max;
+
+    if (uri == NULL) return(NULL);
+
+
+    max = 80;
+    ret = g_malloc(max + 1);
+    len = 0;
+
+    if (uri->scheme != NULL) {
+	p = uri->scheme;
+	while (*p != 0) {
+	    if (len >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+		ret = temp;
+	    }
+	    ret[len++] = *p++;
+	}
+	if (len >= max) {
+            temp = realloc2n(ret, &max);
+            if (temp == NULL) goto mem_error;
+            ret = temp;
+	}
+	ret[len++] = ':';
+    }
+    if (uri->opaque != NULL) {
+	p = uri->opaque;
+	while (*p != 0) {
+	    if (len + 3 >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+                ret = temp;
+	    }
+	    if (IS_RESERVED(*(p)) || IS_UNRESERVED(*(p)))
+		ret[len++] = *p++;
+	    else {
+		int val = *(unsigned char *)p++;
+		int hi = val / 0x10, lo = val % 0x10;
+		ret[len++] = '%';
+		ret[len++] = hi + (hi > 9? 'A'-10 : '0');
+		ret[len++] = lo + (lo > 9? 'A'-10 : '0');
+	    }
+	}
+    } else {
+	if (uri->server != NULL) {
+	    if (len + 3 >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+                ret = temp;
+	    }
+	    ret[len++] = '/';
+	    ret[len++] = '/';
+	    if (uri->user != NULL) {
+		p = uri->user;
+		while (*p != 0) {
+		    if (len + 3 >= max) {
+                        temp = realloc2n(ret, &max);
+                        if (temp == NULL) goto mem_error;
+                        ret = temp;
+		    }
+		    if ((IS_UNRESERVED(*(p))) ||
+			((*(p) == ';')) || ((*(p) == ':')) ||
+			((*(p) == '&')) || ((*(p) == '=')) ||
+			((*(p) == '+')) || ((*(p) == '$')) ||
+			((*(p) == ',')))
+			ret[len++] = *p++;
+		    else {
+			int val = *(unsigned char *)p++;
+			int hi = val / 0x10, lo = val % 0x10;
+			ret[len++] = '%';
+			ret[len++] = hi + (hi > 9? 'A'-10 : '0');
+			ret[len++] = lo + (lo > 9? 'A'-10 : '0');
+		    }
+		}
+		if (len + 3 >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		ret[len++] = '@';
+	    }
+	    p = uri->server;
+	    while (*p != 0) {
+		if (len >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		ret[len++] = *p++;
+	    }
+	    if (uri->port > 0) {
+		if (len + 10 >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		len += snprintf(&ret[len], max - len, ":%d", uri->port);
+	    }
+	} else if (uri->authority != NULL) {
+	    if (len + 3 >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+                ret = temp;
+	    }
+	    ret[len++] = '/';
+	    ret[len++] = '/';
+	    p = uri->authority;
+	    while (*p != 0) {
+		if (len + 3 >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		if ((IS_UNRESERVED(*(p))) ||
+                    ((*(p) == '$')) || ((*(p) == ',')) || ((*(p) == ';')) ||
+                    ((*(p) == ':')) || ((*(p) == '@')) || ((*(p) == '&')) ||
+                    ((*(p) == '=')) || ((*(p) == '+')))
+		    ret[len++] = *p++;
+		else {
+		    int val = *(unsigned char *)p++;
+		    int hi = val / 0x10, lo = val % 0x10;
+		    ret[len++] = '%';
+		    ret[len++] = hi + (hi > 9? 'A'-10 : '0');
+		    ret[len++] = lo + (lo > 9? 'A'-10 : '0');
+		}
+	    }
+	} else if (uri->scheme != NULL) {
+	    if (len + 3 >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+                ret = temp;
+	    }
+	    ret[len++] = '/';
+	    ret[len++] = '/';
+	}
+	if (uri->path != NULL) {
+	    p = uri->path;
+	    /*
+	     * the colon in file:///d: should not be escaped or
+	     * Windows accesses fail later.
+	     */
+	    if ((uri->scheme != NULL) &&
+		(p[0] == '/') &&
+		(((p[1] >= 'a') && (p[1] <= 'z')) ||
+		 ((p[1] >= 'A') && (p[1] <= 'Z'))) &&
+		(p[2] == ':') &&
+	        (!strcmp(uri->scheme, "file"))) {
+		if (len + 3 >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		ret[len++] = *p++;
+		ret[len++] = *p++;
+		ret[len++] = *p++;
+	    }
+	    while (*p != 0) {
+		if (len + 3 >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		if ((IS_UNRESERVED(*(p))) || ((*(p) == '/')) ||
+                    ((*(p) == ';')) || ((*(p) == '@')) || ((*(p) == '&')) ||
+	            ((*(p) == '=')) || ((*(p) == '+')) || ((*(p) == '$')) ||
+	            ((*(p) == ',')))
+		    ret[len++] = *p++;
+		else {
+		    int val = *(unsigned char *)p++;
+		    int hi = val / 0x10, lo = val % 0x10;
+		    ret[len++] = '%';
+		    ret[len++] = hi + (hi > 9? 'A'-10 : '0');
+		    ret[len++] = lo + (lo > 9? 'A'-10 : '0');
+		}
+	    }
+	}
+	if (uri->query != NULL) {
+	    if (len + 1 >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+                ret = temp;
+	    }
+	    ret[len++] = '?';
+	    p = uri->query;
+	    while (*p != 0) {
+		if (len + 1 >= max) {
+                    temp = realloc2n(ret, &max);
+                    if (temp == NULL) goto mem_error;
+                    ret = temp;
+		}
+		ret[len++] = *p++;
+	    }
+	}
+    }
+    if (uri->fragment != NULL) {
+	if (len + 3 >= max) {
+            temp = realloc2n(ret, &max);
+            if (temp == NULL) goto mem_error;
+            ret = temp;
+	}
+	ret[len++] = '#';
+	p = uri->fragment;
+	while (*p != 0) {
+	    if (len + 3 >= max) {
+                temp = realloc2n(ret, &max);
+                if (temp == NULL) goto mem_error;
+                ret = temp;
+	    }
+	    if ((IS_UNRESERVED(*(p))) || (IS_RESERVED(*(p))))
+		ret[len++] = *p++;
+	    else {
+		int val = *(unsigned char *)p++;
+		int hi = val / 0x10, lo = val % 0x10;
+		ret[len++] = '%';
+		ret[len++] = hi + (hi > 9? 'A'-10 : '0');
+		ret[len++] = lo + (lo > 9? 'A'-10 : '0');
+	    }
+	}
+    }
+    if (len >= max) {
+        temp = realloc2n(ret, &max);
+        if (temp == NULL) goto mem_error;
+        ret = temp;
+    }
+    ret[len] = 0;
+    return(ret);
+
+mem_error:
+    g_free(ret);
+    return(NULL);
+}
+
+/**
+ * uri_clean:
+ * @uri:  pointer to an URI
+ *
+ * Make sure the URI struct is free of content
+ */
+static void
+uri_clean(URI *uri) {
+    if (uri == NULL) return;
+
+    if (uri->scheme != NULL) g_free(uri->scheme);
+    uri->scheme = NULL;
+    if (uri->server != NULL) g_free(uri->server);
+    uri->server = NULL;
+    if (uri->user != NULL) g_free(uri->user);
+    uri->user = NULL;
+    if (uri->path != NULL) g_free(uri->path);
+    uri->path = NULL;
+    if (uri->fragment != NULL) g_free(uri->fragment);
+    uri->fragment = NULL;
+    if (uri->opaque != NULL) g_free(uri->opaque);
+    uri->opaque = NULL;
+    if (uri->authority != NULL) g_free(uri->authority);
+    uri->authority = NULL;
+    if (uri->query != NULL) g_free(uri->query);
+    uri->query = NULL;
+}
+
+/**
+ * uri_free:
+ * @uri:  pointer to an URI
+ *
+ * Free up the URI struct
+ */
+void
+uri_free(URI *uri) {
+    uri_clean(uri);
+    g_free(uri);
+}
+
+/************************************************************************
+ *									*
+ *			Helper functions				*
+ *									*
+ ************************************************************************/
+
+/**
+ * normalize_uri_path:
+ * @path:  pointer to the path string
+ *
+ * Applies the 5 normalization steps to a path string--that is, RFC 2396
+ * Section 5.2, steps 6.c through 6.g.
+ *
+ * Normalization occurs directly on the string, no new allocation is done
+ *
+ * Returns 0 or an error code
+ */
+static int
+normalize_uri_path(char *path) {
+    char *cur, *out;
+
+    if (path == NULL)
+	return(-1);
+
+    /* Skip all initial "/" chars.  We want to get to the beginning of the
+     * first non-empty segment.
+     */
+    cur = path;
+    while (cur[0] == '/')
+      ++cur;
+    if (cur[0] == '\0')
+      return(0);
+
+    /* Keep everything we've seen so far.  */
+    out = cur;
+
+    /*
+     * Analyze each segment in sequence for cases (c) and (d).
+     */
+    while (cur[0] != '\0') {
+	/*
+	 * c) All occurrences of "./", where "." is a complete path segment,
+	 *    are removed from the buffer string.
+	 */
+	if ((cur[0] == '.') && (cur[1] == '/')) {
+	    cur += 2;
+	    /* '//' normalization should be done at this point too */
+	    while (cur[0] == '/')
+		cur++;
+	    continue;
+	}
+
+	/*
+	 * d) If the buffer string ends with "." as a complete path segment,
+	 *    that "." is removed.
+	 */
+	if ((cur[0] == '.') && (cur[1] == '\0'))
+	    break;
+
+	/* Otherwise keep the segment.  */
+	while (cur[0] != '/') {
+            if (cur[0] == '\0')
+              goto done_cd;
+	    (out++)[0] = (cur++)[0];
+	}
+	/* nomalize // */
+	while ((cur[0] == '/') && (cur[1] == '/'))
+	    cur++;
+
+        (out++)[0] = (cur++)[0];
+    }
+ done_cd:
+    out[0] = '\0';
+
+    /* Reset to the beginning of the first segment for the next sequence.  */
+    cur = path;
+    while (cur[0] == '/')
+      ++cur;
+    if (cur[0] == '\0')
+	return(0);
+
+    /*
+     * Analyze each segment in sequence for cases (e) and (f).
+     *
+     * e) All occurrences of "<segment>/../", where <segment> is a
+     *    complete path segment not equal to "..", are removed from the
+     *    buffer string.  Removal of these path segments is performed
+     *    iteratively, removing the leftmost matching pattern on each
+     *    iteration, until no matching pattern remains.
+     *
+     * f) If the buffer string ends with "<segment>/..", where <segment>
+     *    is a complete path segment not equal to "..", that
+     *    "<segment>/.." is removed.
+     *
+     * To satisfy the "iterative" clause in (e), we need to collapse the
+     * string every time we find something that needs to be removed.  Thus,
+     * we don't need to keep two pointers into the string: we only need a
+     * "current position" pointer.
+     */
+    while (1) {
+        char *segp, *tmp;
+
+        /* At the beginning of each iteration of this loop, "cur" points to
+         * the first character of the segment we want to examine.
+         */
+
+        /* Find the end of the current segment.  */
+        segp = cur;
+        while ((segp[0] != '/') && (segp[0] != '\0'))
+          ++segp;
+
+        /* If this is the last segment, we're done (we need at least two
+         * segments to meet the criteria for the (e) and (f) cases).
+         */
+        if (segp[0] == '\0')
+          break;
+
+        /* If the first segment is "..", or if the next segment _isn't_ "..",
+         * keep this segment and try the next one.
+         */
+        ++segp;
+        if (((cur[0] == '.') && (cur[1] == '.') && (segp == cur+3))
+            || ((segp[0] != '.') || (segp[1] != '.')
+                || ((segp[2] != '/') && (segp[2] != '\0')))) {
+          cur = segp;
+          continue;
+        }
+
+        /* If we get here, remove this segment and the next one and back up
+         * to the previous segment (if there is one), to implement the
+         * "iteratively" clause.  It's pretty much impossible to back up
+         * while maintaining two pointers into the buffer, so just compact
+         * the whole buffer now.
+         */
+
+        /* If this is the end of the buffer, we're done.  */
+        if (segp[2] == '\0') {
+          cur[0] = '\0';
+          break;
+        }
+        /* Valgrind complained, strcpy(cur, segp + 3); */
+        /* string will overlap, do not use strcpy */
+        tmp = cur;
+        segp += 3;
+        while ((*tmp++ = *segp++) != 0)
+          ;
+
+        /* If there are no previous segments, then keep going from here.  */
+        segp = cur;
+        while ((segp > path) && ((--segp)[0] == '/'))
+          ;
+        if (segp == path)
+          continue;
+
+        /* "segp" is pointing to the end of a previous segment; find it's
+         * start.  We need to back up to the previous segment and start
+         * over with that to handle things like "foo/bar/../..".  If we
+         * don't do this, then on the first pass we'll remove the "bar/..",
+         * but be pointing at the second ".." so we won't realize we can also
+         * remove the "foo/..".
+         */
+        cur = segp;
+        while ((cur > path) && (cur[-1] != '/'))
+          --cur;
+    }
+    out[0] = '\0';
+
+    /*
+     * g) If the resulting buffer string still begins with one or more
+     *    complete path segments of "..", then the reference is
+     *    considered to be in error. Implementations may handle this
+     *    error by retaining these components in the resolved path (i.e.,
+     *    treating them as part of the final URI), by removing them from
+     *    the resolved path (i.e., discarding relative levels above the
+     *    root), or by avoiding traversal of the reference.
+     *
+     * We discard them from the final path.
+     */
+    if (path[0] == '/') {
+      cur = path;
+      while ((cur[0] == '/') && (cur[1] == '.') && (cur[2] == '.')
+             && ((cur[3] == '/') || (cur[3] == '\0')))
+	cur += 3;
+
+      if (cur != path) {
+	out = path;
+	while (cur[0] != '\0')
+          (out++)[0] = (cur++)[0];
+	out[0] = 0;
+      }
+    }
+
+    return(0);
+}
+
+static int is_hex(char c) {
+    if (((c >= '0') && (c <= '9')) ||
+        ((c >= 'a') && (c <= 'f')) ||
+        ((c >= 'A') && (c <= 'F')))
+	return(1);
+    return(0);
+}
+
+
+/**
+ * uri_string_unescape:
+ * @str:  the string to unescape
+ * @len:   the length in bytes to unescape (or <= 0 to indicate full string)
+ * @target:  optional destination buffer
+ *
+ * Unescaping routine, but does not check that the string is an URI. The
+ * output is a direct unsigned char translation of %XX values (no encoding)
+ * Note that the length of the result can only be smaller or same size as
+ * the input string.
+ *
+ * Returns a copy of the string, but unescaped, will return NULL only in case
+ * of error
+ */
+char *
+uri_string_unescape(const char *str, int len, char *target) {
+    char *ret, *out;
+    const char *in;
+
+    if (str == NULL)
+	return(NULL);
+    if (len <= 0) len = strlen(str);
+    if (len < 0) return(NULL);
+
+    if (target == NULL) {
+	ret = g_malloc(len + 1);
+    } else
+	ret = target;
+    in = str;
+    out = ret;
+    while(len > 0) {
+	if ((len > 2) && (*in == '%') && (is_hex(in[1])) && (is_hex(in[2]))) {
+	    in++;
+	    if ((*in >= '0') && (*in <= '9'))
+	        *out = (*in - '0');
+	    else if ((*in >= 'a') && (*in <= 'f'))
+	        *out = (*in - 'a') + 10;
+	    else if ((*in >= 'A') && (*in <= 'F'))
+	        *out = (*in - 'A') + 10;
+	    in++;
+	    if ((*in >= '0') && (*in <= '9'))
+	        *out = *out * 16 + (*in - '0');
+	    else if ((*in >= 'a') && (*in <= 'f'))
+	        *out = *out * 16 + (*in - 'a') + 10;
+	    else if ((*in >= 'A') && (*in <= 'F'))
+	        *out = *out * 16 + (*in - 'A') + 10;
+	    in++;
+	    len -= 3;
+	    out++;
+	} else {
+	    *out++ = *in++;
+	    len--;
+	}
+    }
+    *out = 0;
+    return(ret);
+}
+
+/**
+ * uri_string_escape:
+ * @str:  string to escape
+ * @list: exception list string of chars not to escape
+ *
+ * This routine escapes a string to hex, ignoring reserved characters (a-z)
+ * and the characters in the exception list.
+ *
+ * Returns a new escaped string or NULL in case of error.
+ */
+char *
+uri_string_escape(const char *str, const char *list) {
+    char *ret, ch;
+    char *temp;
+    const char *in;
+    int len, out;
+
+    if (str == NULL)
+	return(NULL);
+    if (str[0] == 0)
+	return(g_strdup(str));
+    len = strlen(str);
+    if (!(len > 0)) return(NULL);
+
+    len += 20;
+    ret = g_malloc(len);
+    in = str;
+    out = 0;
+    while(*in != 0) {
+	if (len - out <= 3) {
+            temp = realloc2n(ret, &len);
+	    ret = temp;
+	}
+
+	ch = *in;
+
+	if ((ch != '@') && (!IS_UNRESERVED(ch)) && (!strchr(list, ch))) {
+	    unsigned char val;
+	    ret[out++] = '%';
+	    val = ch >> 4;
+	    if (val <= 9)
+		ret[out++] = '0' + val;
+	    else
+		ret[out++] = 'A' + val - 0xA;
+	    val = ch & 0xF;
+	    if (val <= 9)
+		ret[out++] = '0' + val;
+	    else
+		ret[out++] = 'A' + val - 0xA;
+	    in++;
+	} else {
+	    ret[out++] = *in++;
+	}
+
+    }
+    ret[out] = 0;
+    return(ret);
+}
+
+/************************************************************************
+ *									*
+ *			Public functions				*
+ *									*
+ ************************************************************************/
+
+/**
+ * uri_resolve:
+ * @URI:  the URI instance found in the document
+ * @base:  the base value
+ *
+ * Computes he final URI of the reference done by checking that
+ * the given URI is valid, and building the final URI using the
+ * base URI. This is processed according to section 5.2 of the
+ * RFC 2396
+ *
+ * 5.2. Resolving Relative References to Absolute Form
+ *
+ * Returns a new URI string (to be freed by the caller) or NULL in case
+ *         of error.
+ */
+char *
+uri_resolve(const char *uri, const char *base) {
+    char *val = NULL;
+    int ret, len, indx, cur, out;
+    URI *ref = NULL;
+    URI *bas = NULL;
+    URI *res = NULL;
+
+    /*
+     * 1) The URI reference is parsed into the potential four components and
+     *    fragment identifier, as described in Section 4.3.
+     *
+     *    NOTE that a completely empty URI is treated by modern browsers
+     *    as a reference to "." rather than as a synonym for the current
+     *    URI.  Should we do that here?
+     */
+    if (uri == NULL)
+	ret = -1;
+    else {
+	if (*uri) {
+	    ref = uri_new();
+	    if (ref == NULL)
+		goto done;
+	    ret = uri_parse_into(ref, uri);
+	}
+	else
+	    ret = 0;
+    }
+    if (ret != 0)
+	goto done;
+    if ((ref != NULL) && (ref->scheme != NULL)) {
+	/*
+	 * The URI is absolute don't modify.
+	 */
+	val = g_strdup(uri);
+	goto done;
+    }
+    if (base == NULL)
+	ret = -1;
+    else {
+	bas = uri_new();
+	if (bas == NULL)
+	    goto done;
+	ret = uri_parse_into(bas, base);
+    }
+    if (ret != 0) {
+	if (ref)
+	    val = uri_to_string(ref);
+	goto done;
+    }
+    if (ref == NULL) {
+	/*
+	 * the base fragment must be ignored
+	 */
+	if (bas->fragment != NULL) {
+	    g_free(bas->fragment);
+	    bas->fragment = NULL;
+	}
+	val = uri_to_string(bas);
+	goto done;
+    }
+
+    /*
+     * 2) If the path component is empty and the scheme, authority, and
+     *    query components are undefined, then it is a reference to the
+     *    current document and we are done.  Otherwise, the reference URI's
+     *    query and fragment components are defined as found (or not found)
+     *    within the URI reference and not inherited from the base URI.
+     *
+     *    NOTE that in modern browsers, the parsing differs from the above
+     *    in the following aspect:  the query component is allowed to be
+     *    defined while still treating this as a reference to the current
+     *    document.
+     */
+    res = uri_new();
+    if (res == NULL)
+	goto done;
+    if ((ref->scheme == NULL) && (ref->path == NULL) &&
+	((ref->authority == NULL) && (ref->server == NULL))) {
+	if (bas->scheme != NULL)
+	    res->scheme = g_strdup(bas->scheme);
+	if (bas->authority != NULL)
+	    res->authority = g_strdup(bas->authority);
+	else if (bas->server != NULL) {
+	    res->server = g_strdup(bas->server);
+	    if (bas->user != NULL)
+		res->user = g_strdup(bas->user);
+	    res->port = bas->port;
+	}
+	if (bas->path != NULL)
+	    res->path = g_strdup(bas->path);
+	if (ref->query != NULL)
+	    res->query = g_strdup (ref->query);
+	else if (bas->query != NULL)
+	    res->query = g_strdup(bas->query);
+	if (ref->fragment != NULL)
+	    res->fragment = g_strdup(ref->fragment);
+	goto step_7;
+    }
+
+    /*
+     * 3) If the scheme component is defined, indicating that the reference
+     *    starts with a scheme name, then the reference is interpreted as an
+     *    absolute URI and we are done.  Otherwise, the reference URI's
+     *    scheme is inherited from the base URI's scheme component.
+     */
+    if (ref->scheme != NULL) {
+	val = uri_to_string(ref);
+	goto done;
+    }
+    if (bas->scheme != NULL)
+	res->scheme = g_strdup(bas->scheme);
+
+    if (ref->query != NULL)
+	res->query = g_strdup(ref->query);
+    if (ref->fragment != NULL)
+	res->fragment = g_strdup(ref->fragment);
+
+    /*
+     * 4) If the authority component is defined, then the reference is a
+     *    network-path and we skip to step 7.  Otherwise, the reference
+     *    URI's authority is inherited from the base URI's authority
+     *    component, which will also be undefined if the URI scheme does not
+     *    use an authority component.
+     */
+    if ((ref->authority != NULL) || (ref->server != NULL)) {
+	if (ref->authority != NULL)
+	    res->authority = g_strdup(ref->authority);
+	else {
+	    res->server = g_strdup(ref->server);
+	    if (ref->user != NULL)
+		res->user = g_strdup(ref->user);
+            res->port = ref->port;
+	}
+	if (ref->path != NULL)
+	    res->path = g_strdup(ref->path);
+	goto step_7;
+    }
+    if (bas->authority != NULL)
+	res->authority = g_strdup(bas->authority);
+    else if (bas->server != NULL) {
+	res->server = g_strdup(bas->server);
+	if (bas->user != NULL)
+	    res->user = g_strdup(bas->user);
+	res->port = bas->port;
+    }
+
+    /*
+     * 5) If the path component begins with a slash character ("/"), then
+     *    the reference is an absolute-path and we skip to step 7.
+     */
+    if ((ref->path != NULL) && (ref->path[0] == '/')) {
+	res->path = g_strdup(ref->path);
+	goto step_7;
+    }
+
+
+    /*
+     * 6) If this step is reached, then we are resolving a relative-path
+     *    reference.  The relative path needs to be merged with the base
+     *    URI's path.  Although there are many ways to do this, we will
+     *    describe a simple method using a separate string buffer.
+     *
+     * Allocate a buffer large enough for the result string.
+     */
+    len = 2; /* extra / and 0 */
+    if (ref->path != NULL)
+	len += strlen(ref->path);
+    if (bas->path != NULL)
+	len += strlen(bas->path);
+    res->path = g_malloc(len);
+    res->path[0] = 0;
+
+    /*
+     * a) All but the last segment of the base URI's path component is
+     *    copied to the buffer.  In other words, any characters after the
+     *    last (right-most) slash character, if any, are excluded.
+     */
+    cur = 0;
+    out = 0;
+    if (bas->path != NULL) {
+	while (bas->path[cur] != 0) {
+	    while ((bas->path[cur] != 0) && (bas->path[cur] != '/'))
+		cur++;
+	    if (bas->path[cur] == 0)
+		break;
+
+	    cur++;
+	    while (out < cur) {
+		res->path[out] = bas->path[out];
+		out++;
+	    }
+	}
+    }
+    res->path[out] = 0;
+
+    /*
+     * b) The reference's path component is appended to the buffer
+     *    string.
+     */
+    if (ref->path != NULL && ref->path[0] != 0) {
+	indx = 0;
+	/*
+	 * Ensure the path includes a '/'
+	 */
+	if ((out == 0) && (bas->server != NULL))
+	    res->path[out++] = '/';
+	while (ref->path[indx] != 0) {
+	    res->path[out++] = ref->path[indx++];
+	}
+    }
+    res->path[out] = 0;
+
+    /*
+     * Steps c) to h) are really path normalization steps
+     */
+    normalize_uri_path(res->path);
+
+step_7:
+
+    /*
+     * 7) The resulting URI components, including any inherited from the
+     *    base URI, are recombined to give the absolute form of the URI
+     *    reference.
+     */
+    val = uri_to_string(res);
+
+done:
+    if (ref != NULL)
+	uri_free(ref);
+    if (bas != NULL)
+	uri_free(bas);
+    if (res != NULL)
+	uri_free(res);
+    return(val);
+}
+
+/**
+ * uri_resolve_relative:
+ * @URI:  the URI reference under consideration
+ * @base:  the base value
+ *
+ * Expresses the URI of the reference in terms relative to the
+ * base.  Some examples of this operation include:
+ *     base = "http://site1.com/docs/book1.html"
+ *        URI input                        URI returned
+ *     docs/pic1.gif                    pic1.gif
+ *     docs/img/pic1.gif                img/pic1.gif
+ *     img/pic1.gif                     ../img/pic1.gif
+ *     http://site1.com/docs/pic1.gif   pic1.gif
+ *     http://site2.com/docs/pic1.gif   http://site2.com/docs/pic1.gif
+ *
+ *     base = "docs/book1.html"
+ *        URI input                        URI returned
+ *     docs/pic1.gif                    pic1.gif
+ *     docs/img/pic1.gif                img/pic1.gif
+ *     img/pic1.gif                     ../img/pic1.gif
+ *     http://site1.com/docs/pic1.gif   http://site1.com/docs/pic1.gif
+ *
+ *
+ * Note: if the URI reference is really wierd or complicated, it may be
+ *       worthwhile to first convert it into a "nice" one by calling
+ *       uri_resolve (using 'base') before calling this routine,
+ *       since this routine (for reasonable efficiency) assumes URI has
+ *       already been through some validation.
+ *
+ * Returns a new URI string (to be freed by the caller) or NULL in case
+ * error.
+ */
+char *
+uri_resolve_relative (const char *uri, const char * base)
+{
+    char *val = NULL;
+    int ret;
+    int ix;
+    int pos = 0;
+    int nbslash = 0;
+    int len;
+    URI *ref = NULL;
+    URI *bas = NULL;
+    char *bptr, *uptr, *vptr;
+    int remove_path = 0;
+
+    if ((uri == NULL) || (*uri == 0))
+	return NULL;
+
+    /*
+     * First parse URI into a standard form
+     */
+    ref = uri_new ();
+    if (ref == NULL)
+	return NULL;
+    /* If URI not already in "relative" form */
+    if (uri[0] != '.') {
+	ret = uri_parse_into (ref, uri);
+	if (ret != 0)
+	    goto done;		/* Error in URI, return NULL */
+    } else
+	ref->path = g_strdup(uri);
+
+    /*
+     * Next parse base into the same standard form
+     */
+    if ((base == NULL) || (*base == 0)) {
+	val = g_strdup (uri);
+	goto done;
+    }
+    bas = uri_new ();
+    if (bas == NULL)
+	goto done;
+    if (base[0] != '.') {
+	ret = uri_parse_into (bas, base);
+	if (ret != 0)
+	    goto done;		/* Error in base, return NULL */
+    } else
+	bas->path = g_strdup(base);
+
+    /*
+     * If the scheme / server on the URI differs from the base,
+     * just return the URI
+     */
+    if ((ref->scheme != NULL) &&
+	((bas->scheme == NULL) ||
+	 (strcmp (bas->scheme, ref->scheme)) ||
+	 (strcmp (bas->server, ref->server)))) {
+	val = g_strdup (uri);
+	goto done;
+    }
+    if (!strcmp(bas->path, ref->path)) {
+	val = g_strdup("");
+	goto done;
+    }
+    if (bas->path == NULL) {
+	val = g_strdup(ref->path);
+	goto done;
+    }
+    if (ref->path == NULL) {
+        ref->path = (char *) "/";
+	remove_path = 1;
+    }
+
+    /*
+     * At this point (at last!) we can compare the two paths
+     *
+     * First we take care of the special case where either of the
+     * two path components may be missing (bug 316224)
+     */
+    if (bas->path == NULL) {
+	if (ref->path != NULL) {
+	    uptr = ref->path;
+	    if (*uptr == '/')
+		uptr++;
+	    /* exception characters from uri_to_string */
+	    val = uri_string_escape(uptr, "/;&=+$,");
+	}
+	goto done;
+    }
+    bptr = bas->path;
+    if (ref->path == NULL) {
+	for (ix = 0; bptr[ix] != 0; ix++) {
+	    if (bptr[ix] == '/')
+		nbslash++;
+	}
+	uptr = NULL;
+	len = 1;	/* this is for a string terminator only */
+    } else {
+    /*
+     * Next we compare the two strings and find where they first differ
+     */
+	if ((ref->path[pos] == '.') && (ref->path[pos+1] == '/'))
+            pos += 2;
+	if ((*bptr == '.') && (bptr[1] == '/'))
+            bptr += 2;
+	else if ((*bptr == '/') && (ref->path[pos] != '/'))
+	    bptr++;
+	while ((bptr[pos] == ref->path[pos]) && (bptr[pos] != 0))
+	    pos++;
+
+	if (bptr[pos] == ref->path[pos]) {
+	    val = g_strdup("");
+	    goto done;		/* (I can't imagine why anyone would do this) */
+	}
+
+	/*
+	 * In URI, "back up" to the last '/' encountered.  This will be the
+	 * beginning of the "unique" suffix of URI
+	 */
+	ix = pos;
+	if ((ref->path[ix] == '/') && (ix > 0))
+	    ix--;
+	else if ((ref->path[ix] == 0) && (ix > 1) && (ref->path[ix - 1] == '/'))
+	    ix -= 2;
+	for (; ix > 0; ix--) {
+	    if (ref->path[ix] == '/')
+		break;
+	}
+	if (ix == 0) {
+	    uptr = ref->path;
+	} else {
+	    ix++;
+	    uptr = &ref->path[ix];
+	}
+
+	/*
+	 * In base, count the number of '/' from the differing point
+	 */
+	if (bptr[pos] != ref->path[pos]) {/* check for trivial URI == base */
+	    for (; bptr[ix] != 0; ix++) {
+		if (bptr[ix] == '/')
+		    nbslash++;
+	    }
+	}
+	len = strlen (uptr) + 1;
+    }
+
+    if (nbslash == 0) {
+	if (uptr != NULL)
+	    /* exception characters from uri_to_string */
+	    val = uri_string_escape(uptr, "/;&=+$,");
+	goto done;
+    }
+
+    /*
+     * Allocate just enough space for the returned string -
+     * length of the remainder of the URI, plus enough space
+     * for the "../" groups, plus one for the terminator
+     */
+    val = g_malloc (len + 3 * nbslash);
+    vptr = val;
+    /*
+     * Put in as many "../" as needed
+     */
+    for (; nbslash>0; nbslash--) {
+	*vptr++ = '.';
+	*vptr++ = '.';
+	*vptr++ = '/';
+    }
+    /*
+     * Finish up with the end of the URI
+     */
+    if (uptr != NULL) {
+        if ((vptr > val) && (len > 0) &&
+	    (uptr[0] == '/') && (vptr[-1] == '/')) {
+	    memcpy (vptr, uptr + 1, len - 1);
+	    vptr[len - 2] = 0;
+	} else {
+	    memcpy (vptr, uptr, len);
+	    vptr[len - 1] = 0;
+	}
+    } else {
+	vptr[len - 1] = 0;
+    }
+
+    /* escape the freshly-built path */
+    vptr = val;
+	/* exception characters from uri_to_string */
+    val = uri_string_escape(vptr, "/;&=+$,");
+    g_free(vptr);
+
+done:
+    /*
+     * Free the working variables
+     */
+    if (remove_path != 0)
+        ref->path = NULL;
+    if (ref != NULL)
+	uri_free (ref);
+    if (bas != NULL)
+	uri_free (bas);
+
+    return val;
+}
+
+/*
+ * Utility functions to help parse and assemble query strings.
+ */
+
+struct QueryParams *
+query_params_new (int init_alloc)
+{
+    struct QueryParams *ps;
+
+    if (init_alloc <= 0) init_alloc = 1;
+
+    ps = g_new(QueryParams, 1);
+    ps->n = 0;
+    ps->alloc = init_alloc;
+    ps->p = g_new(QueryParam, ps->alloc);
+
+    return ps;
+}
+
+/* Ensure there is space to store at least one more parameter
+ * at the end of the set.
+ */
+static int
+query_params_append (struct QueryParams *ps,
+               const char *name, const char *value)
+{
+    if (ps->n >= ps->alloc) {
+        ps->p = g_renew(QueryParam, ps->p, ps->alloc * 2);
+        ps->alloc *= 2;
+    }
+
+    ps->p[ps->n].name = g_strdup(name);
+    ps->p[ps->n].value = value ? g_strdup(value) : NULL;
+    ps->p[ps->n].ignore = 0;
+    ps->n++;
+
+    return 0;
+}
+
+void
+query_params_free (struct QueryParams *ps)
+{
+    int i;
+
+    for (i = 0; i < ps->n; ++i) {
+        g_free (ps->p[i].name);
+        g_free (ps->p[i].value);
+    }
+    g_free (ps->p);
+    g_free (ps);
+}
+
+struct QueryParams *
+query_params_parse (const char *query)
+{
+    struct QueryParams *ps;
+    const char *end, *eq;
+
+    ps = query_params_new (0);
+    if (!query || query[0] == '\0') return ps;
+
+    while (*query) {
+        char *name = NULL, *value = NULL;
+
+        /* Find the next separator, or end of the string. */
+        end = strchr (query, '&');
+        if (!end)
+            end = strchr (query, ';');
+        if (!end)
+            end = query + strlen (query);
+
+        /* Find the first '=' character between here and end. */
+        eq = strchr (query, '=');
+        if (eq && eq >= end) eq = NULL;
+
+        /* Empty section (eg. "&&"). */
+        if (end == query)
+            goto next;
+
+        /* If there is no '=' character, then we have just "name"
+         * and consistent with CGI.pm we assume value is "".
+         */
+        else if (!eq) {
+            name = uri_string_unescape (query, end - query, NULL);
+            value = NULL;
+        }
+        /* Or if we have "name=" here (works around annoying
+         * problem when calling uri_string_unescape with len = 0).
+         */
+        else if (eq+1 == end) {
+            name = uri_string_unescape (query, eq - query, NULL);
+            value = g_new0(char, 1);
+        }
+        /* If the '=' character is at the beginning then we have
+         * "=value" and consistent with CGI.pm we _ignore_ this.
+         */
+        else if (query == eq)
+            goto next;
+
+        /* Otherwise it's "name=value". */
+        else {
+            name = uri_string_unescape (query, eq - query, NULL);
+            value = uri_string_unescape (eq+1, end - (eq+1), NULL);
+        }
+
+        /* Append to the parameter set. */
+        query_params_append (ps, name, value);
+        g_free(name);
+        g_free(value);
+
+    next:
+        query = end;
+        if (*query) query ++; /* skip '&' separator */
+    }
+
+    return ps;
+}
diff --git a/uri.h b/uri.h
new file mode 100644
index 0000000..de99b3b
--- /dev/null
+++ b/uri.h
@@ -0,0 +1,113 @@
+/**
+ * Summary: library of generic URI related routines
+ * Description: library of generic URI related routines
+ *              Implements RFC 2396
+ *
+ * Copyright (C) 1998-2003 Daniel Veillard.  All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
+ * DANIEL VEILLARD BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Except as contained in this notice, the name of Daniel Veillard shall not
+ * be used in advertising or otherwise to promote the sale, use or other
+ * dealings in this Software without prior written authorization from him.
+ *
+ * Author: Daniel Veillard
+ **
+ * Copyright (C) 2007 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Authors:
+ *    Richard W.M. Jones <rjones@redhat.com>
+ *
+ * Utility functions to help parse and assemble query strings.
+ */
+
+#ifndef QEMU_URI_H
+#define QEMU_URI_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * URI:
+ *
+ * A parsed URI reference. This is a struct containing the various fields
+ * as described in RFC 2396 but separated for further processing.
+ */
+typedef struct URI {
+    char *scheme;	/* the URI scheme */
+    char *opaque;	/* opaque part */
+    char *authority;	/* the authority part */
+    char *server;	/* the server part */
+    char *user;		/* the user part */
+    int port;		/* the port number */
+    char *path;		/* the path string */
+    char *fragment;	/* the fragment identifier */
+    int  cleanup;	/* parsing potentially unclean URI */
+    char *query;	/* the query string (as it appears in the URI) */
+} URI;
+
+URI *uri_new(void);
+char *uri_resolve(const char *URI, const char *base);
+char *uri_resolve_relative(const char *URI, const char *base);
+URI *uri_parse(const char *str);
+URI *uri_parse_raw(const char *str, int raw);
+int uri_parse_into(URI *uri, const char *str);
+char *uri_to_string(URI *uri);
+char *uri_string_escape(const char *str, const char *list);
+char *uri_string_unescape(const char *str, int len, char *target);
+void uri_free(URI *uri);
+
+/* Single web service query parameter 'name=value'. */
+typedef struct QueryParam {
+  char *name;			/* Name (unescaped). */
+  char *value;			/* Value (unescaped). */
+  int ignore;			/* Ignore this field in qparam_get_query */
+} QueryParam;
+
+/* Set of parameters. */
+typedef struct QueryParams {
+  int n;			/* number of parameters used */
+  int alloc;			/* allocated space */
+  QueryParam *p;		/* array of parameters */
+} QueryParams;
+
+struct QueryParams *query_params_new (int init_alloc);
+int query_param_append (QueryParams *ps, const char *name, const char *value);
+extern char *query_param_to_string (const QueryParams *ps);
+extern QueryParams *query_params_parse (const char *query);
+extern void query_params_free (QueryParams *ps);
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* QEMU_URI_H */

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v9 3/4] configure: Add a config option for GlusterFS as block backend
  2012-09-24  9:10 [Qemu-devel] [PATCH v9 0/4] GlusterFS support in QEMU - v9 Bharata B Rao
  2012-09-24  9:10 ` [Qemu-devel] [PATCH v9 1/4] aio: Fix qemu_aio_wait() to maintain correct walking_handlers count Bharata B Rao
  2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library Bharata B Rao
@ 2012-09-24  9:12 ` Bharata B Rao
  2012-09-24  9:13 ` [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU " Bharata B Rao
  3 siblings, 0 replies; 26+ messages in thread
From: Bharata B Rao @ 2012-09-24  9:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	Daniel Veillard, Blue Swirl, Avi Kivity, Paolo Bonzini

configure: Add a config option for GlusterFS as block backend

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

GlusterFS support in QEMU depends on libgfapi, libgfrpc and
libgfxdr provided by GlusterFS.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---

 configure |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)


diff --git a/configure b/configure
index 1b86517..eb1af9f 100755
--- a/configure
+++ b/configure
@@ -219,6 +219,7 @@ want_tools="yes"
 libiscsi=""
 coroutine=""
 seccomp=""
+glusterfs=""
 
 # parse CC options first
 for opt do
@@ -856,6 +857,10 @@ for opt do
   ;;
   --disable-seccomp) seccomp="no"
   ;;
+  --disable-glusterfs) glusterfs="no"
+  ;;
+  --enable-glusterfs) glusterfs="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1128,6 +1133,8 @@ echo "  --disable-seccomp        disable seccomp support"
 echo "  --enable-seccomp         enables seccomp support"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "                           gthread, ucontext, sigaltstack, windows"
+echo "  --enable-glusterfs       enable GlusterFS backend"
+echo "  --disable-glusterfs      disable GlusterFS backend"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2307,6 +2314,29 @@ EOF
   fi
 fi
 
+##########################################
+# glusterfs probe
+if test "$glusterfs" != "no" ; then
+  cat > $TMPC <<EOF
+#include <glusterfs/api/glfs.h>
+int main(void) {
+    (void) glfs_new("volume");
+    return 0;
+}
+EOF
+  glusterfs_libs="-lgfapi -lgfrpc -lgfxdr"
+  if compile_prog "" "$glusterfs_libs" ; then
+    glusterfs=yes
+    libs_tools="$glusterfs_libs $libs_tools"
+    libs_softmmu="$glusterfs_libs $libs_softmmu"
+  else
+    if test "$glusterfs" = "yes" ; then
+      feature_not_found "GlusterFS backend support"
+    fi
+    glusterfs=no
+  fi
+fi
+
 #
 # Check for xxxat() functions when we are building linux-user
 # emulator.  This is done because older glibc versions don't
@@ -3174,6 +3204,7 @@ echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "seccomp support   $seccomp"
 echo "coroutine backend $coroutine_backend"
+echo "GlusterFS support $glusterfs"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3520,6 +3551,10 @@ if test "$has_environ" = "yes" ; then
   echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak
 fi
 
+if test "$glusterfs" = "yes" ; then
+  echo "CONFIG_GLUSTERFS=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-24  9:10 [Qemu-devel] [PATCH v9 0/4] GlusterFS support in QEMU - v9 Bharata B Rao
                   ` (2 preceding siblings ...)
  2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 3/4] configure: Add a config option for GlusterFS as block backend Bharata B Rao
@ 2012-09-24  9:13 ` Bharata B Rao
  2012-09-24  9:26   ` Paolo Bonzini
                     ` (2 more replies)
  3 siblings, 3 replies; 26+ messages in thread
From: Bharata B Rao @ 2012-09-24  9:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	Daniel Veillard, Blue Swirl, Avi Kivity, Paolo Bonzini

block: Support GlusterFS as a QEMU block backend.

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

This patch adds gluster as the new block backend in QEMU. This gives
QEMU the ability to boot VM images from gluster volumes. Its already
possible to boot from VM images on gluster volumes using FUSE mount, but
this patchset provides the ability to boot VM images from gluster volumes
by by-passing the FUSE layer in gluster. This is made possible by
using libgfapi routines to perform IO on gluster volumes directly.

VM Image on gluster volume is specified like this:

file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]

'gluster' is the protocol.

'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.

'server' specifies the server 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 [ ].
If transport type is 'unix', then server field is ignored, but the
'socket' field needs to be populated with the path to unix domain
socket.

'port' is the port number on which glusterd is listening. This is optional
and if not specified, QEMU will send 0 which will make gluster to use the
default port. port is ignored for unix type of transport.

'volname' is the name of the gluster volume which contains the VM image.

'image' is the path to the actual VM image that resides on gluster volume.

Examples:

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://server.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

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---

 block/Makefile.objs |    1 
 block/gluster.c     |  642 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 643 insertions(+), 0 deletions(-)
 create mode 100644 block/gluster.c


diff --git a/block/Makefile.objs b/block/Makefile.objs
index b5754d3..a1ae67f 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,3 +9,4 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(CONFIG_CURL) += curl.o
 block-obj-$(CONFIG_RBD) += rbd.o
+block-obj-$(CONFIG_GLUSTERFS) += gluster.o
diff --git a/block/gluster.c b/block/gluster.c
new file mode 100644
index 0000000..a2f8303
--- /dev/null
+++ b/block/gluster.c
@@ -0,0 +1,642 @@
+/*
+ * GlusterFS backend for QEMU
+ *
+ * Copyright (C) 2012 Bharata B Rao <bharata@linux.vnet.ibm.com>
+ *
+ * Pipe handling mechanism in AIO implementation is derived from
+ * block/rbd.c. Hence,
+ *
+ * Copyright (C) 2010-2011 Christian Brunner <chb@muc.de>,
+ *                         Josh Durgin <josh.durgin@dreamhost.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+#include <glusterfs/api/glfs.h>
+#include "block_int.h"
+#include "qemu_socket.h"
+#include "uri.h"
+
+typedef struct GlusterAIOCB {
+    BlockDriverAIOCB common;
+    int64_t size;
+    int ret;
+    bool *finished;
+    QEMUBH *bh;
+} GlusterAIOCB;
+
+typedef struct BDRVGlusterState {
+    struct glfs *glfs;
+    int fds[2];
+    struct glfs_fd *fd;
+    int qemu_aio_count;
+    int event_reader_pos;
+    GlusterAIOCB *event_acb;
+} BDRVGlusterState;
+
+#define GLUSTER_FD_READ  0
+#define GLUSTER_FD_WRITE 1
+
+typedef struct GlusterConf {
+    char *server;
+    int port;
+    char *volname;
+    char *image;
+    char *transport;
+} GlusterConf;
+
+static void qemu_gluster_gconf_free(GlusterConf *gconf)
+{
+    g_free(gconf->server);
+    g_free(gconf->volname);
+    g_free(gconf->image);
+    g_free(gconf->transport);
+    g_free(gconf);
+}
+
+static int parse_volume_options(GlusterConf *gconf, char *path)
+{
+    char *token, *saveptr;
+
+    /* volname */
+    token = strtok_r(path, "/", &saveptr);
+    if (!token) {
+        return -EINVAL;
+    }
+    gconf->volname = g_strdup(token);
+
+    /* image */
+    token = strtok_r(NULL, "?", &saveptr);
+    if (!token) {
+        return -EINVAL;
+    }
+    gconf->image = g_strdup(token);
+    return 0;
+}
+
+/*
+ * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
+ *
+ * 'gluster' is the protocol.
+ *
+ * '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.
+ *
+ * 'server' specifies the server 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 [ ].
+ * If transport type is 'unix', then server field is ignored, but the
+ * 'socket' field needs to be populated with the path to unix domain
+ * socket.
+ *
+ * 'port' is the port number on which glusterd is listening. This is optional
+ * and if not specified, QEMU will send 0 which will make gluster to use the
+ * default port. port is ignored for unix type of transport.
+ *
+ * 'volname' is the name of the gluster volume which contains the VM image.
+ *
+ * 'image' is the path to the actual VM image that resides on gluster volume.
+ *
+ * Examples:
+ *
+ * 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://server.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)
+{
+    URI *uri;
+    QueryParams *qp = NULL;
+    bool is_unix = false;
+    int ret = 0;
+    char *unescape_str = NULL;
+
+    uri = uri_parse(filename);
+    if (!uri) {
+        return -EINVAL;
+    }
+
+    /* transport */
+    if (!strcmp(uri->scheme, "gluster")) {
+        gconf->transport = g_strdup("tcp");
+    } else if (!strcmp(uri->scheme, "gluster+tcp")) {
+        gconf->transport = g_strdup("tcp");
+    } else if (!strcmp(uri->scheme, "gluster+unix")) {
+        gconf->transport = g_strdup("unix");
+        is_unix = true;
+    } else if (!strcmp(uri->scheme, "gluster+rdma")) {
+        gconf->transport = g_strdup("rdma");
+    } else {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    ret = parse_volume_options(gconf, uri->path);
+    if (ret < 0) {
+        goto out;
+    }
+
+    if (uri->query) {
+        unescape_str = uri_string_unescape(uri->query, -1, NULL);
+        if (!unescape_str) {
+            ret = -EINVAL;
+            goto out;
+        }
+    }
+
+    qp = query_params_parse(unescape_str);
+    if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    if (is_unix) {
+        if (strcmp(qp->p[0].name, "socket")) {
+            ret = -EINVAL;
+            goto out;
+        }
+        gconf->server = g_strdup(qp->p[0].value);
+    } else {
+        gconf->server = g_strdup(uri->server);
+        gconf->port = uri->port;
+    }
+
+out:
+    if (qp) {
+        query_params_free(qp);
+    }
+    g_free(unescape_str);
+    uri_free(uri);
+    return ret;
+}
+
+static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename)
+{
+    struct glfs *glfs = NULL;
+    int ret;
+
+    ret = qemu_gluster_parseuri(gconf, filename);
+    if (ret < 0) {
+        error_report("Usage: file=gluster[+transport]://[server[:port]]/"
+            "volname/image[?socket=...]");
+        errno = -ret;
+        goto out;
+    }
+
+    glfs = glfs_new(gconf->volname);
+    if (!glfs) {
+        goto out;
+    }
+
+    ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->server,
+            gconf->port);
+    if (ret < 0) {
+        goto out;
+    }
+
+    /*
+     * TODO: Use GF_LOG_ERROR instead of hard code value of 4 here when
+     * GlusterFS makes GF_LOG_* macros available to libgfapi users.
+     */
+    ret = glfs_set_logging(glfs, "-", 4);
+    if (ret < 0) {
+        goto out;
+    }
+
+    ret = glfs_init(glfs);
+    if (ret) {
+        error_report("Gluster connection failed for server=%s port=%d "
+             "volume=%s image=%s transport=%s\n", gconf->server, gconf->port,
+             gconf->volname, gconf->image, gconf->transport);
+        goto out;
+    }
+    return glfs;
+
+out:
+    if (glfs) {
+        glfs_fini(glfs);
+    }
+    return NULL;
+}
+
+static void qemu_gluster_complete_aio(GlusterAIOCB *acb, BDRVGlusterState *s)
+{
+    int ret;
+    bool *finished = acb->finished;
+    BlockDriverCompletionFunc *cb = acb->common.cb;
+    void *opaque = acb->common.opaque;
+
+    if (!acb->ret || acb->ret == acb->size) {
+        ret = 0; /* Success */
+    } else if (acb->ret < 0) {
+        ret = acb->ret; /* Read/Write failed */
+    } else {
+        ret = -EIO; /* Partial read/write - fail it */
+    }
+
+    s->qemu_aio_count--;
+    qemu_aio_release(acb);
+    cb(opaque, ret);
+    if (finished) {
+        *finished = true;
+    }
+}
+
+static void qemu_gluster_aio_event_reader(void *opaque)
+{
+    BDRVGlusterState *s = opaque;
+    ssize_t ret;
+
+    do {
+        char *p = (char *)&s->event_acb;
+
+        ret = read(s->fds[GLUSTER_FD_READ], p + s->event_reader_pos,
+                   sizeof(s->event_acb) - s->event_reader_pos);
+        if (ret > 0) {
+            s->event_reader_pos += ret;
+            if (s->event_reader_pos == sizeof(s->event_acb)) {
+                s->event_reader_pos = 0;
+                qemu_gluster_complete_aio(s->event_acb, s);
+            }
+        }
+    } while (ret < 0 && errno == EINTR);
+}
+
+static int qemu_gluster_aio_flush_cb(void *opaque)
+{
+    BDRVGlusterState *s = opaque;
+
+    return (s->qemu_aio_count > 0);
+}
+
+static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
+    int bdrv_flags)
+{
+    BDRVGlusterState *s = bs->opaque;
+    int open_flags = 0;
+    int ret = 0;
+    GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
+
+    s->glfs = qemu_gluster_init(gconf, filename);
+    if (!s->glfs) {
+        ret = -errno;
+        goto out;
+    }
+
+    open_flags |=  O_BINARY;
+    open_flags &= ~O_ACCMODE;
+    if (bdrv_flags & BDRV_O_RDWR) {
+        open_flags |= O_RDWR;
+    } else {
+        open_flags |= O_RDONLY;
+    }
+
+    if ((bdrv_flags & BDRV_O_NOCACHE)) {
+        open_flags |= O_DIRECT;
+    }
+
+    s->fd = glfs_open(s->glfs, gconf->image, open_flags);
+    if (!s->fd) {
+        ret = -errno;
+        goto out;
+    }
+
+    ret = qemu_pipe(s->fds);
+    if (ret < 0) {
+        ret = -errno;
+        goto out;
+    }
+    fcntl(s->fds[GLUSTER_FD_READ], F_SETFL, O_NONBLOCK);
+    qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ],
+        qemu_gluster_aio_event_reader, NULL, qemu_gluster_aio_flush_cb, s);
+
+out:
+    qemu_gluster_gconf_free(gconf);
+    if (!ret) {
+        return ret;
+    }
+    if (s->fd) {
+        glfs_close(s->fd);
+    }
+    if (s->glfs) {
+        glfs_fini(s->glfs);
+    }
+    return ret;
+}
+
+static int qemu_gluster_create(const char *filename,
+        QEMUOptionParameter *options)
+{
+    struct glfs *glfs;
+    struct glfs_fd *fd;
+    int ret = 0;
+    int64_t total_size = 0;
+    GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
+
+    glfs = qemu_gluster_init(gconf, filename);
+    if (!glfs) {
+        ret = -errno;
+        goto out;
+    }
+
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            total_size = options->value.n / BDRV_SECTOR_SIZE;
+        }
+        options++;
+    }
+
+    fd = glfs_creat(glfs, gconf->image,
+        O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
+    if (!fd) {
+        ret = -errno;
+    } else {
+        if (glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
+            ret = -errno;
+        }
+        if (glfs_close(fd) != 0) {
+            ret = -errno;
+        }
+    }
+out:
+    qemu_gluster_gconf_free(gconf);
+    if (glfs) {
+        glfs_fini(glfs);
+    }
+    return ret;
+}
+
+static void qemu_gluster_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+    GlusterAIOCB *acb = (GlusterAIOCB *)blockacb;
+    bool finished = false;
+
+    acb->finished = &finished;
+    while (!finished) {
+        qemu_aio_wait();
+    }
+}
+
+static AIOPool gluster_aio_pool = {
+    .aiocb_size = sizeof(GlusterAIOCB),
+    .cancel = qemu_gluster_aio_cancel,
+};
+
+static int qemu_gluster_send_pipe(BDRVGlusterState *s, GlusterAIOCB *acb)
+{
+    int ret = 0;
+
+    while (1) {
+        int fd = s->fds[GLUSTER_FD_WRITE];
+
+        ret = write(fd, (void *)&acb, sizeof(acb));
+        if (ret >= 0) {
+            break;
+        }
+        if (errno == EINTR) {
+            continue;
+        }
+        if (errno != EAGAIN) {
+            break;
+        }
+    }
+    return ret;
+}
+
+static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg)
+{
+    GlusterAIOCB *acb = (GlusterAIOCB *)arg;
+    BlockDriverState *bs = acb->common.bs;
+    BDRVGlusterState *s = bs->opaque;
+
+    acb->ret = ret;
+    if (qemu_gluster_send_pipe(s, acb) < 0) {
+        /*
+         * Gluster AIO callback thread failed to notify the waiting
+         * QEMU thread about IO completion.
+         *
+         * Complete this IO request and make the disk inaccessible for
+         * subsequent reads and writes.
+         */
+        error_report("Gluster failed to notify QEMU about IO completion");
+
+        qemu_mutex_lock_iothread(); /* We are in gluster thread context */
+        acb->common.cb(acb->common.opaque, -EIO);
+        qemu_aio_release(acb);
+        s->qemu_aio_count--;
+        close(s->fds[GLUSTER_FD_READ]);
+        close(s->fds[GLUSTER_FD_WRITE]);
+        qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ], NULL, NULL, NULL,
+            NULL);
+        bs->drv = NULL; /* Make the disk inaccessible */
+        qemu_mutex_unlock_iothread();
+    }
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_rw(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque, int write)
+{
+    int ret;
+    GlusterAIOCB *acb;
+    BDRVGlusterState *s = bs->opaque;
+    size_t size;
+    off_t offset;
+
+    offset = sector_num * BDRV_SECTOR_SIZE;
+    size = nb_sectors * BDRV_SECTOR_SIZE;
+    s->qemu_aio_count++;
+
+    acb = qemu_aio_get(&gluster_aio_pool, bs, cb, opaque);
+    acb->size = size;
+    acb->ret = 0;
+    acb->finished = NULL;
+
+    if (write) {
+        ret = glfs_pwritev_async(s->fd, qiov->iov, qiov->niov, offset, 0,
+            &gluster_finish_aiocb, acb);
+    } else {
+        ret = glfs_preadv_async(s->fd, qiov->iov, qiov->niov, offset, 0,
+            &gluster_finish_aiocb, acb);
+    }
+
+    if (ret < 0) {
+        goto out;
+    }
+    return &acb->common;
+
+out:
+    s->qemu_aio_count--;
+    qemu_aio_release(acb);
+    return NULL;
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_readv(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    return qemu_gluster_aio_rw(bs, sector_num, qiov, nb_sectors, cb, opaque, 0);
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_writev(BlockDriverState *bs,
+        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    return qemu_gluster_aio_rw(bs, sector_num, qiov, nb_sectors, cb, opaque, 1);
+}
+
+static BlockDriverAIOCB *qemu_gluster_aio_flush(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    int ret;
+    GlusterAIOCB *acb;
+    BDRVGlusterState *s = bs->opaque;
+
+    acb = qemu_aio_get(&gluster_aio_pool, bs, cb, opaque);
+    acb->size = 0;
+    acb->ret = 0;
+    acb->finished = NULL;
+    s->qemu_aio_count++;
+
+    ret = glfs_fsync_async(s->fd, &gluster_finish_aiocb, acb);
+    if (ret < 0) {
+        goto out;
+    }
+    return &acb->common;
+
+out:
+    s->qemu_aio_count--;
+    qemu_aio_release(acb);
+    return NULL;
+}
+
+static int64_t qemu_gluster_getlength(BlockDriverState *bs)
+{
+    BDRVGlusterState *s = bs->opaque;
+    int64_t ret;
+
+    ret = glfs_lseek(s->fd, 0, SEEK_END);
+    if (ret < 0) {
+        return -errno;
+    } else {
+        return ret;
+    }
+}
+
+static int64_t qemu_gluster_allocated_file_size(BlockDriverState *bs)
+{
+    BDRVGlusterState *s = bs->opaque;
+    struct stat st;
+    int ret;
+
+    ret = glfs_fstat(s->fd, &st);
+    if (ret < 0) {
+        return -errno;
+    } else {
+        return st.st_blocks * 512;
+    }
+}
+
+static void qemu_gluster_close(BlockDriverState *bs)
+{
+    BDRVGlusterState *s = bs->opaque;
+
+    close(s->fds[GLUSTER_FD_READ]);
+    close(s->fds[GLUSTER_FD_WRITE]);
+    qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ], NULL, NULL, NULL, NULL);
+
+    if (s->fd) {
+        glfs_close(s->fd);
+        s->fd = NULL;
+    }
+    glfs_fini(s->glfs);
+}
+
+static QEMUOptionParameter qemu_gluster_create_options[] = {
+    {
+        .name = BLOCK_OPT_SIZE,
+        .type = OPT_SIZE,
+        .help = "Virtual disk size"
+    },
+    { NULL }
+};
+
+static BlockDriver bdrv_gluster = {
+    .format_name                  = "gluster",
+    .protocol_name                = "gluster",
+    .instance_size                = sizeof(BDRVGlusterState),
+    .bdrv_file_open               = qemu_gluster_open,
+    .bdrv_close                   = qemu_gluster_close,
+    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_getlength               = qemu_gluster_getlength,
+    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_aio_readv               = qemu_gluster_aio_readv,
+    .bdrv_aio_writev              = qemu_gluster_aio_writev,
+    .bdrv_aio_flush               = qemu_gluster_aio_flush,
+    .create_options               = qemu_gluster_create_options,
+};
+
+static BlockDriver bdrv_gluster_tcp = {
+    .format_name                  = "gluster",
+    .protocol_name                = "gluster+tcp",
+    .instance_size                = sizeof(BDRVGlusterState),
+    .bdrv_file_open               = qemu_gluster_open,
+    .bdrv_close                   = qemu_gluster_close,
+    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_getlength               = qemu_gluster_getlength,
+    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_aio_readv               = qemu_gluster_aio_readv,
+    .bdrv_aio_writev              = qemu_gluster_aio_writev,
+    .bdrv_aio_flush               = qemu_gluster_aio_flush,
+    .create_options               = qemu_gluster_create_options,
+};
+
+static BlockDriver bdrv_gluster_unix = {
+    .format_name                  = "gluster",
+    .protocol_name                = "gluster+unix",
+    .instance_size                = sizeof(BDRVGlusterState),
+    .bdrv_file_open               = qemu_gluster_open,
+    .bdrv_close                   = qemu_gluster_close,
+    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_getlength               = qemu_gluster_getlength,
+    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_aio_readv               = qemu_gluster_aio_readv,
+    .bdrv_aio_writev              = qemu_gluster_aio_writev,
+    .bdrv_aio_flush               = qemu_gluster_aio_flush,
+    .create_options               = qemu_gluster_create_options,
+};
+
+static BlockDriver bdrv_gluster_rdma = {
+    .format_name                  = "gluster",
+    .protocol_name                = "gluster+rdma",
+    .instance_size                = sizeof(BDRVGlusterState),
+    .bdrv_file_open               = qemu_gluster_open,
+    .bdrv_close                   = qemu_gluster_close,
+    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_getlength               = qemu_gluster_getlength,
+    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_aio_readv               = qemu_gluster_aio_readv,
+    .bdrv_aio_writev              = qemu_gluster_aio_writev,
+    .bdrv_aio_flush               = qemu_gluster_aio_flush,
+    .create_options               = qemu_gluster_create_options,
+};
+
+static void bdrv_gluster_init(void)
+{
+    bdrv_register(&bdrv_gluster_rdma);
+    bdrv_register(&bdrv_gluster_unix);
+    bdrv_register(&bdrv_gluster_tcp);
+    bdrv_register(&bdrv_gluster);
+}
+
+block_init(bdrv_gluster_init);

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-24  9:13 ` [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU " Bharata B Rao
@ 2012-09-24  9:26   ` Paolo Bonzini
  2012-09-24  9:44     ` Bharata B Rao
  2012-09-26 10:00   ` Kevin Wolf
  2012-10-03 15:50   ` [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend) Paolo Bonzini
  2 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-24  9:26 UTC (permalink / raw)
  To: bharata
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	qemu-devel, Stefan Hajnoczi, Harsh Bora, Amar Tumballi,
	Richard W.M. Jones, Daniel Veillard, Blue Swirl, Avi Kivity

> +static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
> +{
> +    URI *uri;
> +    QueryParams *qp = NULL;
> +    bool is_unix = false;
> +    int ret = 0;
> +    char *unescape_str = NULL;
> +
> +    uri = uri_parse(filename);
> +    if (!uri) {
> +        return -EINVAL;
> +    }
> +
> +    /* transport */
> +    if (!strcmp(uri->scheme, "gluster")) {
> +        gconf->transport = g_strdup("tcp");
> +    } else if (!strcmp(uri->scheme, "gluster+tcp")) {
> +        gconf->transport = g_strdup("tcp");
> +    } else if (!strcmp(uri->scheme, "gluster+unix")) {
> +        gconf->transport = g_strdup("unix");
> +        is_unix = true;
> +    } else if (!strcmp(uri->scheme, "gluster+rdma")) {
> +        gconf->transport = g_strdup("rdma");
> +    } else {
> +        ret = -EINVAL;
> +        goto out;
> +    }
> +
> +    ret = parse_volume_options(gconf, uri->path);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    if (uri->query) {
> +        unescape_str = uri_string_unescape(uri->query, -1, NULL);
> +        if (!unescape_str) {
> +            ret = -EINVAL;
> +            goto out;
> +        }
> +    }
> +
> +    qp = query_params_parse(unescape_str);

query_params_parse already does the unescaping.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-24  9:26   ` Paolo Bonzini
@ 2012-09-24  9:44     ` Bharata B Rao
  0 siblings, 0 replies; 26+ messages in thread
From: Bharata B Rao @ 2012-09-24  9:44 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	qemu-devel, Stefan Hajnoczi, Harsh Bora, Amar Tumballi,
	Richard W.M. Jones, Daniel Veillard, Blue Swirl, Avi Kivity

On Mon, Sep 24, 2012 at 05:26:53AM -0400, Paolo Bonzini wrote:
> > +    qp = query_params_parse(unescape_str);
> 
> query_params_parse already does the unescaping.

Hmm it failed to parse the options properly when I had an escape sequence,
hence resorted to unescaping the query string manually.

Look at the below gdb debug steps of your uri.c...

2302	    test("gluster+unix:///b?c=d%26e=f");
(gdb) s
test (x=0x4062c4 "gluster+unix:///b?c=d%26e=f") at uri.c:2279
2279	    URI *uri = uri_parse(x);
(gdb) n
2283	    if (!uri) {
(gdb) p *uri
$1 = {scheme = 0x607070 "gluster+unix", opaque = 0x0, authority = 0x0, 
  server = 0x0, user = 0x0, port = 0, path = 0x607090 "/b", fragment = 0x0, 
  cleanup = 0, query = 0x6070b0 "c=d%26e=f"}
(gdb) n
2289	    qp = query_params_parse(uri->query);
(gdb) p *qp
$2 = {n = 1, alloc = 1, p = 0x6070f0}

You can see that qp->n is still 1, but 2 was expected.

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library Bharata B Rao
@ 2012-09-24 10:27   ` Richard W.M. Jones
  2012-09-24 11:11     ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Richard W.M. Jones @ 2012-09-24 10:27 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Daniel Veillard, Blue Swirl, Avi Kivity, Paolo Bonzini


On Mon, Sep 24, 2012 at 02:42:02PM +0530, Bharata B Rao wrote:
> qemu: URI parsing library
> 
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Add a new URI parsing library to QEMU. The code has been borrowed from
> libxml2 and libvirt.

Rather than duplicating the libxml2 code, I think it would be better
to depend on libxml2.  While it's not ideal to add more external
dependencies, libxml2 is very commonly available.  Also libxml2 gets
frequent security updates and you never know when the URI code here
might be found to contain some sort of bug / security issue.

This will need a change to the qemu configure script of course.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-09-24 10:27   ` Richard W.M. Jones
@ 2012-09-24 11:11     ` Paolo Bonzini
  2012-09-27  9:03       ` Avi Kivity
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-24 11:11 UTC (permalink / raw)
  To: Richard W.M. Jones
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Daniel Veillard, Blue Swirl, Avi Kivity, Bharata B Rao



----- Messaggio originale -----
> Da: "Richard W.M. Jones" <rjones@redhat.com>
> A: "Bharata B Rao" <bharata@linux.vnet.ibm.com>
> Cc: qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>, "Kevin Wolf" <kwolf@redhat.com>, "Avi Kivity"
> <avi@redhat.com>, "Anthony Liguori" <aliguori@us.ibm.com>, "Stefan Hajnoczi" <stefanha@gmail.com>, "Blue Swirl"
> <blauwirbel@gmail.com>, "Anand Avati" <aavati@redhat.com>, "Vijay Bellur" <vbellur@redhat.com>, "Amar Tumballi"
> <amarts@redhat.com>, "Harsh Bora" <harsh@linux.vnet.ibm.com>, "Daniel Veillard" <veillard@redhat.com>
> Inviato: Lunedì, 24 settembre 2012 12:27:38
> Oggetto: Re: [PATCH v9 2/4] qemu: URI parsing library
> 
> 
> On Mon, Sep 24, 2012 at 02:42:02PM +0530, Bharata B Rao wrote:
> > qemu: URI parsing library
> > 
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > Add a new URI parsing library to QEMU. The code has been borrowed
> > from libxml2 and libvirt.
> 
> Rather than duplicating the libxml2 code, I think it would be better
> to depend on libxml2.  While it's not ideal to add more external
> dependencies, libxml2 is very commonly available.  Also libxml2 gets
> frequent security updates and you never know when the URI code here
> might be found to contain some sort of bug / security issue.

You're preaching to the choir here, but I think there are limited
advantages in this case.  The URI parsing has seen hardly any change
in the last 2 years and is surprisingly self-contained even in libxml2:
the dependencies were limited to memory allocation hooks basically.

A better plan would be to incorporate this code into glib, completing
the extremely sparse URI support that is already there.  However, we
would not be able to use it anyway, because we support compiling on old
glib versions.

Paolo

> This will need a change to the qemu configure script of course.
> 
> Rich.
> 
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones
> virt-df lists disk usage of guests without needing to install any
> software inside the virtual machine.  Supports Linux and Windows.
> http://et.redhat.com/~rjones/virt-df/
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-24  9:13 ` [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU " Bharata B Rao
  2012-09-24  9:26   ` Paolo Bonzini
@ 2012-09-26 10:00   ` Kevin Wolf
  2012-09-26 10:08     ` Paolo Bonzini
  2012-09-26 16:11     ` Bharata B Rao
  2012-10-03 15:50   ` [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend) Paolo Bonzini
  2 siblings, 2 replies; 26+ messages in thread
From: Kevin Wolf @ 2012-09-26 10:00 UTC (permalink / raw)
  To: bharata
  Cc: Anthony Liguori, Anand Avati, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, Paolo Bonzini, Daniel Veillard

Am 24.09.2012 11:13, schrieb Bharata B Rao:
> block: Support GlusterFS as a QEMU block backend.
> 
> From: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> This patch adds gluster as the new block backend in QEMU. This gives
> QEMU the ability to boot VM images from gluster volumes. Its already
> possible to boot from VM images on gluster volumes using FUSE mount, but
> this patchset provides the ability to boot VM images from gluster volumes
> by by-passing the FUSE layer in gluster. This is made possible by
> using libgfapi routines to perform IO on gluster volumes directly.
> 
> VM Image on gluster volume is specified like this:
> 
> file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
> 
> 'gluster' is the protocol.
> 
> '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.
> 
> 'server' specifies the server 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 [ ].
> If transport type is 'unix', then server field is ignored, but the
> 'socket' field needs to be populated with the path to unix domain
> socket.
> 
> 'port' is the port number on which glusterd is listening. This is optional
> and if not specified, QEMU will send 0 which will make gluster to use the
> default port. port is ignored for unix type of transport.
> 
> 'volname' is the name of the gluster volume which contains the VM image.
> 
> 'image' is the path to the actual VM image that resides on gluster volume.
> 
> Examples:
> 
> 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://server.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
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
> 
>  block/Makefile.objs |    1 
>  block/gluster.c     |  642 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 643 insertions(+), 0 deletions(-)
>  create mode 100644 block/gluster.c
> 
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index b5754d3..a1ae67f 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -9,3 +9,4 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o
>  block-obj-$(CONFIG_LIBISCSI) += iscsi.o
>  block-obj-$(CONFIG_CURL) += curl.o
>  block-obj-$(CONFIG_RBD) += rbd.o
> +block-obj-$(CONFIG_GLUSTERFS) += gluster.o
> diff --git a/block/gluster.c b/block/gluster.c
> new file mode 100644
> index 0000000..a2f8303
> --- /dev/null
> +++ b/block/gluster.c
> @@ -0,0 +1,642 @@
> +/*
> + * GlusterFS backend for QEMU
> + *
> + * Copyright (C) 2012 Bharata B Rao <bharata@linux.vnet.ibm.com>
> + *
> + * Pipe handling mechanism in AIO implementation is derived from
> + * block/rbd.c. Hence,
> + *
> + * Copyright (C) 2010-2011 Christian Brunner <chb@muc.de>,
> + *                         Josh Durgin <josh.durgin@dreamhost.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +#include <glusterfs/api/glfs.h>
> +#include "block_int.h"
> +#include "qemu_socket.h"
> +#include "uri.h"
> +
> +typedef struct GlusterAIOCB {
> +    BlockDriverAIOCB common;
> +    int64_t size;
> +    int ret;
> +    bool *finished;
> +    QEMUBH *bh;
> +} GlusterAIOCB;
> +
> +typedef struct BDRVGlusterState {
> +    struct glfs *glfs;
> +    int fds[2];
> +    struct glfs_fd *fd;
> +    int qemu_aio_count;
> +    int event_reader_pos;
> +    GlusterAIOCB *event_acb;
> +} BDRVGlusterState;
> +
> +#define GLUSTER_FD_READ  0
> +#define GLUSTER_FD_WRITE 1
> +
> +typedef struct GlusterConf {
> +    char *server;
> +    int port;
> +    char *volname;
> +    char *image;
> +    char *transport;
> +} GlusterConf;
> +
> +static void qemu_gluster_gconf_free(GlusterConf *gconf)
> +{
> +    g_free(gconf->server);
> +    g_free(gconf->volname);
> +    g_free(gconf->image);
> +    g_free(gconf->transport);
> +    g_free(gconf);
> +}
> +
> +static int parse_volume_options(GlusterConf *gconf, char *path)
> +{
> +    char *token, *saveptr;
> +
> +    /* volname */
> +    token = strtok_r(path, "/", &saveptr);
> +    if (!token) {
> +        return -EINVAL;
> +    }
> +    gconf->volname = g_strdup(token);
> +
> +    /* image */
> +    token = strtok_r(NULL, "?", &saveptr);

If I understand uri.c right, there is no ? in the path, so there's no
reason to call strtok. You could just use the rest of the string.

> +    if (!token) {
> +        return -EINVAL;
> +    }
> +    gconf->image = g_strdup(token);
> +    return 0;
> +}
> +
> +/*
> + * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
> + *
> + * 'gluster' is the protocol.
> + *
> + * '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.
> + *
> + * 'server' specifies the server 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 [ ].
> + * If transport type is 'unix', then server field is ignored, but the
> + * 'socket' field needs to be populated with the path to unix domain
> + * socket.
> + *
> + * 'port' is the port number on which glusterd is listening. This is optional
> + * and if not specified, QEMU will send 0 which will make gluster to use the
> + * default port. port is ignored for unix type of transport.
> + *
> + * 'volname' is the name of the gluster volume which contains the VM image.
> + *
> + * 'image' is the path to the actual VM image that resides on gluster volume.
> + *
> + * Examples:
> + *
> + * 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://server.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)
> +{
> +    URI *uri;
> +    QueryParams *qp = NULL;
> +    bool is_unix = false;
> +    int ret = 0;
> +    char *unescape_str = NULL;
> +
> +    uri = uri_parse(filename);
> +    if (!uri) {
> +        return -EINVAL;
> +    }
> +
> +    /* transport */
> +    if (!strcmp(uri->scheme, "gluster")) {
> +        gconf->transport = g_strdup("tcp");
> +    } else if (!strcmp(uri->scheme, "gluster+tcp")) {
> +        gconf->transport = g_strdup("tcp");
> +    } else if (!strcmp(uri->scheme, "gluster+unix")) {
> +        gconf->transport = g_strdup("unix");
> +        is_unix = true;
> +    } else if (!strcmp(uri->scheme, "gluster+rdma")) {
> +        gconf->transport = g_strdup("rdma");
> +    } else {
> +        ret = -EINVAL;
> +        goto out;
> +    }
> +
> +    ret = parse_volume_options(gconf, uri->path);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    if (uri->query) {
> +        unescape_str = uri_string_unescape(uri->query, -1, NULL);
> +        if (!unescape_str) {
> +            ret = -EINVAL;
> +            goto out;
> +        }
> +    }

I agree with Paolo here, this need to go away.

The example that you posted ("gluster+unix:///b?c=d%26e=f") has indeed
only one argument with name 'c' and value 'd&e=f'. If you unescape here,
you would incorrectly require double escaping.

> +
> +    qp = query_params_parse(unescape_str);
> +    if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
> +        ret = -EINVAL;
> +        goto out;
> +    }
> +
> +    if (is_unix) {
> +        if (strcmp(qp->p[0].name, "socket")) {
> +            ret = -EINVAL;
> +            goto out;
> +        }
> +        gconf->server = g_strdup(qp->p[0].value);

Maybe add a check that uri->server is empty?

> +    } else {
> +        gconf->server = g_strdup(uri->server);
> +        gconf->port = uri->port;
> +    }
> +
> +out:
> +    if (qp) {
> +        query_params_free(qp);
> +    }
> +    g_free(unescape_str);
> +    uri_free(uri);
> +    return ret;
> +}
> +
> +static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename)
> +{
> +    struct glfs *glfs = NULL;
> +    int ret;
> +
> +    ret = qemu_gluster_parseuri(gconf, filename);
> +    if (ret < 0) {
> +        error_report("Usage: file=gluster[+transport]://[server[:port]]/"
> +            "volname/image[?socket=...]");
> +        errno = -ret;
> +        goto out;
> +    }
> +
> +    glfs = glfs_new(gconf->volname);
> +    if (!glfs) {
> +        goto out;
> +    }
> +
> +    ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->server,
> +            gconf->port);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    /*
> +     * TODO: Use GF_LOG_ERROR instead of hard code value of 4 here when
> +     * GlusterFS makes GF_LOG_* macros available to libgfapi users.
> +     */
> +    ret = glfs_set_logging(glfs, "-", 4);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    ret = glfs_init(glfs);
> +    if (ret) {
> +        error_report("Gluster connection failed for server=%s port=%d "
> +             "volume=%s image=%s transport=%s\n", gconf->server, gconf->port,
> +             gconf->volname, gconf->image, gconf->transport);
> +        goto out;
> +    }
> +    return glfs;
> +
> +out:
> +    if (glfs) {
> +        glfs_fini(glfs);

Does this corrupt errno?

> +    }
> +    return NULL;
> +}
> +
> +static void qemu_gluster_complete_aio(GlusterAIOCB *acb, BDRVGlusterState *s)
> +{
> +    int ret;
> +    bool *finished = acb->finished;
> +    BlockDriverCompletionFunc *cb = acb->common.cb;
> +    void *opaque = acb->common.opaque;
> +
> +    if (!acb->ret || acb->ret == acb->size) {
> +        ret = 0; /* Success */
> +    } else if (acb->ret < 0) {
> +        ret = acb->ret; /* Read/Write failed */
> +    } else {
> +        ret = -EIO; /* Partial read/write - fail it */
> +    }
> +
> +    s->qemu_aio_count--;
> +    qemu_aio_release(acb);
> +    cb(opaque, ret);
> +    if (finished) {
> +        *finished = true;
> +    }
> +}
> +
> +static void qemu_gluster_aio_event_reader(void *opaque)
> +{
> +    BDRVGlusterState *s = opaque;
> +    ssize_t ret;
> +
> +    do {
> +        char *p = (char *)&s->event_acb;
> +
> +        ret = read(s->fds[GLUSTER_FD_READ], p + s->event_reader_pos,
> +                   sizeof(s->event_acb) - s->event_reader_pos);
> +        if (ret > 0) {
> +            s->event_reader_pos += ret;
> +            if (s->event_reader_pos == sizeof(s->event_acb)) {
> +                s->event_reader_pos = 0;
> +                qemu_gluster_complete_aio(s->event_acb, s);
> +            }
> +        }
> +    } while (ret < 0 && errno == EINTR);
> +}
> +
> +static int qemu_gluster_aio_flush_cb(void *opaque)
> +{
> +    BDRVGlusterState *s = opaque;
> +
> +    return (s->qemu_aio_count > 0);
> +}
> +
> +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
> +    int bdrv_flags)
> +{
> +    BDRVGlusterState *s = bs->opaque;
> +    int open_flags = 0;
> +    int ret = 0;
> +    GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
> +
> +    s->glfs = qemu_gluster_init(gconf, filename);
> +    if (!s->glfs) {
> +        ret = -errno;
> +        goto out;
> +    }
> +
> +    open_flags |=  O_BINARY;
> +    open_flags &= ~O_ACCMODE;

open_flags == O_BINARY here, so no O_ACCMODE bits to clear.

> +    if (bdrv_flags & BDRV_O_RDWR) {
> +        open_flags |= O_RDWR;
> +    } else {
> +        open_flags |= O_RDONLY;
> +    }
> +
> +    if ((bdrv_flags & BDRV_O_NOCACHE)) {
> +        open_flags |= O_DIRECT;
> +    }
> +
> +    s->fd = glfs_open(s->glfs, gconf->image, open_flags);
> +    if (!s->fd) {
> +        ret = -errno;
> +        goto out;
> +    }
> +
> +    ret = qemu_pipe(s->fds);
> +    if (ret < 0) {
> +        ret = -errno;
> +        goto out;
> +    }
> +    fcntl(s->fds[GLUSTER_FD_READ], F_SETFL, O_NONBLOCK);
> +    qemu_aio_set_fd_handler(s->fds[GLUSTER_FD_READ],
> +        qemu_gluster_aio_event_reader, NULL, qemu_gluster_aio_flush_cb, s);
> +
> +out:
> +    qemu_gluster_gconf_free(gconf);
> +    if (!ret) {
> +        return ret;
> +    }
> +    if (s->fd) {
> +        glfs_close(s->fd);
> +    }
> +    if (s->glfs) {
> +        glfs_fini(s->glfs);
> +    }
> +    return ret;
> +}
> +
> +static int qemu_gluster_create(const char *filename,
> +        QEMUOptionParameter *options)
> +{
> +    struct glfs *glfs;
> +    struct glfs_fd *fd;
> +    int ret = 0;
> +    int64_t total_size = 0;
> +    GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
> +
> +    glfs = qemu_gluster_init(gconf, filename);
> +    if (!glfs) {
> +        ret = -errno;
> +        goto out;
> +    }
> +
> +    while (options && options->name) {
> +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> +            total_size = options->value.n / BDRV_SECTOR_SIZE;
> +        }
> +        options++;
> +    }
> +
> +    fd = glfs_creat(glfs, gconf->image,
> +        O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
> +    if (!fd) {
> +        ret = -errno;
> +    } else {
> +        if (glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
> +            ret = -errno;
> +        }
> +        if (glfs_close(fd) != 0) {
> +            ret = -errno;
> +        }
> +    }
> +out:
> +    qemu_gluster_gconf_free(gconf);
> +    if (glfs) {
> +        glfs_fini(glfs);
> +    }
> +    return ret;
> +}
> +
> +static void qemu_gluster_aio_cancel(BlockDriverAIOCB *blockacb)
> +{
> +    GlusterAIOCB *acb = (GlusterAIOCB *)blockacb;
> +    bool finished = false;
> +
> +    acb->finished = &finished;
> +    while (!finished) {
> +        qemu_aio_wait();
> +    }
> +}
> +
> +static AIOPool gluster_aio_pool = {
> +    .aiocb_size = sizeof(GlusterAIOCB),
> +    .cancel = qemu_gluster_aio_cancel,
> +};
> +
> +static int qemu_gluster_send_pipe(BDRVGlusterState *s, GlusterAIOCB *acb)
> +{
> +    int ret = 0;
> +
> +    while (1) {
> +        int fd = s->fds[GLUSTER_FD_WRITE];
> +
> +        ret = write(fd, (void *)&acb, sizeof(acb));
> +        if (ret >= 0) {
> +            break;
> +        }
> +        if (errno == EINTR) {
> +            continue;
> +        }
> +        if (errno != EAGAIN) {
> +            break;
> +        }

Variatio delectat? ;-)

How about just do { ... } while (errno == EINTR || errno == EAGAIN); ?

> +    }
> +    return ret;
> +}

Kevin

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-26 10:00   ` Kevin Wolf
@ 2012-09-26 10:08     ` Paolo Bonzini
  2012-09-26 16:11     ` Bharata B Rao
  1 sibling, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-26 10:08 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Anthony Liguori, Anand Avati, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, bharata, Daniel Veillard

Il 26/09/2012 12:00, Kevin Wolf ha scritto:
>> > +
>> > +        ret = write(fd, (void *)&acb, sizeof(acb));
>> > +        if (ret >= 0) {
>> > +            break;
>> > +        }
>> > +        if (errno == EINTR) {
>> > +            continue;
>> > +        }
>> > +        if (errno != EAGAIN) {
>> > +            break;
>> > +        }
> Variatio delectat? ;-)
> 
> How about just do { ... } while (errno == EINTR || errno == EAGAIN); ?

That should be

while ((ret < 0) && (errno == EINTR || errno == EAGAIN));

However, fd here is blocking, so you can just use qemu_write_full.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-26 10:00   ` Kevin Wolf
  2012-09-26 10:08     ` Paolo Bonzini
@ 2012-09-26 16:11     ` Bharata B Rao
  2012-09-26 16:38       ` Paolo Bonzini
  1 sibling, 1 reply; 26+ messages in thread
From: Bharata B Rao @ 2012-09-26 16:11 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Anthony Liguori, Anand Avati, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, Paolo Bonzini, Daniel Veillard

On Wed, Sep 26, 2012 at 12:00:47PM +0200, Kevin Wolf wrote:
> Am 24.09.2012 11:13, schrieb Bharata B Rao:
> > +static int parse_volume_options(GlusterConf *gconf, char *path)
> > +{
> > +    char *token, *saveptr;
> > +
> > +    /* volname */
> > +    token = strtok_r(path, "/", &saveptr);
> > +    if (!token) {
> > +        return -EINVAL;
> > +    }
> > +    gconf->volname = g_strdup(token);
> > +
> > +    /* image */
> > +    token = strtok_r(NULL, "?", &saveptr);
> 
> If I understand uri.c right, there is no ? in the path, so there's no
> reason to call strtok. You could just use the rest of the string.

As you note, I don't need 2nd strtok strictly since the rest of the string
is available in saveptr. But I thought using saveptr is not ideal or preferred.
I wanted to use the most appropriate/safe delimiter to extract the image string
in the 2nd strtok and decided to use '?'.

If you think using saveptr is fine, then I could use that as below...

    /* image */
    if (!*saveptr) {
        return -EINVAL;
    }
    gconf->image = g_strdup(saveptr);

> 
> > +    if (!token) {
> > +        return -EINVAL;
> > +    }
> > +    gconf->image = g_strdup(token);
> > +    return 0;
> > +}
> > +
> > +
> > +    if (uri->query) {
> > +        unescape_str = uri_string_unescape(uri->query, -1, NULL);
> > +        if (!unescape_str) {
> > +            ret = -EINVAL;
> > +            goto out;
> > +        }
> > +    }
> 
> I agree with Paolo here, this need to go away.

Ok will do that.

> > +
> > +    if (is_unix) {
> > +        if (strcmp(qp->p[0].name, "socket")) {
> > +            ret = -EINVAL;
> > +            goto out;
> > +        }
> > +        gconf->server = g_strdup(qp->p[0].value);
> 
> Maybe add a check that uri->server is empty?

I am saying that we will ignore the server and port if
transport type is unix. But I guess I will add this check and change
the comments and patch description accordingly.

> 
> > +    } else {
> > +        gconf->server = g_strdup(uri->server);
> > +        gconf->port = uri->port;
> > +    }
> > +
> > +out:
> > +    if (qp) {
> > +        query_params_free(qp);
> > +    }
> > +    g_free(unescape_str);
> > +    uri_free(uri);
> > +    return ret;
> > +}
> > +
> > +static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename)
> > +{
> > +    struct glfs *glfs = NULL;
> > +    int ret;
> > +
> > +    ret = qemu_gluster_parseuri(gconf, filename);
> > +    if (ret < 0) {
> > +        error_report("Usage: file=gluster[+transport]://[server[:port]]/"
> > +            "volname/image[?socket=...]");
> > +        errno = -ret;
> > +        goto out;
> > +    }
> > +
> > +    glfs = glfs_new(gconf->volname);
> > +    if (!glfs) {
> > +        goto out;
> > +    }
> > +
> > +    ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->server,
> > +            gconf->port);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +
> > +    /*
> > +     * TODO: Use GF_LOG_ERROR instead of hard code value of 4 here when
> > +     * GlusterFS makes GF_LOG_* macros available to libgfapi users.
> > +     */
> > +    ret = glfs_set_logging(glfs, "-", 4);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +
> > +    ret = glfs_init(glfs);
> > +    if (ret) {
> > +        error_report("Gluster connection failed for server=%s port=%d "
> > +             "volume=%s image=%s transport=%s\n", gconf->server, gconf->port,
> > +             gconf->volname, gconf->image, gconf->transport);
> > +        goto out;
> > +    }
> > +    return glfs;
> > +
> > +out:
> > +    if (glfs) {
> > +        glfs_fini(glfs);
> 
> Does this corrupt errno?

Currently glfs_fini() isn't implemented and it returns -1. I guess it could
modify errno when its implemented. At one point of time, I had a logic to save
the errno value from previous calls and restore it to errno if glfs_fini()
fails, but that looked ugly since I had to save errno values from
4 previous calls. Should I just save the errno from glfs_init() since
that does most of the validation, connection establishment etc and is more
likely to fail ?

> > +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
> > +    int bdrv_flags)
> > +{
> > +    BDRVGlusterState *s = bs->opaque;
> > +    int open_flags = 0;
> > +    int ret = 0;
> > +    GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
> > +
> > +    s->glfs = qemu_gluster_init(gconf, filename);
> > +    if (!s->glfs) {
> > +        ret = -errno;
> > +        goto out;
> > +    }
> > +
> > +    open_flags |=  O_BINARY;
> > +    open_flags &= ~O_ACCMODE;
> 
> open_flags == O_BINARY here, so no O_ACCMODE bits to clear.

Right, will fix.

> 
> > +static int qemu_gluster_send_pipe(BDRVGlusterState *s, GlusterAIOCB *acb)
> > +{
> > +    int ret = 0;
> > +
> > +    while (1) {
> > +        int fd = s->fds[GLUSTER_FD_WRITE];
> > +
> > +        ret = write(fd, (void *)&acb, sizeof(acb));
> > +        if (ret >= 0) {
> > +            break;
> > +        }
> > +        if (errno == EINTR) {
> > +            continue;
> > +        }
> > +        if (errno != EAGAIN) {
> > +            break;
> > +        }
> 
> Variatio delectat? ;-)
> 
> How about just do { ... } while (errno == EINTR || errno == EAGAIN); ?

I will go with qemu_write_full(). With that I could get rid of
qemu_gluster_send_pipe() totally.

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-26 16:11     ` Bharata B Rao
@ 2012-09-26 16:38       ` Paolo Bonzini
  2012-09-27  6:41         ` Bharata B Rao
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-26 16:38 UTC (permalink / raw)
  To: bharata
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Daniel Veillard

Il 26/09/2012 18:11, Bharata B Rao ha scritto:
>>> +static int parse_volume_options(GlusterConf *gconf, char *path)
>>> > > +{
>>> > > +    char *token, *saveptr;
>>> > > +
>>> > > +    /* volname */
>>> > > +    token = strtok_r(path, "/", &saveptr);
>>> > > +    if (!token) {
>>> > > +        return -EINVAL;
>>> > > +    }
>>> > > +    gconf->volname = g_strdup(token);
>>> > > +
>>> > > +    /* image */
>>> > > +    token = strtok_r(NULL, "?", &saveptr);
>> > 
>> > If I understand uri.c right, there is no ? in the path, so there's no
>> > reason to call strtok. You could just use the rest of the string.

Actually there could be a %3F which uri.c would unescape to ? (only the
query part is left escaped), so your usage of strtok_r is incorrect.

> As you note, I don't need 2nd strtok strictly since the rest of the string
> is available in saveptr. But I thought using saveptr is not ideal or preferred.
> I wanted to use the most appropriate/safe delimiter to extract the image string
> in the 2nd strtok and decided to use '?'.

I don't think it is defined what saveptr points to.
http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm
says "the strtok_r subroutine also updates the Pointer parameter with
the starting address of the token following the first occurrence of the
Separators parameter".  I read this as:

    *saveptr = token + strlen(token) + 1;

which is consistent with this strtok example from the C standard:

    #include <string.h>
    static char str[] = "?a???b,";
    char *t;

    t = strtok(str, "?");  // t points to the token "a"
    t = strtok(str, ",");  // t points to the token "??b"

Have you tested this code with multiple consecutive slashes?

> If you think using saveptr is fine, then I could use that as below...
> 
>     /* image */
>     if (!*saveptr) {
>         return -EINVAL;
>     }
>     gconf->image = g_strdup(saveptr);
> 

I would avoid strtok_r completely:

    char *p = path + strcpsn(path, "/");
    if (*p == '\0') {
        return -EINVAL;
    }
    gconf->volname = g_strndup(path, p - path);

    p += strspn(p, "/");
    if (*p == '\0') {
        return -EINVAL;
    }
    gconf->image = g_strdup(p);

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-26 16:38       ` Paolo Bonzini
@ 2012-09-27  6:41         ` Bharata B Rao
  2012-09-27  7:19           ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Bharata B Rao @ 2012-09-27  6:41 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Daniel Veillard

On Wed, Sep 26, 2012 at 06:38:02PM +0200, Paolo Bonzini wrote:
> Il 26/09/2012 18:11, Bharata B Rao ha scritto:
> >>> +static int parse_volume_options(GlusterConf *gconf, char *path)
> >>> > > +{
> >>> > > +    char *token, *saveptr;
> >>> > > +
> >>> > > +    /* volname */
> >>> > > +    token = strtok_r(path, "/", &saveptr);
> >>> > > +    if (!token) {
> >>> > > +        return -EINVAL;
> >>> > > +    }
> >>> > > +    gconf->volname = g_strdup(token);
> >>> > > +
> >>> > > +    /* image */
> >>> > > +    token = strtok_r(NULL, "?", &saveptr);
> >> > 
> >> > If I understand uri.c right, there is no ? in the path, so there's no
> >> > reason to call strtok. You could just use the rest of the string.
> 
> Actually there could be a %3F which uri.c would unescape to ? (only the
> query part is left escaped), so your usage of strtok_r is incorrect.

Ok the approch of using 2 strtok as above would fail for URI's like this:

gluster+unix://server/volname/weird%3Fimage?socket=/path/to/socket

> 
> > As you note, I don't need 2nd strtok strictly since the rest of the string
> > is available in saveptr. But I thought using saveptr is not ideal or preferred.
> > I wanted to use the most appropriate/safe delimiter to extract the image string
> > in the 2nd strtok and decided to use '?'.
> 
> I don't think it is defined what saveptr points to.
> http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm
> says "the strtok_r subroutine also updates the Pointer parameter with
> the starting address of the token following the first occurrence of the
> Separators parameter".  I read this as:
> 
>     *saveptr = token + strlen(token) + 1;
> 
> which is consistent with this strtok example from the C standard:
> 
>     #include <string.h>
>     static char str[] = "?a???b,";
>     char *t;
> 
>     t = strtok(str, "?");  // t points to the token "a"
>     t = strtok(str, ",");  // t points to the token "??b"
> 
> Have you tested this code with multiple consecutive slashes?

Yes, both 2-strtok and strtok+saveptr approach work correctly (= as per
my expectation!) with multiple consecutive slashes. I do understand that
2-strtok approach will not work when we have %3F in the path component of
the URI.

For URIs like gluster://server/volname/path/to/image, both the approaches
extract image as "path/to/image".

For URIs like gluster://server/volname//path/to/image, both the approaches
extract image as "/path/to/image".

For gluster://server/volname////path/to/image, the image is extracted as
"//path/to/image".

> 
> > If you think using saveptr is fine, then I could use that as below...
> > 
> >     /* image */
> >     if (!*saveptr) {
> >         return -EINVAL;
> >     }
> >     gconf->image = g_strdup(saveptr);
> > 
> 
> I would avoid strtok_r completely:
> 
>     char *p = path + strcpsn(path, "/");
>     if (*p == '\0') {
>         return -EINVAL;
>     }
>     gconf->volname = g_strndup(path, p - path);
> 
>     p += strspn(p, "/");
>     if (*p == '\0') {
>         return -EINVAL;
>     }
>     gconf->image = g_strdup(p);

This isn't working because for a URI like
gluster://server/volname/path/to/image, uri_parse() will give
"/volname/path/to/image" in uri->path. I would have expected to see
uri->path as "volname/path/to/image" (without 1st slash).

Note that gluster is currently able to resolve image paths that don't
have a preceding slash (like dir/a.img). But I guess we should support
specifying complete image paths too (like /dir/a.img)

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-27  6:41         ` Bharata B Rao
@ 2012-09-27  7:19           ` Paolo Bonzini
  2012-09-27  8:28             ` Bharata B Rao
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-27  7:19 UTC (permalink / raw)
  To: bharata
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Daniel Veillard

Il 27/09/2012 08:41, Bharata B Rao ha scritto:
>>> As you note, I don't need 2nd strtok strictly since the rest of the string
>>> is available in saveptr. But I thought using saveptr is not ideal or preferred.
>>> I wanted to use the most appropriate/safe delimiter to extract the image string
>>> in the 2nd strtok and decided to use '?'.
>>
>> I don't think it is defined what saveptr points to.
>> http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm
>> says "the strtok_r subroutine also updates the Pointer parameter with
>> the starting address of the token following the first occurrence of the
>> Separators parameter".  I read this as:
>>
>>     *saveptr = token + strlen(token) + 1;
>>
>> which is consistent with this strtok example from the C standard:
>>
>>     #include <string.h>
>>     static char str[] = "?a???b,";
>>     char *t;
>>
>>     t = strtok(str, "?");  // t points to the token "a"
>>     t = strtok(str, ",");  // t points to the token "??b"
>>
>> Have you tested this code with multiple consecutive slashes?
> 
> Yes, both 2-strtok and strtok+saveptr approach work correctly (= as per
> my expectation!) with multiple consecutive slashes. I do understand that
> 2-strtok approach will not work when we have %3F in the path component of
> the URI.
> 
> For URIs like gluster://server/volname/path/to/image, both the approaches
> extract image as "path/to/image".
> 
> For URIs like gluster://server/volname//path/to/image, both the approaches
> extract image as "/path/to/image".
> 
> For gluster://server/volname////path/to/image, the image is extracted as
> "//path/to/image".

Should there be three /'s here?  I assume it's just a typo.

I'm concerned that there is no documentation of what saveptr actually
points to.  In many cases the strtok specification doesn't leave much
free room, but in the case you're testing here:

>>>     /* image */
>>>     if (!*saveptr) {
>>>         return -EINVAL;
>>>     }

strtok_r may just as well leave saveptr = NULL for example.

>>>     gconf->image = g_strdup(saveptr);
>>>
>>
>> I would avoid strtok_r completely:
>>
>>     char *p = path + strcpsn(path, "/");
>>     if (*p == '\0') {
>>         return -EINVAL;
>>     }
>>     gconf->volname = g_strndup(path, p - path);
>>
>>     p += strspn(p, "/");
>>     if (*p == '\0') {
>>         return -EINVAL;
>>     }
>>     gconf->image = g_strdup(p);
> 
> This isn't working because for a URI like
> gluster://server/volname/path/to/image, uri_parse() will give
> "/volname/path/to/image" in uri->path. I would have expected to see
> uri->path as "volname/path/to/image" (without 1st slash).

Ok, that's easy enough to fix with an extra strspn,

    char *p = path + strpsn(path, "/");
    p += strcspn(p, "/");


> Note that gluster is currently able to resolve image paths that don't
> have a preceding slash (like dir/a.img). But I guess we should support
> specifying complete image paths too (like /dir/a.img)

How would the URIs look like?

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-27  7:19           ` Paolo Bonzini
@ 2012-09-27  8:28             ` Bharata B Rao
  2012-09-27  8:31               ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Bharata B Rao @ 2012-09-27  8:28 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Daniel Veillard

On Thu, Sep 27, 2012 at 09:19:34AM +0200, Paolo Bonzini wrote:
> > 
> > For gluster://server/volname////path/to/image, the image is extracted as
> > "//path/to/image".
> 
> Should there be three /'s here?  I assume it's just a typo.

Yes it was a typo.

> 
> I'm concerned that there is no documentation of what saveptr actually
> points to.  In many cases the strtok specification doesn't leave much
> free room, but in the case you're testing here:
> 
> >>>     /* image */
> >>>     if (!*saveptr) {
> >>>         return -EINVAL;
> >>>     }
> 
> strtok_r may just as well leave saveptr = NULL for example.

Haven't seen that, but yes can't depend on that I suppose.

> 
> >>>     gconf->image = g_strdup(saveptr);
> >>>
> >>
> >> I would avoid strtok_r completely:
> >>
> >>     char *p = path + strcpsn(path, "/");
> >>     if (*p == '\0') {
> >>         return -EINVAL;
> >>     }
> >>     gconf->volname = g_strndup(path, p - path);
> >>
> >>     p += strspn(p, "/");
> >>     if (*p == '\0') {
> >>         return -EINVAL;
> >>     }
> >>     gconf->image = g_strdup(p);
> > 
> > This isn't working because for a URI like
> > gluster://server/volname/path/to/image, uri_parse() will give
> > "/volname/path/to/image" in uri->path. I would have expected to see
> > uri->path as "volname/path/to/image" (without 1st slash).
> 
> Ok, that's easy enough to fix with an extra strspn,
> 
>     char *p = path + strpsn(path, "/");
>     p += strcspn(p, "/");

Ok, this is how its looking finally...

    char *p, *q;
    p = q = path + strspn(path, "/");
    p += strcspn(p, "/");
    if (*p == '\0') {
        return -EINVAL;
    }
    gconf->volname = g_strndup(q, p - q);

    p += strspn(p, "/");
    if (*p == '\0') {
        return -EINVAL;
    }
    gconf->image = g_strdup(p);

> 
> 
> > Note that gluster is currently able to resolve image paths that don't
> > have a preceding slash (like dir/a.img). But I guess we should support
> > specifying complete image paths too (like /dir/a.img)
> 
> How would the URIs look like?

gluster://server/testvol//dir/a.img ?
Isn't this a valid URI ? Here volname=testvol and image=/dir/a.img.

If that's valid and needs to be supported, then the above strspn based
parsing logic needs some rewrite.

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
  2012-09-27  8:28             ` Bharata B Rao
@ 2012-09-27  8:31               ` Paolo Bonzini
  0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-27  8:31 UTC (permalink / raw)
  To: bharata
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Daniel Veillard

Il 27/09/2012 10:28, Bharata B Rao ha scritto:
>> > 
>> > How would the URIs look like?
> gluster://server/testvol//dir/a.img ?
> Isn't this a valid URI ? Here volname=testvol and image=/dir/a.img.

It is, but I think it would be canonicalized to
gluster://server/testvol/dir/a.img.

> If that's valid and needs to be supported, then the above strspn based
> parsing logic needs some rewrite.

I think we can avoid this, it is unintuitive.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-09-24 11:11     ` Paolo Bonzini
@ 2012-09-27  9:03       ` Avi Kivity
  2012-09-27  9:07         ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Avi Kivity @ 2012-09-27  9:03 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	qemu-devel, Blue Swirl, Bharata B Rao, Daniel Veillard

On 09/24/2012 01:11 PM, Paolo Bonzini wrote:
> 
> A better plan would be to incorporate this code into glib, completing
> the extremely sparse URI support that is already there.  However, we
> would not be able to use it anyway, because we support compiling on old
> glib versions.

If the same (or very similar) API is retained, we could fall back on
libxml2 when glib uri parsing is unavailable.  Eventually we bump our
minimum version requirement and drop the libxml2 dependency (or find out
that xml has invaded our code base and we can't get rid of it).

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-09-27  9:03       ` Avi Kivity
@ 2012-09-27  9:07         ` Paolo Bonzini
  2012-10-02  4:59           ` Daniel Veillard
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-09-27  9:07 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, Richard W.M. Jones,
	qemu-devel, Blue Swirl, Bharata B Rao, Daniel Veillard

Il 27/09/2012 11:03, Avi Kivity ha scritto:
> On 09/24/2012 01:11 PM, Paolo Bonzini wrote:
> > A better plan would be to incorporate this code into glib, completing
> > the extremely sparse URI support that is already there.  However, we
> > would not be able to use it anyway, because we support compiling on old
> > glib versions.
>
> If the same (or very similar) API is retained,

Yes, there is exactly one change in the API (modulo renaming) so we
could just use some #defines or wrappers.

> we could fall back on
> libxml2 when glib uri parsing is unavailable.

That's an interesting idea.  The assumption that glib wants URI parsing
is not proved, but it may work out.

Paolo

> Eventually we bump our
> minimum version requirement and drop the libxml2 dependency (or find out
> that xml has invaded our code base and we can't get rid of it).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-09-27  9:07         ` Paolo Bonzini
@ 2012-10-02  4:59           ` Daniel Veillard
  2012-10-02  6:16             ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel Veillard @ 2012-10-02  4:59 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Bharata B Rao

On Thu, Sep 27, 2012 at 11:07:49AM +0200, Paolo Bonzini wrote:
> Il 27/09/2012 11:03, Avi Kivity ha scritto:
> > On 09/24/2012 01:11 PM, Paolo Bonzini wrote:
> > > A better plan would be to incorporate this code into glib, completing
> > > the extremely sparse URI support that is already there.  However, we
> > > would not be able to use it anyway, because we support compiling on old
> > > glib versions.
> >
> > If the same (or very similar) API is retained,
> 
> Yes, there is exactly one change in the API (modulo renaming) so we
> could just use some #defines or wrappers.

  BTW I'm fine by this assuming:
   - someone keeps an eye on libxml2 upstream in case some security
     problem shows up in that part of the code
   - the code used is the 2.9.0 one where I added quite a bit of
     cleanup on memory handling.

> > we could fall back on
> > libxml2 when glib uri parsing is unavailable.
> 
> That's an interesting idea.  The assumption that glib wants URI parsing
> is not proved, but it may work out.

  Well glib coding style and libxml2 one are fairly different, assuming
you get the change into glib, that will be a relatively different code
base (my recollection is that glib exits on allocation error which tend
to impact API design so this may be more complex than just different
naming for the functions).

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
  2012-10-02  4:59           ` Daniel Veillard
@ 2012-10-02  6:16             ` Paolo Bonzini
  0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2012-10-02  6:16 UTC (permalink / raw)
  To: veillard
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Bharata B Rao


>   BTW I'm fine by this assuming:
>    - someone keeps an eye on libxml2 upstream in case some security
>      problem shows up in that part of the code

Yes.

>    - the code used is the 2.9.0 one where I added quite a bit of
>      cleanup on memory handling.

It is, modulo the fact that QEMU uses glib functions and thus exits on
allocation error.

The QEMU code would be a good starting point to add a glib URI module.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend)
  2012-09-24  9:13 ` [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU " Bharata B Rao
  2012-09-24  9:26   ` Paolo Bonzini
  2012-09-26 10:00   ` Kevin Wolf
@ 2012-10-03 15:50   ` Paolo Bonzini
  2012-10-03 17:58     ` Anand Avati
  2 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-10-03 15:50 UTC (permalink / raw)
  To: bharata
  Cc: Kevin Wolf, Anthony Liguori, Anand Avati, Vijay Bellur,
	Stefan Hajnoczi, Harsh Bora, Amar Tumballi, qemu-devel,
	Richard W.M. Jones, Blue Swirl, Avi Kivity, Daniel Veillard

Il 24/09/2012 11:13, Bharata B Rao ha scritto:
> +
> +    if ((bdrv_flags & BDRV_O_NOCACHE)) {
> +        open_flags |= O_DIRECT;
> +    }
> +

If I understand correctly what I was told, this prevents the brick
server from using its own buffer cache.  This is quite different from
what we do for example over NFS (where the host does no caching, but
nothing prevents it on the remote server).

I think these 3 lines should be removed.  We're bypassing the host
buffer cache just by virtue of using a userspace driver, and that's what
cache=none cares about.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend)
  2012-10-03 15:50   ` [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend) Paolo Bonzini
@ 2012-10-03 17:58     ` Anand Avati
  2012-10-03 18:17       ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Anand Avati @ 2012-10-03 17:58 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, bharata, Daniel Veillard

On 10/03/2012 08:50 AM, Paolo Bonzini wrote:
> Il 24/09/2012 11:13, Bharata B Rao ha scritto:
>> +
>> +    if ((bdrv_flags&  BDRV_O_NOCACHE)) {
>> +        open_flags |= O_DIRECT;
>> +    }
>> +
>
> If I understand correctly what I was told, this prevents the brick
> server from using its own buffer cache.  This is quite different from
> what we do for example over NFS (where the host does no caching, but
> nothing prevents it on the remote server).
>
> I think these 3 lines should be removed.  We're bypassing the host
> buffer cache just by virtue of using a userspace driver, and that's what
> cache=none cares about.
>
> Paolo

O_DIRECT also has an effect on the behavior of the "client side" (the 
part within the qemu) of Gluster stack as well. I presume the intention 
of O_DIRECT is to minimize use of memory (whether as host' page cache or 
buffered data in user space). To that end it is a good idea to leave 
O_DIRECT flag set.

The behavior of whether gluster bricks need to get the O_DIRECT 
propagated or not is a different issue. We are exploring the possibility 
of not sending O_DIRECT flag over the wire to mimic NFS behavior. That 
would be independent of the qemu block driver setting the open flag.

Avati

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend)
  2012-10-03 18:17       ` Paolo Bonzini
@ 2012-10-03 18:17         ` Anand Avati
  2012-10-03 18:27           ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Anand Avati @ 2012-10-03 18:17 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Anthony Liguori, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, bharata, Daniel Veillard

On 10/03/2012 11:17 AM, Paolo Bonzini wrote:
> Il 03/10/2012 19:58, Anand Avati ha scritto:
>>>
>>> I think these 3 lines should be removed.  We're bypassing the host
>>> buffer cache just by virtue of using a userspace driver, and that's what
>>> cache=none cares about.
>>
>> O_DIRECT also has an effect on the behavior of the "client side" (the
>> part within the qemu) of Gluster stack as well. I presume the intention
>> of O_DIRECT is to minimize use of memory (whether as host' page cache or
>> buffered data in user space). To that end it is a good idea to leave
>> O_DIRECT flag set.
>>
>> The behavior of whether gluster bricks need to get the O_DIRECT
>> propagated or not is a different issue. We are exploring the possibility
>> of not sending O_DIRECT flag over the wire to mimic NFS behavior. That
>> would be independent of the qemu block driver setting the open flag.
>
> What is the effect of O_DIRECT on the client exactly?

To avoid caching in the io-cache module, disable read-ahead etc (if 
those translators are loaded). The behavior in write-behind is tunable. 
You could either disable write-behind entirely (which will happen once 
libgfapi supports 0-copy/RDMA) or perform a sliding-window like 
size-limited write-behind (defaults to 1MB).

Avati

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend)
  2012-10-03 17:58     ` Anand Avati
@ 2012-10-03 18:17       ` Paolo Bonzini
  2012-10-03 18:17         ` Anand Avati
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2012-10-03 18:17 UTC (permalink / raw)
  To: Anand Avati
  Cc: Kevin Wolf, Anthony Liguori, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, bharata, Daniel Veillard

Il 03/10/2012 19:58, Anand Avati ha scritto:
>>
>> I think these 3 lines should be removed.  We're bypassing the host
>> buffer cache just by virtue of using a userspace driver, and that's what
>> cache=none cares about.
> 
> O_DIRECT also has an effect on the behavior of the "client side" (the
> part within the qemu) of Gluster stack as well. I presume the intention
> of O_DIRECT is to minimize use of memory (whether as host' page cache or
> buffered data in user space). To that end it is a good idea to leave
> O_DIRECT flag set.
> 
> The behavior of whether gluster bricks need to get the O_DIRECT
> propagated or not is a different issue. We are exploring the possibility
> of not sending O_DIRECT flag over the wire to mimic NFS behavior. That
> would be independent of the qemu block driver setting the open flag.

What is the effect of O_DIRECT on the client exactly?

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend)
  2012-10-03 18:17         ` Anand Avati
@ 2012-10-03 18:27           ` Paolo Bonzini
  0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2012-10-03 18:27 UTC (permalink / raw)
  To: Anand Avati
  Cc: Kevin Wolf, Anthony Liguori, Vijay Bellur, Stefan Hajnoczi,
	Harsh Bora, Amar Tumballi, qemu-devel, Richard W.M. Jones,
	Blue Swirl, Avi Kivity, bharata, Daniel Veillard

Il 03/10/2012 20:17, Anand Avati ha scritto:
> On 10/03/2012 11:17 AM, Paolo Bonzini wrote:
>> Il 03/10/2012 19:58, Anand Avati ha scritto:
>>>>
>>>> I think these 3 lines should be removed.  We're bypassing the host
>>>> buffer cache just by virtue of using a userspace driver, and that's
>>>> what
>>>> cache=none cares about.
>>>
>>> O_DIRECT also has an effect on the behavior of the "client side" (the
>>> part within the qemu) of Gluster stack as well. I presume the intention
>>> of O_DIRECT is to minimize use of memory (whether as host' page cache or
>>> buffered data in user space). To that end it is a good idea to leave
>>> O_DIRECT flag set.
>>>
>>> The behavior of whether gluster bricks need to get the O_DIRECT
>>> propagated or not is a different issue. We are exploring the possibility
>>> of not sending O_DIRECT flag over the wire to mimic NFS behavior. That
>>> would be independent of the qemu block driver setting the open flag.
>>
>> What is the effect of O_DIRECT on the client exactly?
> 
> To avoid caching in the io-cache module, disable read-ahead etc (if
> those translators are loaded). The behavior in write-behind is tunable.
> You could either disable write-behind entirely (which will happen once
> libgfapi supports 0-copy/RDMA) or perform a sliding-window like
> size-limited write-behind (defaults to 1MB).

Thanks for the information.

I think we need to benchmark it and see what is the actual difference in
memory usage vs. performance.  The usual reasons for cache=none (enable
Linux native AIO + allow migration with shared non-coherent storage) do
not apply to gluster.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2012-10-03 18:27 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-24  9:10 [Qemu-devel] [PATCH v9 0/4] GlusterFS support in QEMU - v9 Bharata B Rao
2012-09-24  9:10 ` [Qemu-devel] [PATCH v9 1/4] aio: Fix qemu_aio_wait() to maintain correct walking_handlers count Bharata B Rao
2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library Bharata B Rao
2012-09-24 10:27   ` Richard W.M. Jones
2012-09-24 11:11     ` Paolo Bonzini
2012-09-27  9:03       ` Avi Kivity
2012-09-27  9:07         ` Paolo Bonzini
2012-10-02  4:59           ` Daniel Veillard
2012-10-02  6:16             ` Paolo Bonzini
2012-09-24  9:12 ` [Qemu-devel] [PATCH v9 3/4] configure: Add a config option for GlusterFS as block backend Bharata B Rao
2012-09-24  9:13 ` [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU " Bharata B Rao
2012-09-24  9:26   ` Paolo Bonzini
2012-09-24  9:44     ` Bharata B Rao
2012-09-26 10:00   ` Kevin Wolf
2012-09-26 10:08     ` Paolo Bonzini
2012-09-26 16:11     ` Bharata B Rao
2012-09-26 16:38       ` Paolo Bonzini
2012-09-27  6:41         ` Bharata B Rao
2012-09-27  7:19           ` Paolo Bonzini
2012-09-27  8:28             ` Bharata B Rao
2012-09-27  8:31               ` Paolo Bonzini
2012-10-03 15:50   ` [Qemu-devel] O_DIRECT on glusterfs (was Re: [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend) Paolo Bonzini
2012-10-03 17:58     ` Anand Avati
2012-10-03 18:17       ` Paolo Bonzini
2012-10-03 18:17         ` Anand Avati
2012-10-03 18:27           ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.