8:31 a.m.
This patchset provides support for Gluster protocol based network disks.
Changelog:
v2:
- Addressed review comments by Jiri
- Updated patcheset as per new URI spec
Ref: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html
v1:
- Initial prototype
Harsh Prateek Bora (2):
Qemu/Gluster: Add Gluster protocol as supported network disk formats.
tests: Add tests for gluster protocol based network disks support
docs/schemas/domaincommon.rng | 8 +
src/conf/domain_conf.c | 28 ++-
src/conf/domain_conf.h | 11 ++
src/libvirt_private.syms | 2 +
src/qemu/qemu_command.c | 204 +++++++++++++++++++++
tests/qemuargv2xmltest.c | 1 +
.../qemuxml2argv-disk-drive-network-gluster.args | 1 +
.../qemuxml2argv-disk-drive-network-gluster.xml | 33 ++++
tests/qemuxml2argvtest.c | 2 +
9 files changed, 288 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml
--
1.7.11.4
8:31 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Qemu accepts gluster protocol as supported storage backend beside others.
This patch allows users to specify disks on gluster backends like this:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='gluster' name='volume/image'>
<host name='example.org' port='6000'
transport='tcp'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>
Note: In the <host> element above, transport is an optional attribute.
Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
If transport type is unix, host name specifies path to unix socket.
Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
---
docs/schemas/domaincommon.rng | 8 ++
src/conf/domain_conf.c | 28 +++++-
src/conf/domain_conf.h | 11 +++
src/libvirt_private.syms | 2 +
src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 251 insertions(+), 2 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f47fdad..89d9b9f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1048,6 +1048,7 @@
<value>nbd</value>
<value>rbd</value>
<value>sheepdog</value>
+ <value>gluster</value>
</choice>
</attribute>
<optional>
@@ -1061,6 +1062,13 @@
<attribute name="port">
<ref name="unsignedInt"/>
</attribute>
+ <attribute name="transport">
+ <choice>
+ <value>tcp</value>
+ <value>unix</value>
+ <value>rdma</value>
+ </choice>
+ </attribute>
</element>
</zeroOrMore>
<empty/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33e1e7f..838f079 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -214,7 +214,13 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST,
"nbd",
"rbd",
- "sheepdog")
+ "sheepdog",
+ "gluster")
+
+VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
+ "tcp",
+ "unix",
+ "rdma")
VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST,
"none",
@@ -3460,6 +3466,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
char *source = NULL;
char *target = NULL;
char *protocol = NULL;
+ char *protocol_transport;
char *trans = NULL;
virDomainDiskHostDefPtr hosts = NULL;
int nhosts = 0;
@@ -3566,6 +3573,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
}
hosts[nhosts].name = NULL;
hosts[nhosts].port = NULL;
+ hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
nhosts++;
hosts[nhosts - 1].name = virXMLPropString(child,
"name");
@@ -3580,6 +3588,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
"%s", _("missing port for
host"));
goto error;
}
+ /* transport can be tcp (default), unix or rdma. */
+ protocol_transport = virXMLPropString(child,
"transport");
+ if (protocol_transport != NULL) {
+ hosts[nhosts - 1].transport =
virDomainDiskProtocolTransportTypeFromString(protocol_transport);
+ if (hosts[nhosts - 1].transport < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown protocol transport
type '%s'"),
+ protocol_transport);
+ goto error;
+ }
+ }
}
child = child->next;
}
@@ -11756,8 +11775,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
for (i = 0; i < def->nhosts; i++) {
virBufferEscapeString(buf, " <host
name='%s'",
def->hosts[i].name);
- virBufferEscapeString(buf, " port='%s'/>\n",
+ virBufferEscapeString(buf, " port='%s'",
def->hosts[i].port);
+ if (def->hosts[i].transport) {
+ virBufferAsprintf(buf, " transport='%s'",
+
virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport));
+ }
+ virBufferAddLit(buf, "/>\n");
}
virBufferAddLit(buf, " </source>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14dead3..7ba7e9b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -456,10 +456,19 @@ enum virDomainDiskProtocol {
VIR_DOMAIN_DISK_PROTOCOL_NBD,
VIR_DOMAIN_DISK_PROTOCOL_RBD,
VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
+ VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
VIR_DOMAIN_DISK_PROTOCOL_LAST
};
+enum virDomainDiskProtocolTransport {
+ VIR_DOMAIN_DISK_PROTO_TRANS_TCP,
+ VIR_DOMAIN_DISK_PROTO_TRANS_UNIX,
+ VIR_DOMAIN_DISK_PROTO_TRANS_RDMA,
+
+ VIR_DOMAIN_DISK_PROTO_TRANS_LAST
+};
+
enum virDomainDiskTray {
VIR_DOMAIN_DISK_TRAY_CLOSED,
VIR_DOMAIN_DISK_TRAY_OPEN,
@@ -481,6 +490,7 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr;
struct _virDomainDiskHostDef {
char *name;
char *port;
+ int transport; /* enum virDomainDiskProtocolTransport */
};
enum virDomainDiskIo {
@@ -2166,6 +2176,7 @@ VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
VIR_ENUM_DECL(virDomainDiskErrorPolicy)
VIR_ENUM_DECL(virDomainDiskProtocol)
+VIR_ENUM_DECL(virDomainDiskProtocolTransport)
VIR_ENUM_DECL(virDomainDiskIo)
VIR_ENUM_DECL(virDomainDiskSecretType)
VIR_ENUM_DECL(virDomainDiskTray)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dab607a..8e70837 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -350,6 +350,8 @@ virDomainDiskInsertPreAlloced;
virDomainDiskIoTypeFromString;
virDomainDiskIoTypeToString;
virDomainDiskPathByName;
+virDomainDiskProtocolTransportTypeFromString;
+virDomainDiskProtocolTransportTypeToString;
virDomainDiskRemove;
virDomainDiskRemoveByName;
virDomainDiskTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 20730a9..0bed2bc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2021,6 +2021,168 @@ no_memory:
return -1;
}
+static int qemuParseGlusterString(virDomainDiskDefPtr def)
+{
+ char *host = NULL, *port = NULL, *volimg, *transp = NULL, *marker;
+
+ if (STRPREFIX(def->src, "+")) {
+ transp = def->src;
+ transp++;
+ marker = strstr(def->src, "://");
+ *marker++ = '\0';
+ marker += 2;
+ } else {
+ /* transport type not specified, use tcp as default */
+ def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
+ marker = strstr(def->src, "://");
+ marker += 3;
+ }
+
+ if (transp) {
+ def->hosts->transport =
virDomainDiskProtocolTransportTypeFromString(transp);
+ if (def->hosts->transport < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid gluster transport type '%s'"),
transp);
+ return -1;
+ }
+ }
+
+ /* now marker points to string which can start with one of the following:
+ * IPv6 address in square brackets followed by port (optional)
+ * <hostname> or <IPv4 address> followed by port (optional)
+ * '/' if its a unix socket followed by path to gluster disk volume/image
+ */
+
+ /* parse server, port */
+ if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (STRPREFIX(marker, "[")) {
+ /* IPv6 addr */
+ host = ++marker;
+ marker = strchr(host, ']');
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse IPv6 addr for gluster host %s
"), host);
+ return -1;
+ }
+ *marker++ = '\0';
+ /* parse port if specified */
+ if (STRPREFIX(marker, ":")) {
+ port = ++marker;
+ marker = strchr(port, '/');
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse filename for gluster disk %s
"), port);
+ return -1;
+ }
+ *marker++ = '\0';
+ } else {
+ marker++; /* port not specified, skip slash */
+ }
+ } else {
+ /* IPv4 address / hostname followed by port (optional) */
+ host = marker;
+ marker = strchr(host, '/'); /* skip to path to gluster disk vol/img
*/
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse filename for gluster disk %s "),
port);
+ return -1;
+ }
+ *marker++ = '\0';
+ port = strchr(host, ':');
+ if (port) {
+ /* port was specified with host, separate both */
+ *port++ = '\0';
+ }
+ }
+
+ /* host points to hostname / IPv4 / IPv6 addr
+ * port points to port or NULL is port was not specified
+ */
+ } else {
+ /* transport type is unix, expect one more slash
+ * followed by path to gluster disk vol/img */
+ if (STRPREFIX(marker, "/")) {
+ marker++;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Gluster unix transport url starts with 3 slashes i.e.
gluster+unix:///"));
+ return -1;
+ }
+ }
+
+ /* marker now points to path to gluster disk vol/img */
+ volimg = marker;
+
+ /* if transport type = unix, path to gluster disk vol/img
+ * is followed by ?socket=<path/to/socket> */
+ if (def->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (strstr(marker, "?socket=")) {
+ /* In libvirt xml, socket path is to be provided as
+ * <host name='/path/to/socket' port='0'>
+ */
+ host = strchr(marker, '=');
+ *host++ = '\0';
+ }
+ }
+
+ if (VIR_ALLOC(def->hosts) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+ def->nhosts = 1;
+ def->hosts->name = host;
+ if (port) {
+ def->hosts->port = port;
+ } else {
+ def->hosts->port = strdup("0");
+ }
+ if (!def->hosts->port) {
+ virReportOOMError();
+ return -1;
+ }
+ def->src = strdup(volimg);
+ if (!def->src) {
+ virReportOOMError();
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
+{
+ int ret = 0;
+ virBufferAddLit(opt, "file=");
+ if (disk->nhosts != 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("gluster accepts only one host"));
+ ret = -1;
+ } else {
+ virBufferAsprintf(opt, "gluster+%s://",
+
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
+
+ /* if transport type is not unix, specify server:port */
+ if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (strstr(disk->hosts->name, ":")) {
+ /* if IPv6 addr, use square brackets to enclose it */
+ virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
disk->hosts->port);
+ } else {
+ virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
disk->hosts->port);
+ }
+ }
+
+ /* append source path to gluster disk image */
+ virBufferAsprintf(opt, "/%s", disk->src);
+
+ /* if transport type is unix, server name is path to unix socket, ignore port */
+ if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ virBufferAsprintf(opt, "?socket=%s", disk->hosts->name);
+ }
+ }
+ return ret;
+}
+
char *
qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainDiskDefPtr disk,
@@ -2162,6 +2324,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
goto error;
virBufferAddChar(&opt, ',');
break;
+ case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+ if (qemuBuildGlusterString(disk, &opt) < 0)
+ goto error;
+ virBufferAddChar(&opt, ',');
+ break;
+
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
if (disk->nhosts == 0) {
virBufferEscape(&opt, ',', ",",
"file=sheepdog:%s,",
@@ -5242,6 +5410,18 @@ qemuBuildCommandLine(virConnectPtr conn,
file = virBufferContentAndReset(&opt);
}
break;
+ case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+ {
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ if (qemuBuildGlusterString(disk, &opt) < 0)
+ goto error;
+ if (virBufferError(&opt)) {
+ virReportOOMError();
+ goto error;
+ }
+ file = virBufferContentAndReset(&opt);
+ }
+ break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
if (disk->nhosts == 0) {
if (virAsprintf(&file, "sheepdog:%s,",
disk->src) < 0) {
@@ -6937,6 +7117,21 @@ qemuParseCommandLineDisk(virCapsPtr caps,
goto cleanup;
VIR_FREE(p);
+ } else if (STRPREFIX(def->src, "gluster")) {
+ char *p = def->src;
+
+ def->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
+ def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
+ def->src = strdup(p + strlen("gluster"));
+ if (!def->src) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (qemuParseGlusterString(def) < 0)
+ goto cleanup;
+
+ VIR_FREE(p);
} else if (STRPREFIX(def->src, "sheepdog:")) {
char *p = def->src;
char *port, *vdi;
@@ -8126,6 +8321,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
val += strlen("rbd:");
+ } else if (STRPREFIX(val, "gluster")) {
+ disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
+ disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
+ val += strlen("gluster");
} else if (STRPREFIX(val, "sheepdog:")) {
disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG;
@@ -8211,6 +8410,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
goto no_memory;
}
break;
+ case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+ if (qemuParseGlusterString(disk) < 0)
+ goto error;
+
+ break;
}
}
--
1.7.11.4
9:02 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Thu, Oct 04, 2012 at 19:01:58 +0530, Harsh Prateek Bora wrote:
Qemu accepts gluster protocol as supported storage backend beside
others.
This patch allows users to specify disks on gluster backends like this:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='gluster' name='volume/image'>
<host name='example.org' port='6000'
transport='tcp'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>
Note: In the <host> element above, transport is an optional attribute.
Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
If transport type is unix, host name specifies path to unix socket.
Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
---
docs/schemas/domaincommon.rng | 8 ++
src/conf/domain_conf.c | 28 +++++-
src/conf/domain_conf.h | 11 +++
src/libvirt_private.syms | 2 +
src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 251 insertions(+), 2 deletions(-)
Oh, I just realized that this patch is missing documentation in
docs/formatdomain.html.in. Sorry for not noticing that in v1. Anyway, no need
to resend the whole series just for this. It's sufficient if you send an
additional patch to add just the documentation and it can be squashed while
pushing or in v3, depending what the result of v2 review is going to be.
I'll do a proper review later.
Jirka
2:14 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/04/2012 07:32 PM, Jiri Denemark wrote:
On Thu, Oct 04, 2012 at 19:01:58 +0530, Harsh Prateek Bora wrote:
> Qemu accepts gluster protocol as supported storage backend beside others.
> This patch allows users to specify disks on gluster backends like this:
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw'/>
> <source protocol='gluster' name='volume/image'>
> <host name='example.org' port='6000'
transport='tcp'/>
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
>
> Note: In the <host> element above, transport is an optional attribute.
> Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
> If transport type is unix, host name specifies path to unix socket.
>
> Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
> ---
> docs/schemas/domaincommon.rng | 8 ++
> src/conf/domain_conf.c | 28 +++++-
> src/conf/domain_conf.h | 11 +++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 251 insertions(+), 2 deletions(-)
Oh, I just realized that this patch is missing documentation in
docs/formatdomain.html.in. Sorry for not noticing that in v1. Anyway, no need
to resend the whole series just for this. It's sufficient if you send an
additional patch to add just the documentation and it can be squashed while
pushing or in v3, depending what the result of v2 review is going to be.
Hm, I should have included that in v1 as well, will send a patch or
include in v3 if there are other changes required.
I'll do a proper review later.
Thanks,
Harsh
Jirka
10:47 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/04/2012 07:01 PM, Harsh Prateek Bora wrote:
Qemu accepts gluster protocol as supported storage backend beside
others.
This patch allows users to specify disks on gluster backends like this:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='gluster' name='volume/image'>
<host name='example.org' port='6000'
transport='tcp'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>
Note: In the <host> element above, transport is an optional attribute.
Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
If transport type is unix, host name specifies path to unix socket.
Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
---
docs/schemas/domaincommon.rng | 8 ++
src/conf/domain_conf.c | 28 +++++-
src/conf/domain_conf.h | 11 +++
src/libvirt_private.syms | 2 +
src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 251 insertions(+), 2 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f47fdad..89d9b9f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1048,6 +1048,7 @@
<value>nbd</value>
<value>rbd</value>
<value>sheepdog</value>
+ <value>gluster</value>
</choice>
</attribute>
<optional>
@@ -1061,6 +1062,13 @@
<attribute name="port">
<ref name="unsignedInt"/>
</attribute>
+ <attribute name="transport">
+ <choice>
+ <value>tcp</value>
+ <value>unix</value>
+ <value>rdma</value>
+ </choice>
+ </attribute>
'transport' attribute is optional,
so it should be placed inside
<optional> </optional> ?
</element>
</zeroOrMore>
<empty/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33e1e7f..838f079 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -214,7 +214,13 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST,
"nbd",
"rbd",
- "sheepdog")
+ "sheepdog",
+ "gluster")
+
+VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
+ "tcp",
+ "unix",
+ "rdma")
VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST,
"none",
@@ -3460,6 +3466,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
char *source = NULL;
char *target = NULL;
char *protocol = NULL;
+ char *protocol_transport;
char *trans = NULL;
virDomainDiskHostDefPtr hosts = NULL;
int nhosts = 0;
@@ -3566,6 +3573,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
}
hosts[nhosts].name = NULL;
hosts[nhosts].port = NULL;
+ hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
nhosts++;
hosts[nhosts - 1].name = virXMLPropString(child,
"name");
@@ -3580,6 +3588,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
"%s", _("missing port for
host"));
goto error;
}
+ /* transport can be tcp (default), unix or rdma. */
+ protocol_transport = virXMLPropString(child,
"transport");
+ if (protocol_transport != NULL) {
+ hosts[nhosts - 1].transport =
virDomainDiskProtocolTransportTypeFromString(protocol_transport);
+ if (hosts[nhosts - 1].transport < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown protocol transport
type '%s'"),
+ protocol_transport);
+ goto error;
+ }
+ }
}
child = child->next;
}
@@ -11756,8 +11775,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
for (i = 0; i < def->nhosts; i++) {
virBufferEscapeString(buf, " <host
name='%s'",
def->hosts[i].name);
- virBufferEscapeString(buf, " port='%s'/>\n",
+ virBufferEscapeString(buf, " port='%s'",
def->hosts[i].port);
+ if (def->hosts[i].transport) {
+ virBufferAsprintf(buf, " transport='%s'",
+
virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport));
+ }
+ virBufferAddLit(buf, "/>\n");
}
virBufferAddLit(buf, " </source>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14dead3..7ba7e9b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -456,10 +456,19 @@ enum virDomainDiskProtocol {
VIR_DOMAIN_DISK_PROTOCOL_NBD,
VIR_DOMAIN_DISK_PROTOCOL_RBD,
VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
+ VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
VIR_DOMAIN_DISK_PROTOCOL_LAST
};
+enum virDomainDiskProtocolTransport {
+ VIR_DOMAIN_DISK_PROTO_TRANS_TCP,
+ VIR_DOMAIN_DISK_PROTO_TRANS_UNIX,
+ VIR_DOMAIN_DISK_PROTO_TRANS_RDMA,
+
+ VIR_DOMAIN_DISK_PROTO_TRANS_LAST
+};
+
enum virDomainDiskTray {
VIR_DOMAIN_DISK_TRAY_CLOSED,
VIR_DOMAIN_DISK_TRAY_OPEN,
@@ -481,6 +490,7 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr;
struct _virDomainDiskHostDef {
char *name;
char *port;
+ int transport; /* enum virDomainDiskProtocolTransport */
};
enum virDomainDiskIo {
@@ -2166,6 +2176,7 @@ VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
VIR_ENUM_DECL(virDomainDiskErrorPolicy)
VIR_ENUM_DECL(virDomainDiskProtocol)
+VIR_ENUM_DECL(virDomainDiskProtocolTransport)
VIR_ENUM_DECL(virDomainDiskIo)
VIR_ENUM_DECL(virDomainDiskSecretType)
VIR_ENUM_DECL(virDomainDiskTray)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dab607a..8e70837 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -350,6 +350,8 @@ virDomainDiskInsertPreAlloced;
virDomainDiskIoTypeFromString;
virDomainDiskIoTypeToString;
virDomainDiskPathByName;
+virDomainDiskProtocolTransportTypeFromString;
+virDomainDiskProtocolTransportTypeToString;
virDomainDiskRemove;
virDomainDiskRemoveByName;
virDomainDiskTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 20730a9..0bed2bc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2021,6 +2021,168 @@ no_memory:
return -1;
}
+static int qemuParseGlusterString(virDomainDiskDefPtr def)
+{
+ char *host = NULL, *port = NULL, *volimg, *transp = NULL, *marker;
+
+ if (STRPREFIX(def->src, "+")) {
+ transp = def->src;
+ transp++;
+ marker = strstr(def->src, "://");
+ *marker++ = '\0';
+ marker += 2;
+ } else {
+ /* transport type not specified, use tcp as default */
+ def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
+ marker = strstr(def->src, "://");
+ marker += 3;
+ }
+
+ if (transp) {
+ def->hosts->transport =
virDomainDiskProtocolTransportTypeFromString(transp);
+ if (def->hosts->transport < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid gluster transport type '%s'"),
transp);
+ return -1;
+ }
+ }
+
+ /* now marker points to string which can start with one of the following:
+ * IPv6 address in square brackets followed by port (optional)
+ * <hostname> or <IPv4 address> followed by port (optional)
+ * '/' if its a unix socket followed by path to gluster disk volume/image
+ */
+
+ /* parse server, port */
+ if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (STRPREFIX(marker, "[")) {
+ /* IPv6 addr */
+ host = ++marker;
+ marker = strchr(host, ']');
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse IPv6 addr for gluster host %s
"), host);
+ return -1;
+ }
+ *marker++ = '\0';
+ /* parse port if specified */
+ if (STRPREFIX(marker, ":")) {
+ port = ++marker;
+ marker = strchr(port, '/');
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse filename for gluster disk %s
"), port);
+ return -1;
+ }
+ *marker++ = '\0';
+ } else {
+ marker++; /* port not specified, skip slash */
+ }
+ } else {
+ /* IPv4 address / hostname followed by port (optional) */
+ host = marker;
+ marker = strchr(host, '/'); /* skip to path to gluster disk vol/img
*/
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse filename for gluster disk %s
"), port);
+ return -1;
+ }
+ *marker++ = '\0';
+ port = strchr(host, ':');
+ if (port) {
+ /* port was specified with host, separate both */
+ *port++ = '\0';
+ }
+ }
+
+ /* host points to hostname / IPv4 / IPv6 addr
+ * port points to port or NULL is port was not specified
+ */
+ } else {
+ /* transport type is unix, expect one more slash
+ * followed by path to gluster disk vol/img */
+ if (STRPREFIX(marker, "/")) {
+ marker++;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Gluster unix transport url starts with 3 slashes i.e.
gluster+unix:///"));
+ return -1;
+ }
+ }
+
+ /* marker now points to path to gluster disk vol/img */
+ volimg = marker;
+
+ /* if transport type = unix, path to gluster disk vol/img
+ * is followed by ?socket=<path/to/socket> */
+ if (def->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (strstr(marker, "?socket=")) {
+ /* In libvirt xml, socket path is to be provided as
+ * <host name='/path/to/socket' port='0'>
+ */
+ host = strchr(marker, '=');
+ *host++ = '\0';
+ }
+ }
+
+ if (VIR_ALLOC(def->hosts) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+ def->nhosts = 1;
+ def->hosts->name = host;
+ if (port) {
+ def->hosts->port = port;
+ } else {
+ def->hosts->port = strdup("0");
+ }
+ if (!def->hosts->port) {
+ virReportOOMError();
+ return -1;
+ }
+ def->src = strdup(volimg);
+ if (!def->src) {
+ virReportOOMError();
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
+{
+ int ret = 0;
+ virBufferAddLit(opt, "file=");
+ if (disk->nhosts != 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("gluster accepts only one host"));
+ ret = -1;
+ } else {
+ virBufferAsprintf(opt, "gluster+%s://",
+
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
+
+ /* if transport type is not unix, specify server:port */
+ if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (strstr(disk->hosts->name, ":")) {
Wouldn't it
be better to check for ':' and check for absence of "." (and
vice versa in the else) so that if someone specified a.b.c:d
or a:b:c:d:e.f we can throw error rite away, instead of qemu erroring
out later in time ? Its a very primtive check but
can help to catch user input error early enuf.
+ /* if IPv6 addr, use square brackets to enclose it
*/
+ virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
disk->hosts->port);
+ } else {
+ virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
disk->hosts->port);
+ }
+ }
+
+ /* append source path to gluster disk image */
+ virBufferAsprintf(opt, "/%s", disk->src);
+
+ /* if transport type is unix, server name is path to unix socket, ignore port
*/
+ if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ virBufferAsprintf(opt, "?socket=%s", disk->hosts->name);
+ }
This can be clubbed as part of else clause to the above if condn (if
(disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that ways
we avoid an un-necessary check of transport here.
It also means that disk->src needs to be pulled inside the if & else
clauses, which I feel should be ok.
2:09 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/04/2012 09:17 PM, Deepak C Shetty wrote:
On 10/04/2012 07:01 PM, Harsh Prateek Bora wrote:
> Qemu accepts gluster protocol as supported storage backend beside others.
> This patch allows users to specify disks on gluster backends like this:
> <disk type='network'
device='disk' > <driver
name='qemu' type='raw'/ >
<source protocol='gluster' name='volume/image' > <host name='example.org'
port='6000' transport='tcp'/ >
</source > <target
dev='vda' bus='virtio'/ >
</disk >
Note: In the <host> element above, transport is an optional attribute.
> Valid transport values are tcp, unix or rdma. If none specified, tcp
> is assumed.
> If transport type is unix, host name specifies path to unix socket.
> Signed-off-by: Harsh Prateek Bora
<harsh(a)linux.vnet.ibm.com > ---
> docs/schemas/domaincommon.rng | 8 ++
> src/conf/domain_conf.c | 28 +++++-
> src/conf/domain_conf.h | 11 +++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_command.c | 204
> ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 251 insertions(+), 2 deletions(-)
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng
> index f47fdad..89d9b9f 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1048,6 +1048,7 @@
> <value>nbd</value >
<value>rbd</value >
<value>sheepdog</value > +
<value>gluster</value >
</choice >
</attribute >
<optional > @@ -1061,6 +1062,13 @@
> <attribute name="port" > <ref
name="unsignedInt"/ >
</attribute > +
<attribute name="transport" > +
<choice > +
<value>tcp</value > +
<value>unix</value > +
<value>rdma</value > +
</choice > +
</attribute 'transport' attribute is
optional, so it should be placed inside
<optional> </optional> ?
Not necessarily, <zeroOrMore> works for it. You may want to test the
patch without specifying 'transport' attribute!
[...]
> +
> +static int
> +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
> +{
> + int ret = 0;
> + virBufferAddLit(opt, "file=");
> + if (disk->nhosts != 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("gluster accepts only one host"));
> + ret = -1;
> + } else {
> + virBufferAsprintf(opt, "gluster+%s://",
> +
> virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
> +
> + /* if transport type is not unix, specify server:port */
> + if (disk->hosts->transport !=
> VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(disk->hosts->name, ":")) {
Wouldn't it be better to check for ':' and check for absence of "."
(and
vice versa in the else) so that if someone specified a.b.c:d
or a:b:c:d:e.f we can throw error rite away, instead of qemu erroring
out later in time ? Its a very primtive check but
can help to catch user input error early enuf.
I chose to check for only ':' to decide if its a IPv6 addr because it
doesnt make sense to be partial towards '.' What if someone specifies a
host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr is as
bad as any other invalid char.
> + /* if IPv6 addr, use square brackets to enclose it */
> + virBufferAsprintf(opt, "[%s]:%s",
disk->hosts->name,
> disk->hosts->port);
> + } else {
> + virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
> disk->hosts->port);
> + }
> + }
> +
> + /* append source path to gluster disk image */
> + virBufferAsprintf(opt, "/%s", disk->src);
> +
> + /* if transport type is unix, server name is path to unix
> socket, ignore port */
> + if (disk->hosts->transport ==
> VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
> + }
This can be clubbed as part of else clause to the above if condn (if
(disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that ways
we avoid an un-necessary check of transport here.
It also means that disk->src needs to be pulled inside the if & else
clauses, which I feel should be ok.
That was the only reason I kept it out of else. Isn't it better to avoid
redundant code than replacing if with an else ?
regards,
Harsh
2:43 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/04/2012 01:09 PM, Harsh Bora wrote:
On 10/04/2012 09:17 PM, Deepak C Shetty wrote:
> On 10/04/2012 07:01 PM, Harsh Prateek Bora wrote:
>> Qemu accepts gluster protocol as supported storage backend beside
>> others.
>> This patch allows users to specify disks on gluster backends like this:
>>
>> <disk type='network' device='disk'>
>> <driver name='qemu' type='raw'/>
>> <source protocol='gluster' name='volume/image'>
>> <host name='example.org' port='6000'
transport='tcp'/>
>> + <attribute
name="transport">
>> + <choice>
>> + <value>tcp</value>
>> + <value>unix</value>
>> + <value>rdma</value>
>> + </choice>
>> + </attribute>
> 'transport' attribute is optional, so it should be placed inside
> <optional> </optional> ?
Not necessarily, <zeroOrMore> works for it. You may want to test the
patch without specifying 'transport' attribute!
Deepak is right - the <attribute name="transport"> block must be
embedded in an <optional> block. <zeroOrMore> only applies to elements,
but as written, your zeroOrMore says that it is okay to omit the overall
<host> element, but that if you provide a <host> element, it MUST have a
transport='...' attribute, and that is not correct. For back-compat
reasons, ALL new attributes must be in an <optional> block since older
clients of the XML did not provide the new attributes.
I chose to check for only ':' to decide if its a IPv6 addr because it
doesnt make sense to be partial towards '.' What if someone specifies a
host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr is as
bad as any other invalid char.
'.' is valid in IPv6 addr. But yes, ':' is mandatory in IPv6, and
forbidden in IPv4, so it makes a good distinguishing test between the
two families.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:56 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/05/2012 01:13 AM, Eric Blake wrote:
> I chose to check for only ':' to decide if its a
IPv6 addr because it
> doesnt make sense to be partial towards '.' What if someone specifies a
> host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr is as
> bad as any other invalid char.
'.' is valid in IPv6 addr. But yes, ':' is mandatory in IPv6, and
forbidden in IPv4, so it makes a good distinguishing test between the
two families.
So, are you suggesting to validate IPv4 only and that too based on the
absence of ':' and presence of '.'? Does that really suffice to validate
an IPv4 since any other special character is also an invalid separator
for IPv4 ?
regards,
Harsh
On 10/04/2012 01:09 PM, Harsh Bora wrote:
> On 10/04/2012 09:17 PM, Deepak C Shetty wrote:
>> On 10/04/2012 07:01 PM, Harsh Prateek Bora wrote:
>>> Qemu accepts gluster protocol as supported storage backend beside
>>> others.
>>> This patch allows users to specify disks on gluster backends like this:
>> >>> <disk
type='network' device='disk'
>>> <driver name='qemu' type='raw'/ >>> <source protocol='gluster'
name='volume/image' >>> <host
name='example.org' port='6000' transport='tcp'/
>>> + <attribute name="transport" >>> + <choice >>> +
<value>tcp</value >>> +
<value>unix</value >>> +
<value>rdma</value >>> +
</choice >>> +
</attribute >> 'transport'
attribute is optional, so it should be placed inside
>> <optional> </optional> ?
> Not necessarily, <zeroOrMore> works for it. You may
want to test the
> patch without specifying 'transport' attribute!
Deepak is right - the <attribute name="transport"> block must be
embedded in an <optional> block. <zeroOrMore> only applies to elements,
but as written, your zeroOrMore says that it is okay to omit the overall
<host> element, but that if you provide a <host> element, it MUST have a
transport='...' attribute, and that is not correct. For back-compat
reasons, ALL new attributes must be in an <optional> block since older
clients of the XML did not provide the new attributes.
Looking at the grammar, it appeared to me like what you said, however,
when I tested the patch, zeroOrMore worked successfully for the
transport attribute as well. However, I am willing to use <optional>
block for consistency.
3:37 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/04/2012 01:56 PM, Harsh Bora wrote:
> Deepak is right - the <attribute
name="transport"> block must be
> embedded in an <optional> block. <zeroOrMore> only applies to elements,
> but as written, your zeroOrMore says that it is okay to omit the overall
> <host> element, but that if you provide a <host> element, it MUST have a
> transport='...' attribute, and that is not correct. For back-compat
> reasons, ALL new attributes must be in an <optional> block since older
> clients of the XML did not provide the new attributes.
Looking at the grammar, it appeared to me like what you said, however,
when I tested the patch, zeroOrMore worked successfully for the
transport attribute as well. However, I am willing to use <optional>
block for consistency.
Hmm, I guess I'd have to read up on the actual grammar of RNG notation
to see if using <zeroOrMore> as an alternate spelling of <optional> for
fixed-name attributes is kosher or just something that our particular
RNG validation engine permits, but it's certainly not typical usage,
when you realize that there will never be more than one of that
attribute. At any rate, we've hashed this topic to death :)
>
>>
>> I chose to check for only ':' to decide if its a IPv6 addr because it
>> doesnt make sense to be partial towards '.' What if someone specifies a
>> host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr is as
>> bad as any other invalid char.
>
> '.' is valid in IPv6 addr. But yes, ':' is mandatory in IPv6, and
> forbidden in IPv4, so it makes a good distinguishing test between the
> two families.
So, are you suggesting to validate IPv4 only and that too based on the
absence of ':' and presence of '.'? Does that really suffice to validate
an IPv4 since any other special character is also an invalid separator
for IPv4 ?
Ultimately, you want to accept both types of IP addresses. I was just
replying to your quote about '.' not being valid in IPv6 as being an
incorrect statement; I don't really know what the best separator
character is that you want to be using in this context, because I
haven't been following the original conversation closely enough. You
should also realize that hostnames cannot contain ':' or ';' (I'm not
sure about ','), so your question about someone setting a hostname to
'12:12;12,12' to confuse the parser is not worth worrying about.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:07 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/05/2012 02:07 AM, Eric Blake wrote:
On 10/04/2012 01:56 PM, Harsh Bora wrote:
>> Deepak is right - the <attribute name="transport"> block must be
>> embedded in an <optional> block. <zeroOrMore> only applies to
elements,
>> but as written, your zeroOrMore says that it is okay to omit the overall
>> <host> element, but that if you provide a <host> element, it MUST
have a
>> transport='...' attribute, and that is not correct. For back-compat
>> reasons, ALL new attributes must be in an <optional> block since older
>> clients of the XML did not provide the new attributes.
>
> Looking at the grammar, it appeared to me like what you said, however,
> when I tested the patch, zeroOrMore worked successfully for the
> transport attribute as well. However, I am willing to use <optional>
> block for consistency.
Hmm, I guess I'd have to read up on the actual grammar of RNG notation
to see if using <zeroOrMore> as an alternate spelling of <optional> for
fixed-name attributes is kosher or just something that our particular
RNG validation engine permits, but it's certainly not typical usage,
when you realize that there will never be more than one of that
attribute. At any rate, we've hashed this topic to death :)
Hm, I think its better to have <optional> even if the xml works
otherwise, since this is how one would expect the grammar to be written
for an optional attribute.
>
>>
>>>
>>> I chose to check for only ':' to decide if its a IPv6 addr because
it
>>> doesnt make sense to be partial towards '.' What if someone specifies
a
>>> host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr is
as
>>> bad as any other invalid char.
>>
>> '.' is valid in IPv6 addr. But yes, ':' is mandatory in IPv6,
and
>> forbidden in IPv4, so it makes a good distinguishing test between the
>> two families.
>
> So, are you suggesting to validate IPv4 only and that too based on the
> absence of ':' and presence of '.'? Does that really suffice to
validate
> an IPv4 since any other special character is also an invalid separator
> for IPv4 ?
Ultimately, you want to accept both types of IP addresses. I was just
replying to your quote about '.' not being valid in IPv6 as being an
incorrect statement; I don't really know what the best separator
character is that you want to be using in this context, because I
haven't been following the original conversation closely enough. You
should also realize that hostnames cannot contain ':' or ';' (I'm
not
sure about ','), so your question about someone setting a hostname to
'12:12;12,12' to confuse the parser is not worth worrying about.
Ok, I understand that IPv6 can have a '.' though ':' is mandatory in
IPv6, and the current code should work for IPv6 addresses with a . as
well. However, I am not sure if we need to validate the user provided
hostname/IPv4/IPv6 addresses if it contains an invalid separator.
Also, the parsing code for IPv4 should work for hostnames as well. By
talking about IP address with diff separators, I just meant to say that
presence of . and absence of : doesnt necessarily mean a valid IPv4
address since presence of other invalid separator would also mean an
invalid IPv4 address.
regards,
Harsh
12:09 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
>
>>
>>>
>>>>
>>>> I chose to check for only ':' to decide if its a IPv6 addr
because it
>>>> doesnt make sense to be partial towards '.' What if someone
>>>> specifies a
>>>> host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr
>>>> is as
>>>> bad as any other invalid char.
>>>
>>> '.' is valid in IPv6 addr. But yes, ':' is mandatory in
IPv6, and
>>> forbidden in IPv4, so it makes a good distinguishing test between the
>>> two families.
>>
>> So, are you suggesting to validate IPv4 only and that too based on the
>> absence of ':' and presence of '.'? Does that really suffice to
>> validate
>> an IPv4 since any other special character is also an invalid separator
>> for IPv4 ?
>
> Ultimately, you want to accept both types of IP addresses. I was just
> replying to your quote about '.' not being valid in IPv6 as being an
> incorrect statement; I don't really know what the best separator
> character is that you want to be using in this context, because I
> haven't been following the original conversation closely enough. You
> should also realize that hostnames cannot contain ':' or ';' (I'm
not
> sure about ','), so your question about someone setting a hostname to
> '12:12;12,12' to confuse the parser is not worth worrying about.
>
Ok, I understand that IPv6 can have a '.' though ':' is mandatory in
IPv6, and the current code should work for IPv6 addresses with a . as
well. However, I am not sure if we need to validate the user provided
hostname/IPv4/IPv6 addresses if it contains an invalid separator.
Also, the parsing code for IPv4 should work for hostnames as well. By
talking about IP address with diff separators, I just meant to say
that presence of . and absence of : doesnt necessarily mean a valid
IPv4 address since presence of other invalid separator would also mean
an invalid IPv4 address.
I didn't knew '.' is allowed in IPv6, if thats the case, then i take
back my suggestion here :)
12:09 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
>
>> + /* if IPv6 addr, use square brackets to enclose it */
>> + virBufferAsprintf(opt, "[%s]:%s",
disk->hosts->name,
>> disk->hosts->port);
>> + } else {
>> + virBufferAsprintf(opt, "%s:%s",
disk->hosts->name,
>> disk->hosts->port);
>> + }
>> + }
>> +
>> + /* append source path to gluster disk image */
>> + virBufferAsprintf(opt, "/%s", disk->src);
>> +
>> + /* if transport type is unix, server name is path to unix
>> socket, ignore port */
>> + if (disk->hosts->transport ==
>> VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>> + virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
>> + }
> This can be clubbed as part of else clause to the above if condn (if
> (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that ways
> we avoid an un-necessary check of transport here.
> It also means that disk->src needs to be pulled inside the if & else
> clauses, which I feel should be ok.
That was the only reason I kept it out of else. Isn't it better to
avoid redundant code than replacing if with an else ?
I think it can be coded to
look non-redundant too.. you just have to use
the disk->src inside the virBufferAsprintf, like you are using
disk->hosts->name. I saw Dan's comment abt using virURIPtr, so maybe
things change with that.
4:43 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Thu, Oct 04, 2012 at 09:17:08PM +0530, Deepak C Shetty wrote:
>+ virBufferAsprintf(opt, "gluster+%s://",
>+
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
>+
>+ /* if transport type is not unix, specify server:port */
>+ if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>+ if (strstr(disk->hosts->name, ":")) {
Wouldn't it be better to check for ':' and check for absence of
"."
(and vice versa in the else) so that if someone specified a.b.c:d
or a:b:c:d:e.f we can throw error rite away, instead of qemu
erroring out later in time ? Its a very primtive check but
can help to catch user input error early enuf.
>+ /* if IPv6 addr, use square brackets to enclose it */
>+ virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
disk->hosts->port);
>+ } else {
>+ virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
disk->hosts->port);
>+ }
>+ }
>+
>+ /* append source path to gluster disk image */
>+ virBufferAsprintf(opt, "/%s", disk->src);
>+
>+ /* if transport type is unix, server name is path to unix socket, ignore
port */
>+ if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>+ virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
>+ }
This can be clubbed as part of else clause to the above if condn (if
(disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that
ways we avoid an un-necessary check of transport here.
It also means that disk->src needs to be pulled inside the if & else
clauses, which I feel should be ok.
Since this code is building up a URI, it really should be re-written to
use the virURIPtr object, instead of virBufferPtr. This will ensure that
things like [] for IPv6 are handled automatically, as well as doing the
correct escaping of parameter values. This code is quite possibly
exploitable as it stands if the user provided cleverly crafted values
for disks->hosts->name which abused URI syntax
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:08 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/05/2012 03:13 AM, Daniel P. Berrange wrote:
On Thu, Oct 04, 2012 at 09:17:08PM +0530, Deepak C Shetty wrote:
>> + virBufferAsprintf(opt, "gluster+%s://",
>> +
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
>> +
>> + /* if transport type is not unix, specify server:port */
>> + if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>> + if (strstr(disk->hosts->name, ":")) {
> Wouldn't it be better to check for ':' and check for absence of
"."
> (and vice versa in the else) so that if someone specified a.b.c:d
> or a:b:c:d:e.f we can throw error rite away, instead of qemu
> erroring out later in time ? Its a very primtive check but
> can help to catch user input error early enuf.
>
>> + /* if IPv6 addr, use square brackets to enclose it */
>> + virBufferAsprintf(opt, "[%s]:%s",
disk->hosts->name, disk->hosts->port);
>> + } else {
>> + virBufferAsprintf(opt, "%s:%s",
disk->hosts->name, disk->hosts->port);
>> + }
>> + }
>> +
>> + /* append source path to gluster disk image */
>> + virBufferAsprintf(opt, "/%s", disk->src);
>> +
>> + /* if transport type is unix, server name is path to unix socket, ignore
port */
>> + if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>> + virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
>> + }
> This can be clubbed as part of else clause to the above if condn (if
> (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that
> ways we avoid an un-necessary check of transport here.
> It also means that disk->src needs to be pulled inside the if & else
> clauses, which I feel should be ok.
Since this code is building up a URI, it really should be re-written to
use the virURIPtr object, instead of virBufferPtr. This will ensure that
things like [] for IPv6 are handled automatically, as well as doing the
correct escaping of parameter values. This code is quite possibly
exploitable as it stands if the user provided cleverly crafted values
for disks->hosts->name which abused URI syntax
Hm, I was thinking of moving to URI infra as a separate patch later,
however looks like that is the way to go. I will look into that.
regards,
Harsh
Daniel
6:35 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/05/2012 03:13 AM, Daniel P. Berrange wrote:
On Thu, Oct 04, 2012 at 09:17:08PM +0530, Deepak C Shetty wrote:
>> + virBufferAsprintf(opt, "gluster+%s://",
>> +
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
>> +
>> + /* if transport type is not unix, specify server:port */
>> + if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>> + if (strstr(disk->hosts->name, ":")) {
> Wouldn't it be better to check for ':' and check for absence of
"."
> (and vice versa in the else) so that if someone specified a.b.c:d
> or a:b:c:d:e.f we can throw error rite away, instead of qemu
> erroring out later in time ? Its a very primtive check but
> can help to catch user input error early enuf.
>
>> + /* if IPv6 addr, use square brackets to enclose it */
>> + virBufferAsprintf(opt, "[%s]:%s",
disk->hosts->name, disk->hosts->port);
>> + } else {
>> + virBufferAsprintf(opt, "%s:%s",
disk->hosts->name, disk->hosts->port);
>> + }
>> + }
>> +
>> + /* append source path to gluster disk image */
>> + virBufferAsprintf(opt, "/%s", disk->src);
>> +
>> + /* if transport type is unix, server name is path to unix socket, ignore
port */
>> + if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>> + virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
>> + }
> This can be clubbed as part of else clause to the above if condn (if
> (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that
> ways we avoid an un-necessary check of transport here.
> It also means that disk->src needs to be pulled inside the if & else
> clauses, which I feel should be ok.
Since this code is building up a URI, it really should be re-written to
use the virURIPtr object, instead of virBufferPtr. This will ensure that
things like [] for IPv6 are handled automatically, as well as doing the
correct escaping of parameter values. This code is quite possibly
exploitable as it stands if the user provided cleverly crafted values
for disks->hosts->name which abused URI syntax
Hi Daniel,
I had a look at src/util/viruri.c and found that we have virURIParse()
which takes a const char* and returns a virURIPtr object, likewise we
have virURIFormat() which take a virURIPtr and returns a char*.
Are you suggesting to manually populate a virURI struct and pass its
address to virURIFormat() to build the cmdline from libvirt XML or are
you suggesting to first build a cmdline (virBuffer) from the XML and
pass it to virURIParse to validate if the libvirt XML has been provided
a valid URI ?
Also, IIUC, the usage of virURIPtr is going to be limited inside this
function only, since the calling code expects the passed virBuffer to be
populated with the required command line, right?
regards,
Harsh
Daniel
10:27 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Mon, Oct 22, 2012 at 05:05:19PM +0530, Harsh Bora wrote:
On 10/05/2012 03:13 AM, Daniel P. Berrange wrote:
>On Thu, Oct 04, 2012 at 09:17:08PM +0530, Deepak C Shetty wrote:
>>>+ virBufferAsprintf(opt, "gluster+%s://",
>>>+
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
>>>+
>>>+ /* if transport type is not unix, specify server:port */
>>>+ if (disk->hosts->transport !=
VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>>>+ if (strstr(disk->hosts->name, ":")) {
>>Wouldn't it be better to check for ':' and check for absence of
"."
>>(and vice versa in the else) so that if someone specified a.b.c:d
>>or a:b:c:d:e.f we can throw error rite away, instead of qemu
>>erroring out later in time ? Its a very primtive check but
>>can help to catch user input error early enuf.
>>
>>>+ /* if IPv6 addr, use square brackets to enclose it */
>>>+ virBufferAsprintf(opt, "[%s]:%s",
disk->hosts->name, disk->hosts->port);
>>>+ } else {
>>>+ virBufferAsprintf(opt, "%s:%s",
disk->hosts->name, disk->hosts->port);
>>>+ }
>>>+ }
>>>+
>>>+ /* append source path to gluster disk image */
>>>+ virBufferAsprintf(opt, "/%s", disk->src);
>>>+
>>>+ /* if transport type is unix, server name is path to unix socket,
ignore port */
>>>+ if (disk->hosts->transport ==
VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
>>>+ virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
>>>+ }
>>This can be clubbed as part of else clause to the above if condn (if
>>(disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that
>>ways we avoid an un-necessary check of transport here.
>>It also means that disk->src needs to be pulled inside the if & else
>>clauses, which I feel should be ok.
>
>Since this code is building up a URI, it really should be re-written to
>use the virURIPtr object, instead of virBufferPtr. This will ensure that
>things like [] for IPv6 are handled automatically, as well as doing the
>correct escaping of parameter values. This code is quite possibly
>exploitable as it stands if the user provided cleverly crafted values
>for disks->hosts->name which abused URI syntax
Hi Daniel,
I had a look at src/util/viruri.c and found that we have
virURIParse() which takes a const char* and returns a virURIPtr
object, likewise we have virURIFormat() which take a virURIPtr and
returns a char*.
Are you suggesting to manually populate a virURI struct and pass its
address to virURIFormat() to build the cmdline from libvirt XML or
are you suggesting to first build a cmdline (virBuffer) from the XML
and pass it to virURIParse to validate if the libvirt XML has been
provided a valid URI ?
I mean to populate a virURIPtr object and then call virURIFormat
to turn it into a string, thus guaranteeing that escaping is
properly done.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:57 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Il 04/10/2012 15:31, Harsh Prateek Bora ha scritto:
Qemu accepts gluster protocol as supported storage backend beside
others.
This patch allows users to specify disks on gluster backends like this:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='gluster' name='volume/image'>
<host name='example.org' port='6000'
transport='tcp'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>
Note: In the <host> element above, transport is an optional attribute.
Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
If transport type is unix, host name specifies path to unix socket.
I would rather add a new attribute "socket" than overload the host name.
The host name for Unix sockets is really localhost.
Paolo
Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
---
docs/schemas/domaincommon.rng | 8 ++
src/conf/domain_conf.c | 28 +++++-
src/conf/domain_conf.h | 11 +++
src/libvirt_private.syms | 2 +
src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 251 insertions(+), 2 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f47fdad..89d9b9f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1048,6 +1048,7 @@
<value>nbd</value>
<value>rbd</value>
<value>sheepdog</value>
+ <value>gluster</value>
</choice>
</attribute>
<optional>
@@ -1061,6 +1062,13 @@
<attribute name="port">
<ref name="unsignedInt"/>
</attribute>
+ <attribute name="transport">
+ <choice>
+ <value>tcp</value>
+ <value>unix</value>
+ <value>rdma</value>
+ </choice>
+ </attribute>
</element>
</zeroOrMore>
<empty/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33e1e7f..838f079 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -214,7 +214,13 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST,
"nbd",
"rbd",
- "sheepdog")
+ "sheepdog",
+ "gluster")
+
+VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
+ "tcp",
+ "unix",
+ "rdma")
VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST,
"none",
@@ -3460,6 +3466,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
char *source = NULL;
char *target = NULL;
char *protocol = NULL;
+ char *protocol_transport;
char *trans = NULL;
virDomainDiskHostDefPtr hosts = NULL;
int nhosts = 0;
@@ -3566,6 +3573,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
}
hosts[nhosts].name = NULL;
hosts[nhosts].port = NULL;
+ hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
nhosts++;
hosts[nhosts - 1].name = virXMLPropString(child,
"name");
@@ -3580,6 +3588,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
"%s", _("missing port for
host"));
goto error;
}
+ /* transport can be tcp (default), unix or rdma. */
+ protocol_transport = virXMLPropString(child,
"transport");
+ if (protocol_transport != NULL) {
+ hosts[nhosts - 1].transport =
virDomainDiskProtocolTransportTypeFromString(protocol_transport);
+ if (hosts[nhosts - 1].transport < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown protocol transport
type '%s'"),
+ protocol_transport);
+ goto error;
+ }
+ }
}
child = child->next;
}
@@ -11756,8 +11775,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
for (i = 0; i < def->nhosts; i++) {
virBufferEscapeString(buf, " <host
name='%s'",
def->hosts[i].name);
- virBufferEscapeString(buf, " port='%s'/>\n",
+ virBufferEscapeString(buf, " port='%s'",
def->hosts[i].port);
+ if (def->hosts[i].transport) {
+ virBufferAsprintf(buf, " transport='%s'",
+
virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport));
+ }
+ virBufferAddLit(buf, "/>\n");
}
virBufferAddLit(buf, " </source>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14dead3..7ba7e9b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -456,10 +456,19 @@ enum virDomainDiskProtocol {
VIR_DOMAIN_DISK_PROTOCOL_NBD,
VIR_DOMAIN_DISK_PROTOCOL_RBD,
VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
+ VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
VIR_DOMAIN_DISK_PROTOCOL_LAST
};
+enum virDomainDiskProtocolTransport {
+ VIR_DOMAIN_DISK_PROTO_TRANS_TCP,
+ VIR_DOMAIN_DISK_PROTO_TRANS_UNIX,
+ VIR_DOMAIN_DISK_PROTO_TRANS_RDMA,
+
+ VIR_DOMAIN_DISK_PROTO_TRANS_LAST
+};
+
enum virDomainDiskTray {
VIR_DOMAIN_DISK_TRAY_CLOSED,
VIR_DOMAIN_DISK_TRAY_OPEN,
@@ -481,6 +490,7 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr;
struct _virDomainDiskHostDef {
char *name;
char *port;
+ int transport; /* enum virDomainDiskProtocolTransport */
};
enum virDomainDiskIo {
@@ -2166,6 +2176,7 @@ VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
VIR_ENUM_DECL(virDomainDiskErrorPolicy)
VIR_ENUM_DECL(virDomainDiskProtocol)
+VIR_ENUM_DECL(virDomainDiskProtocolTransport)
VIR_ENUM_DECL(virDomainDiskIo)
VIR_ENUM_DECL(virDomainDiskSecretType)
VIR_ENUM_DECL(virDomainDiskTray)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dab607a..8e70837 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -350,6 +350,8 @@ virDomainDiskInsertPreAlloced;
virDomainDiskIoTypeFromString;
virDomainDiskIoTypeToString;
virDomainDiskPathByName;
+virDomainDiskProtocolTransportTypeFromString;
+virDomainDiskProtocolTransportTypeToString;
virDomainDiskRemove;
virDomainDiskRemoveByName;
virDomainDiskTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 20730a9..0bed2bc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2021,6 +2021,168 @@ no_memory:
return -1;
}
+static int qemuParseGlusterString(virDomainDiskDefPtr def)
+{
+ char *host = NULL, *port = NULL, *volimg, *transp = NULL, *marker;
+
+ if (STRPREFIX(def->src, "+")) {
+ transp = def->src;
+ transp++;
+ marker = strstr(def->src, "://");
+ *marker++ = '\0';
+ marker += 2;
+ } else {
+ /* transport type not specified, use tcp as default */
+ def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
+ marker = strstr(def->src, "://");
+ marker += 3;
+ }
+
+ if (transp) {
+ def->hosts->transport =
virDomainDiskProtocolTransportTypeFromString(transp);
+ if (def->hosts->transport < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid gluster transport type '%s'"),
transp);
+ return -1;
+ }
+ }
+
+ /* now marker points to string which can start with one of the following:
+ * IPv6 address in square brackets followed by port (optional)
+ * <hostname> or <IPv4 address> followed by port (optional)
+ * '/' if its a unix socket followed by path to gluster disk volume/image
+ */
+
+ /* parse server, port */
+ if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (STRPREFIX(marker, "[")) {
+ /* IPv6 addr */
+ host = ++marker;
+ marker = strchr(host, ']');
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse IPv6 addr for gluster host %s
"), host);
+ return -1;
+ }
+ *marker++ = '\0';
+ /* parse port if specified */
+ if (STRPREFIX(marker, ":")) {
+ port = ++marker;
+ marker = strchr(port, '/');
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse filename for gluster disk %s
"), port);
+ return -1;
+ }
+ *marker++ = '\0';
+ } else {
+ marker++; /* port not specified, skip slash */
+ }
+ } else {
+ /* IPv4 address / hostname followed by port (optional) */
+ host = marker;
+ marker = strchr(host, '/'); /* skip to path to gluster disk vol/img
*/
+ if (!marker) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse filename for gluster disk %s
"), port);
+ return -1;
+ }
+ *marker++ = '\0';
+ port = strchr(host, ':');
+ if (port) {
+ /* port was specified with host, separate both */
+ *port++ = '\0';
+ }
+ }
+
+ /* host points to hostname / IPv4 / IPv6 addr
+ * port points to port or NULL is port was not specified
+ */
+ } else {
+ /* transport type is unix, expect one more slash
+ * followed by path to gluster disk vol/img */
+ if (STRPREFIX(marker, "/")) {
+ marker++;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Gluster unix transport url starts with 3 slashes i.e.
gluster+unix:///"));
+ return -1;
+ }
+ }
+
+ /* marker now points to path to gluster disk vol/img */
+ volimg = marker;
+
+ /* if transport type = unix, path to gluster disk vol/img
+ * is followed by ?socket=<path/to/socket> */
+ if (def->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (strstr(marker, "?socket=")) {
+ /* In libvirt xml, socket path is to be provided as
+ * <host name='/path/to/socket' port='0'>
+ */
+ host = strchr(marker, '=');
+ *host++ = '\0';
+ }
+ }
+
+ if (VIR_ALLOC(def->hosts) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+ def->nhosts = 1;
+ def->hosts->name = host;
+ if (port) {
+ def->hosts->port = port;
+ } else {
+ def->hosts->port = strdup("0");
+ }
+ if (!def->hosts->port) {
+ virReportOOMError();
+ return -1;
+ }
+ def->src = strdup(volimg);
+ if (!def->src) {
+ virReportOOMError();
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
+{
+ int ret = 0;
+ virBufferAddLit(opt, "file=");
+ if (disk->nhosts != 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("gluster accepts only one host"));
+ ret = -1;
+ } else {
+ virBufferAsprintf(opt, "gluster+%s://",
+
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
+
+ /* if transport type is not unix, specify server:port */
+ if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ if (strstr(disk->hosts->name, ":")) {
+ /* if IPv6 addr, use square brackets to enclose it */
+ virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
disk->hosts->port);
+ } else {
+ virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
disk->hosts->port);
+ }
+ }
+
+ /* append source path to gluster disk image */
+ virBufferAsprintf(opt, "/%s", disk->src);
+
+ /* if transport type is unix, server name is path to unix socket, ignore port
*/
+ if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
+ virBufferAsprintf(opt, "?socket=%s", disk->hosts->name);
+ }
+ }
+ return ret;
+}
+
char *
qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainDiskDefPtr disk,
@@ -2162,6 +2324,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
goto error;
virBufferAddChar(&opt, ',');
break;
+ case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+ if (qemuBuildGlusterString(disk, &opt) < 0)
+ goto error;
+ virBufferAddChar(&opt, ',');
+ break;
+
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
if (disk->nhosts == 0) {
virBufferEscape(&opt, ',', ",",
"file=sheepdog:%s,",
@@ -5242,6 +5410,18 @@ qemuBuildCommandLine(virConnectPtr conn,
file = virBufferContentAndReset(&opt);
}
break;
+ case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+ {
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ if (qemuBuildGlusterString(disk, &opt) < 0)
+ goto error;
+ if (virBufferError(&opt)) {
+ virReportOOMError();
+ goto error;
+ }
+ file = virBufferContentAndReset(&opt);
+ }
+ break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
if (disk->nhosts == 0) {
if (virAsprintf(&file, "sheepdog:%s,",
disk->src) < 0) {
@@ -6937,6 +7117,21 @@ qemuParseCommandLineDisk(virCapsPtr caps,
goto cleanup;
VIR_FREE(p);
+ } else if (STRPREFIX(def->src, "gluster")) {
+ char *p = def->src;
+
+ def->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
+ def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
+ def->src = strdup(p + strlen("gluster"));
+ if (!def->src) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (qemuParseGlusterString(def) < 0)
+ goto cleanup;
+
+ VIR_FREE(p);
} else if (STRPREFIX(def->src, "sheepdog:")) {
char *p = def->src;
char *port, *vdi;
@@ -8126,6 +8321,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
val += strlen("rbd:");
+ } else if (STRPREFIX(val, "gluster")) {
+ disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
+ disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
+ val += strlen("gluster");
} else if (STRPREFIX(val, "sheepdog:")) {
disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG;
@@ -8211,6 +8410,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
goto no_memory;
}
break;
+ case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+ if (qemuParseGlusterString(disk) < 0)
+ goto error;
+
+ break;
}
}
7:03 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/05/2012 05:27 PM, Paolo Bonzini wrote:
Il 04/10/2012 15:31, Harsh Prateek Bora ha scritto:
> Qemu accepts gluster protocol as supported storage backend beside others.
> This patch allows users to specify disks on gluster backends like this:
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw'/>
> <source protocol='gluster' name='volume/image'>
> <host name='example.org' port='6000'
transport='tcp'/>
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
>
> Note: In the <host> element above, transport is an optional attribute.
> Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
> If transport type is unix, host name specifies path to unix socket.
I would rather add a new attribute "socket" than overload the host name.
The host name for Unix sockets is really localhost.
Yeh, that makes perfect sense.
~ Harsh
Paolo
> Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
> ---
> docs/schemas/domaincommon.rng | 8 ++
> src/conf/domain_conf.c | 28 +++++-
> src/conf/domain_conf.h | 11 +++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 251 insertions(+), 2 deletions(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f47fdad..89d9b9f 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1048,6 +1048,7 @@
> <value>nbd</value>
> <value>rbd</value>
> <value>sheepdog</value>
> + <value>gluster</value>
> </choice>
> </attribute>
> <optional>
> @@ -1061,6 +1062,13 @@
> <attribute name="port">
> <ref name="unsignedInt"/>
> </attribute>
> + <attribute name="transport">
> + <choice>
> + <value>tcp</value>
> + <value>unix</value>
> + <value>rdma</value>
> + </choice>
> + </attribute>
> </element>
> </zeroOrMore>
> <empty/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 33e1e7f..838f079 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -214,7 +214,13 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
> VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST,
> "nbd",
> "rbd",
> - "sheepdog")
> + "sheepdog",
> + "gluster")
> +
> +VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
> + "tcp",
> + "unix",
> + "rdma")
>
> VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST,
> "none",
> @@ -3460,6 +3466,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> char *source = NULL;
> char *target = NULL;
> char *protocol = NULL;
> + char *protocol_transport;
> char *trans = NULL;
> virDomainDiskHostDefPtr hosts = NULL;
> int nhosts = 0;
> @@ -3566,6 +3573,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> }
> hosts[nhosts].name = NULL;
> hosts[nhosts].port = NULL;
> + hosts[nhosts].transport =
VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> nhosts++;
>
> hosts[nhosts - 1].name = virXMLPropString(child,
"name");
> @@ -3580,6 +3588,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> "%s", _("missing port
for host"));
> goto error;
> }
> + /* transport can be tcp (default), unix or rdma. */
> + protocol_transport = virXMLPropString(child,
"transport");
> + if (protocol_transport != NULL) {
> + hosts[nhosts - 1].transport =
virDomainDiskProtocolTransportTypeFromString(protocol_transport);
> + if (hosts[nhosts - 1].transport < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown protocol
transport type '%s'"),
> + protocol_transport);
> + goto error;
> + }
> + }
> }
> child = child->next;
> }
> @@ -11756,8 +11775,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
> for (i = 0; i < def->nhosts; i++) {
> virBufferEscapeString(buf, " <host
name='%s'",
> def->hosts[i].name);
> - virBufferEscapeString(buf, "
port='%s'/>\n",
> + virBufferEscapeString(buf, " port='%s'",
> def->hosts[i].port);
> + if (def->hosts[i].transport) {
> + virBufferAsprintf(buf, " transport='%s'",
> +
virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport));
> + }
> + virBufferAddLit(buf, "/>\n");
> }
> virBufferAddLit(buf, " </source>\n");
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 14dead3..7ba7e9b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -456,10 +456,19 @@ enum virDomainDiskProtocol {
> VIR_DOMAIN_DISK_PROTOCOL_NBD,
> VIR_DOMAIN_DISK_PROTOCOL_RBD,
> VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
> + VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
>
> VIR_DOMAIN_DISK_PROTOCOL_LAST
> };
>
> +enum virDomainDiskProtocolTransport {
> + VIR_DOMAIN_DISK_PROTO_TRANS_TCP,
> + VIR_DOMAIN_DISK_PROTO_TRANS_UNIX,
> + VIR_DOMAIN_DISK_PROTO_TRANS_RDMA,
> +
> + VIR_DOMAIN_DISK_PROTO_TRANS_LAST
> +};
> +
> enum virDomainDiskTray {
> VIR_DOMAIN_DISK_TRAY_CLOSED,
> VIR_DOMAIN_DISK_TRAY_OPEN,
> @@ -481,6 +490,7 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr;
> struct _virDomainDiskHostDef {
> char *name;
> char *port;
> + int transport; /* enum virDomainDiskProtocolTransport */
> };
>
> enum virDomainDiskIo {
> @@ -2166,6 +2176,7 @@ VIR_ENUM_DECL(virDomainDiskBus)
> VIR_ENUM_DECL(virDomainDiskCache)
> VIR_ENUM_DECL(virDomainDiskErrorPolicy)
> VIR_ENUM_DECL(virDomainDiskProtocol)
> +VIR_ENUM_DECL(virDomainDiskProtocolTransport)
> VIR_ENUM_DECL(virDomainDiskIo)
> VIR_ENUM_DECL(virDomainDiskSecretType)
> VIR_ENUM_DECL(virDomainDiskTray)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index dab607a..8e70837 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -350,6 +350,8 @@ virDomainDiskInsertPreAlloced;
> virDomainDiskIoTypeFromString;
> virDomainDiskIoTypeToString;
> virDomainDiskPathByName;
> +virDomainDiskProtocolTransportTypeFromString;
> +virDomainDiskProtocolTransportTypeToString;
> virDomainDiskRemove;
> virDomainDiskRemoveByName;
> virDomainDiskTypeFromString;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 20730a9..0bed2bc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2021,6 +2021,168 @@ no_memory:
> return -1;
> }
>
> +static int qemuParseGlusterString(virDomainDiskDefPtr def)
> +{
> + char *host = NULL, *port = NULL, *volimg, *transp = NULL, *marker;
> +
> + if (STRPREFIX(def->src, "+")) {
> + transp = def->src;
> + transp++;
> + marker = strstr(def->src, "://");
> + *marker++ = '\0';
> + marker += 2;
> + } else {
> + /* transport type not specified, use tcp as default */
> + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> + marker = strstr(def->src, "://");
> + marker += 3;
> + }
> +
> + if (transp) {
> + def->hosts->transport =
virDomainDiskProtocolTransportTypeFromString(transp);
> + if (def->hosts->transport < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid gluster transport type
'%s'"), transp);
> + return -1;
> + }
> + }
> +
> + /* now marker points to string which can start with one of the following:
> + * IPv6 address in square brackets followed by port (optional)
> + * <hostname> or <IPv4 address> followed by port (optional)
> + * '/' if its a unix socket followed by path to gluster disk
volume/image
> + */
> +
> + /* parse server, port */
> + if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (STRPREFIX(marker, "[")) {
> + /* IPv6 addr */
> + host = ++marker;
> + marker = strchr(host, ']');
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse IPv6 addr for gluster host %s
"), host);
> + return -1;
> + }
> + *marker++ = '\0';
> + /* parse port if specified */
> + if (STRPREFIX(marker, ":")) {
> + port = ++marker;
> + marker = strchr(port, '/');
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse filename for gluster disk %s
"), port);
> + return -1;
> + }
> + *marker++ = '\0';
> + } else {
> + marker++; /* port not specified, skip slash */
> + }
> + } else {
> + /* IPv4 address / hostname followed by port (optional) */
> + host = marker;
> + marker = strchr(host, '/'); /* skip to path to gluster disk
vol/img */
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse filename for gluster disk %s
"), port);
> + return -1;
> + }
> + *marker++ = '\0';
> + port = strchr(host, ':');
> + if (port) {
> + /* port was specified with host, separate both */
> + *port++ = '\0';
> + }
> + }
> +
> + /* host points to hostname / IPv4 / IPv6 addr
> + * port points to port or NULL is port was not specified
> + */
> + } else {
> + /* transport type is unix, expect one more slash
> + * followed by path to gluster disk vol/img */
> + if (STRPREFIX(marker, "/")) {
> + marker++;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Gluster unix transport url starts with 3 slashes
i.e. gluster+unix:///"));
> + return -1;
> + }
> + }
> +
> + /* marker now points to path to gluster disk vol/img */
> + volimg = marker;
> +
> + /* if transport type = unix, path to gluster disk vol/img
> + * is followed by ?socket=<path/to/socket> */
> + if (def->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(marker, "?socket=")) {
> + /* In libvirt xml, socket path is to be provided as
> + * <host name='/path/to/socket' port='0'>
> + */
> + host = strchr(marker, '=');
> + *host++ = '\0';
> + }
> + }
> +
> + if (VIR_ALLOC(def->hosts) < 0) {
> + virReportOOMError();
> + return -1;
> + }
> + def->nhosts = 1;
> + def->hosts->name = host;
> + if (port) {
> + def->hosts->port = port;
> + } else {
> + def->hosts->port = strdup("0");
> + }
> + if (!def->hosts->port) {
> + virReportOOMError();
> + return -1;
> + }
> + def->src = strdup(volimg);
> + if (!def->src) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
> +{
> + int ret = 0;
> + virBufferAddLit(opt, "file=");
> + if (disk->nhosts != 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("gluster accepts only one host"));
> + ret = -1;
> + } else {
> + virBufferAsprintf(opt, "gluster+%s://",
> +
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
> +
> + /* if transport type is not unix, specify server:port */
> + if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(disk->hosts->name, ":")) {
> + /* if IPv6 addr, use square brackets to enclose it */
> + virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
disk->hosts->port);
> + } else {
> + virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
disk->hosts->port);
> + }
> + }
> +
> + /* append source path to gluster disk image */
> + virBufferAsprintf(opt, "/%s", disk->src);
> +
> + /* if transport type is unix, server name is path to unix socket, ignore
port */
> + if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
> + }
> + }
> + return ret;
> +}
> +
> char *
> qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> virDomainDiskDefPtr disk,
> @@ -2162,6 +2324,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> goto error;
> virBufferAddChar(&opt, ',');
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + if (qemuBuildGlusterString(disk, &opt) < 0)
> + goto error;
> + virBufferAddChar(&opt, ',');
> + break;
> +
> case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
> if (disk->nhosts == 0) {
> virBufferEscape(&opt, ',', ",",
"file=sheepdog:%s,",
> @@ -5242,6 +5410,18 @@ qemuBuildCommandLine(virConnectPtr conn,
> file = virBufferContentAndReset(&opt);
> }
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + {
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> + if (qemuBuildGlusterString(disk, &opt) < 0)
> + goto error;
> + if (virBufferError(&opt)) {
> + virReportOOMError();
> + goto error;
> + }
> + file = virBufferContentAndReset(&opt);
> + }
> + break;
> case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
> if (disk->nhosts == 0) {
> if (virAsprintf(&file, "sheepdog:%s,",
disk->src) < 0) {
> @@ -6937,6 +7117,21 @@ qemuParseCommandLineDisk(virCapsPtr caps,
> goto cleanup;
>
> VIR_FREE(p);
> + } else if (STRPREFIX(def->src, "gluster")) {
> + char *p = def->src;
> +
> + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
> + def->src = strdup(p + strlen("gluster"));
> + if (!def->src) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (qemuParseGlusterString(def) < 0)
> + goto cleanup;
> +
> + VIR_FREE(p);
> } else if (STRPREFIX(def->src, "sheepdog:")) {
> char *p = def->src;
> char *port, *vdi;
> @@ -8126,6 +8321,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
> val += strlen("rbd:");
> + } else if (STRPREFIX(val, "gluster")) {
> + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
> + val += strlen("gluster");
> } else if (STRPREFIX(val, "sheepdog:")) {
> disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG;
> @@ -8211,6 +8410,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> goto no_memory;
> }
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + if (qemuParseGlusterString(disk) < 0)
> + goto error;
> +
> + break;
> }
> }
>
>
4:36 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/05/2012 05:27 PM, Paolo Bonzini wrote:
Il 04/10/2012 15:31, Harsh Prateek Bora ha scritto:
> Qemu accepts gluster protocol as supported storage backend beside others.
> This patch allows users to specify disks on gluster backends like this:
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw'/>
> <source protocol='gluster' name='volume/image'>
> <host name='example.org' port='6000'
transport='tcp'/>
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
>
> Note: In the <host> element above, transport is an optional attribute.
> Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
> If transport type is unix, host name specifies path to unix socket.
I would rather add a new attribute "socket" than overload the host name.
The host name for Unix sockets is really localhost.
After looking into the URI infra, I realized, it is better to have the
new attribute name as 'query' since socket=</path/to/socket> in
qemu-gluster commandline is nothing but a query in the whole URI
following by a '?' character.
Since, the _virURI struct in libvirt also has a member 'query' for the
query string in a URI, it makes sense to keep the XML attribute as
generic as that so that it can be used by others for related purpose.
Needless to say, users will have to specify sockets as
<host name='example.org' port='6000' transport='unix'
query='socket=/path/to/sock' />
Also, since the libvirt mandates the host element to be provided a name
and port, but the qemu-gluster commandline doesnt allow server-port for
the unix transport, it will have to be handled differently, otherwise
the virURIFormat will add in the server, port info into the URI.
If no objections, I will shared the updated patches soon.
Harsh
Paolo
> Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
> ---
> docs/schemas/domaincommon.rng | 8 ++
> src/conf/domain_conf.c | 28 +++++-
> src/conf/domain_conf.h | 11 +++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 251 insertions(+), 2 deletions(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f47fdad..89d9b9f 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1048,6 +1048,7 @@
> <value>nbd</value>
> <value>rbd</value>
> <value>sheepdog</value>
> + <value>gluster</value>
> </choice>
> </attribute>
> <optional>
> @@ -1061,6 +1062,13 @@
> <attribute name="port">
> <ref name="unsignedInt"/>
> </attribute>
> + <attribute name="transport">
> + <choice>
> + <value>tcp</value>
> + <value>unix</value>
> + <value>rdma</value>
> + </choice>
> + </attribute>
> </element>
> </zeroOrMore>
> <empty/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 33e1e7f..838f079 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -214,7 +214,13 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
> VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST,
> "nbd",
> "rbd",
> - "sheepdog")
> + "sheepdog",
> + "gluster")
> +
> +VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
> + "tcp",
> + "unix",
> + "rdma")
>
> VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST,
> "none",
> @@ -3460,6 +3466,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> char *source = NULL;
> char *target = NULL;
> char *protocol = NULL;
> + char *protocol_transport;
> char *trans = NULL;
> virDomainDiskHostDefPtr hosts = NULL;
> int nhosts = 0;
> @@ -3566,6 +3573,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> }
> hosts[nhosts].name = NULL;
> hosts[nhosts].port = NULL;
> + hosts[nhosts].transport =
VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> nhosts++;
>
> hosts[nhosts - 1].name = virXMLPropString(child,
"name");
> @@ -3580,6 +3588,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> "%s", _("missing port
for host"));
> goto error;
> }
> + /* transport can be tcp (default), unix or rdma. */
> + protocol_transport = virXMLPropString(child,
"transport");
> + if (protocol_transport != NULL) {
> + hosts[nhosts - 1].transport =
virDomainDiskProtocolTransportTypeFromString(protocol_transport);
> + if (hosts[nhosts - 1].transport < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown protocol
transport type '%s'"),
> + protocol_transport);
> + goto error;
> + }
> + }
> }
> child = child->next;
> }
> @@ -11756,8 +11775,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
> for (i = 0; i < def->nhosts; i++) {
> virBufferEscapeString(buf, " <host
name='%s'",
> def->hosts[i].name);
> - virBufferEscapeString(buf, "
port='%s'/>\n",
> + virBufferEscapeString(buf, " port='%s'",
> def->hosts[i].port);
> + if (def->hosts[i].transport) {
> + virBufferAsprintf(buf, " transport='%s'",
> +
virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport));
> + }
> + virBufferAddLit(buf, "/>\n");
> }
> virBufferAddLit(buf, " </source>\n");
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 14dead3..7ba7e9b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -456,10 +456,19 @@ enum virDomainDiskProtocol {
> VIR_DOMAIN_DISK_PROTOCOL_NBD,
> VIR_DOMAIN_DISK_PROTOCOL_RBD,
> VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
> + VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
>
> VIR_DOMAIN_DISK_PROTOCOL_LAST
> };
>
> +enum virDomainDiskProtocolTransport {
> + VIR_DOMAIN_DISK_PROTO_TRANS_TCP,
> + VIR_DOMAIN_DISK_PROTO_TRANS_UNIX,
> + VIR_DOMAIN_DISK_PROTO_TRANS_RDMA,
> +
> + VIR_DOMAIN_DISK_PROTO_TRANS_LAST
> +};
> +
> enum virDomainDiskTray {
> VIR_DOMAIN_DISK_TRAY_CLOSED,
> VIR_DOMAIN_DISK_TRAY_OPEN,
> @@ -481,6 +490,7 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr;
> struct _virDomainDiskHostDef {
> char *name;
> char *port;
> + int transport; /* enum virDomainDiskProtocolTransport */
> };
>
> enum virDomainDiskIo {
> @@ -2166,6 +2176,7 @@ VIR_ENUM_DECL(virDomainDiskBus)
> VIR_ENUM_DECL(virDomainDiskCache)
> VIR_ENUM_DECL(virDomainDiskErrorPolicy)
> VIR_ENUM_DECL(virDomainDiskProtocol)
> +VIR_ENUM_DECL(virDomainDiskProtocolTransport)
> VIR_ENUM_DECL(virDomainDiskIo)
> VIR_ENUM_DECL(virDomainDiskSecretType)
> VIR_ENUM_DECL(virDomainDiskTray)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index dab607a..8e70837 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -350,6 +350,8 @@ virDomainDiskInsertPreAlloced;
> virDomainDiskIoTypeFromString;
> virDomainDiskIoTypeToString;
> virDomainDiskPathByName;
> +virDomainDiskProtocolTransportTypeFromString;
> +virDomainDiskProtocolTransportTypeToString;
> virDomainDiskRemove;
> virDomainDiskRemoveByName;
> virDomainDiskTypeFromString;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 20730a9..0bed2bc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2021,6 +2021,168 @@ no_memory:
> return -1;
> }
>
> +static int qemuParseGlusterString(virDomainDiskDefPtr def)
> +{
> + char *host = NULL, *port = NULL, *volimg, *transp = NULL, *marker;
> +
> + if (STRPREFIX(def->src, "+")) {
> + transp = def->src;
> + transp++;
> + marker = strstr(def->src, "://");
> + *marker++ = '\0';
> + marker += 2;
> + } else {
> + /* transport type not specified, use tcp as default */
> + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> + marker = strstr(def->src, "://");
> + marker += 3;
> + }
> +
> + if (transp) {
> + def->hosts->transport =
virDomainDiskProtocolTransportTypeFromString(transp);
> + if (def->hosts->transport < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid gluster transport type
'%s'"), transp);
> + return -1;
> + }
> + }
> +
> + /* now marker points to string which can start with one of the following:
> + * IPv6 address in square brackets followed by port (optional)
> + * <hostname> or <IPv4 address> followed by port (optional)
> + * '/' if its a unix socket followed by path to gluster disk
volume/image
> + */
> +
> + /* parse server, port */
> + if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (STRPREFIX(marker, "[")) {
> + /* IPv6 addr */
> + host = ++marker;
> + marker = strchr(host, ']');
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse IPv6 addr for gluster host %s
"), host);
> + return -1;
> + }
> + *marker++ = '\0';
> + /* parse port if specified */
> + if (STRPREFIX(marker, ":")) {
> + port = ++marker;
> + marker = strchr(port, '/');
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse filename for gluster disk %s
"), port);
> + return -1;
> + }
> + *marker++ = '\0';
> + } else {
> + marker++; /* port not specified, skip slash */
> + }
> + } else {
> + /* IPv4 address / hostname followed by port (optional) */
> + host = marker;
> + marker = strchr(host, '/'); /* skip to path to gluster disk
vol/img */
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse filename for gluster disk %s
"), port);
> + return -1;
> + }
> + *marker++ = '\0';
> + port = strchr(host, ':');
> + if (port) {
> + /* port was specified with host, separate both */
> + *port++ = '\0';
> + }
> + }
> +
> + /* host points to hostname / IPv4 / IPv6 addr
> + * port points to port or NULL is port was not specified
> + */
> + } else {
> + /* transport type is unix, expect one more slash
> + * followed by path to gluster disk vol/img */
> + if (STRPREFIX(marker, "/")) {
> + marker++;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Gluster unix transport url starts with 3 slashes
i.e. gluster+unix:///"));
> + return -1;
> + }
> + }
> +
> + /* marker now points to path to gluster disk vol/img */
> + volimg = marker;
> +
> + /* if transport type = unix, path to gluster disk vol/img
> + * is followed by ?socket=<path/to/socket> */
> + if (def->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(marker, "?socket=")) {
> + /* In libvirt xml, socket path is to be provided as
> + * <host name='/path/to/socket' port='0'>
> + */
> + host = strchr(marker, '=');
> + *host++ = '\0';
> + }
> + }
> +
> + if (VIR_ALLOC(def->hosts) < 0) {
> + virReportOOMError();
> + return -1;
> + }
> + def->nhosts = 1;
> + def->hosts->name = host;
> + if (port) {
> + def->hosts->port = port;
> + } else {
> + def->hosts->port = strdup("0");
> + }
> + if (!def->hosts->port) {
> + virReportOOMError();
> + return -1;
> + }
> + def->src = strdup(volimg);
> + if (!def->src) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
> +{
> + int ret = 0;
> + virBufferAddLit(opt, "file=");
> + if (disk->nhosts != 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("gluster accepts only one host"));
> + ret = -1;
> + } else {
> + virBufferAsprintf(opt, "gluster+%s://",
> +
virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
> +
> + /* if transport type is not unix, specify server:port */
> + if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(disk->hosts->name, ":")) {
> + /* if IPv6 addr, use square brackets to enclose it */
> + virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
disk->hosts->port);
> + } else {
> + virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
disk->hosts->port);
> + }
> + }
> +
> + /* append source path to gluster disk image */
> + virBufferAsprintf(opt, "/%s", disk->src);
> +
> + /* if transport type is unix, server name is path to unix socket, ignore
port */
> + if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + virBufferAsprintf(opt, "?socket=%s",
disk->hosts->name);
> + }
> + }
> + return ret;
> +}
> +
> char *
> qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> virDomainDiskDefPtr disk,
> @@ -2162,6 +2324,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> goto error;
> virBufferAddChar(&opt, ',');
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + if (qemuBuildGlusterString(disk, &opt) < 0)
> + goto error;
> + virBufferAddChar(&opt, ',');
> + break;
> +
> case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
> if (disk->nhosts == 0) {
> virBufferEscape(&opt, ',', ",",
"file=sheepdog:%s,",
> @@ -5242,6 +5410,18 @@ qemuBuildCommandLine(virConnectPtr conn,
> file = virBufferContentAndReset(&opt);
> }
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + {
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> + if (qemuBuildGlusterString(disk, &opt) < 0)
> + goto error;
> + if (virBufferError(&opt)) {
> + virReportOOMError();
> + goto error;
> + }
> + file = virBufferContentAndReset(&opt);
> + }
> + break;
> case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
> if (disk->nhosts == 0) {
> if (virAsprintf(&file, "sheepdog:%s,",
disk->src) < 0) {
> @@ -6937,6 +7117,21 @@ qemuParseCommandLineDisk(virCapsPtr caps,
> goto cleanup;
>
> VIR_FREE(p);
> + } else if (STRPREFIX(def->src, "gluster")) {
> + char *p = def->src;
> +
> + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
> + def->src = strdup(p + strlen("gluster"));
> + if (!def->src) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (qemuParseGlusterString(def) < 0)
> + goto cleanup;
> +
> + VIR_FREE(p);
> } else if (STRPREFIX(def->src, "sheepdog:")) {
> char *p = def->src;
> char *port, *vdi;
> @@ -8126,6 +8321,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
> val += strlen("rbd:");
> + } else if (STRPREFIX(val, "gluster")) {
> + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER;
> + val += strlen("gluster");
> } else if (STRPREFIX(val, "sheepdog:")) {
> disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK;
> disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG;
> @@ -8211,6 +8410,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
> goto no_memory;
> }
> break;
> + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
> + if (qemuParseGlusterString(disk) < 0)
> + goto error;
> +
> + break;
> }
> }
>
>
5:08 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Il 26/10/2012 11:36, Harsh Bora ha scritto:
On 10/05/2012 05:27 PM, Paolo Bonzini wrote:
> Il 04/10/2012 15:31, Harsh Prateek Bora ha scritto:
>> Qemu accepts gluster protocol as supported storage backend beside
>> others.
>> This patch allows users to specify disks on gluster backends like this:
>>
>> <disk type='network' device='disk'>
>> <driver name='qemu' type='raw'/>
>> <source protocol='gluster' name='volume/image'>
>> <host name='example.org' port='6000'
transport='tcp'/>
>> </source>
>> <target dev='vda' bus='virtio'/>
>> </disk>
>>
>> Note: In the <host> element above, transport is an optional attribute.
>> Valid transport values are tcp, unix or rdma. If none specified, tcp
>> is assumed.
>> If transport type is unix, host name specifies path to unix socket.
>
> I would rather add a new attribute "socket" than overload the host name.
> The host name for Unix sockets is really localhost.
After looking into the URI infra, I realized, it is better to have the
new attribute name as 'query' since socket=</path/to/socket> in
qemu-gluster commandline is nothing but a query in the whole URI
following by a '?' character.
Since, the _virURI struct in libvirt also has a member 'query' for the
query string in a URI, it makes sense to keep the XML attribute as
generic as that so that it can be used by others for related purpose.
No, each query parameter should map to a separate XML attribute or element.
Needless to say, users will have to specify sockets as
<host name='example.org' port='6000' transport='unix'
query='socket=/path/to/sock' />
Also, since the libvirt mandates the host element to be provided a name
and port
Then change the schema. In RNG it would be like this:
<zeroOrMore>
<element name="host">
<choice>
<group>
<optional>
<attribute name="transport">
<choice>
<value>tcp</value>
<value>rdma</value>
</choice>
</attribute>
</optional>
<attribute name="name">
<ref name="dnsName"/>
</attribute>
<attribute name="port">
<ref name="unsignedInt"/>
</attribute>
</group>
<group>
<attribute name="transport">
<value>unix</value>
</attribute>
<attribute name="socket">
<ref name="absFilePath"/>
</attribute>
</group>
</choice>
</element>
</zeroOrMore>
, but the qemu-gluster commandline doesnt allow server-port for
the unix transport, it will have to be handled differently, otherwise
the virURIFormat will add in the server, port info into the URI.
An empty host and port will return gluster+unix:///volname?server=/path
correctly after virURIFormat.
Paolo
8:38 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Fri, Oct 26, 2012 at 15:06:19 +0530, Harsh Bora wrote:
On 10/05/2012 05:27 PM, Paolo Bonzini wrote:
> Il 04/10/2012 15:31, Harsh Prateek Bora ha scritto:
>> Qemu accepts gluster protocol as supported storage backend beside others.
>> This patch allows users to specify disks on gluster backends like this:
>>
>> <disk type='network' device='disk'>
>> <driver name='qemu' type='raw'/>
>> <source protocol='gluster' name='volume/image'>
>> <host name='example.org' port='6000'
transport='tcp'/>
>> </source>
>> <target dev='vda' bus='virtio'/>
>> </disk>
>>
>> Note: In the <host> element above, transport is an optional attribute.
>> Valid transport values are tcp, unix or rdma. If none specified, tcp is
assumed.
>> If transport type is unix, host name specifies path to unix socket.
>
> I would rather add a new attribute "socket" than overload the host name.
> The host name for Unix sockets is really localhost.
After looking into the URI infra, I realized, it is better to have the
new attribute name as 'query' since socket=</path/to/socket> in
qemu-gluster commandline is nothing but a query in the whole URI
following by a '?' character.
Since, the _virURI struct in libvirt also has a member 'query' for the
query string in a URI, it makes sense to keep the XML attribute as
generic as that so that it can be used by others for related purpose.
Needless to say, users will have to specify sockets as
<host name='example.org' port='6000' transport='unix'
query='socket=/path/to/sock' />
Ah, this looks horrible. We really want to avoid specifying random garbage for
hostname and port in case of unix transport. What Paolo is suggesting is to
create
<host name='example.org' port='6000' transport='tcp'/>
or
<host socket='/path/to/sock' transport='unix'/>
However, I don't like this too much either. What if we add a general socket
element? In other words,
<host name='example.org' port='6000'
transport='tcp|rdma'/> or
<socket type='unix' path='/path/to/sock'/>
where the type attribute in socket element would determine what other
attributes can be used (path for unix sockets). Internally, both elements
could be described by a unified socket structure.
Jirka
8:49 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Il 26/10/2012 15:38, Jiri Denemark ha scritto:
<host name='example.org' port='6000'
transport='tcp'/> or
<host socket='/path/to/sock' transport='unix'/>
However, I don't like this too much either. What if we add a general socket
element? In other words,
<host name='example.org' port='6000'
transport='tcp|rdma'/> or
<socket type='unix' path='/path/to/sock'/>
where the type attribute in socket element would determine what other
attributes can be used (path for unix sockets). Internally, both elements
could be described by a unified socket structure.
The reason why I suggested reusing <host> is because you could in
principle have socket as an attribute even for other transports.
libvirt for example has it as a socket parameter for its ssh transport.
What about moving the transport to source:
<source protocol='gluster' name='Volume2/Image'
transport='tcp'>
<host name='example.org' port='6000'/>
</source>
<source protocol='gluster' name='Volume2/Image'
transport='unix'>
<socket path='/path/to/sock'/>
</source>
where hypothetically an ssh transport could have both host and socket
sub-elements.
Paolo
8:55 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Fri, Oct 26, 2012 at 15:49:01 +0200, Paolo Bonzini wrote:
Il 26/10/2012 15:38, Jiri Denemark ha scritto:
> <host name='example.org' port='6000'
transport='tcp'/> or
> <host socket='/path/to/sock' transport='unix'/>
>
> However, I don't like this too much either. What if we add a general socket
> element? In other words,
>
> <host name='example.org' port='6000'
transport='tcp|rdma'/> or
> <socket type='unix' path='/path/to/sock'/>
>
> where the type attribute in socket element would determine what other
> attributes can be used (path for unix sockets). Internally, both elements
> could be described by a unified socket structure.
The reason why I suggested reusing <host> is because you could in
principle have socket as an attribute even for other transports.
libvirt for example has it as a socket parameter for its ssh transport.
What about moving the transport to source:
<source protocol='gluster' name='Volume2/Image'
transport='tcp'>
<host name='example.org' port='6000'/>
</source>
<source protocol='gluster' name='Volume2/Image'
transport='unix'>
<socket path='/path/to/sock'/>
</source>
where hypothetically an ssh transport could have both host and socket
sub-elements.
Hmm, I'm starting to think we are making it too complicated :-) Let's see what
other think.
Jirka
9:01 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Il 26/10/2012 15:55, Jiri Denemark ha scritto:
> > What about moving the transport to source:
> >
> > <source protocol='gluster' name='Volume2/Image'
transport='tcp'>
> > <host name='example.org' port='6000'/>
> > </source>
> >
> > <source protocol='gluster' name='Volume2/Image'
transport='unix'>
> > <socket path='/path/to/sock'/>
> > </source>
> >
> > where hypothetically an ssh transport could have both host and socket
> > sub-elements.
Hmm, I'm starting to think we are making it too complicated :-) Let's see what
other think.
Oh, I'm fine with Harsh's v2. :) Just trying to propose something based
on your observations.
Paolo
9:40 a.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Il 26/10/2012 16:01, Paolo Bonzini ha scritto:
>>> >> > What about moving the transport to source:
>>> >> >
>>> >> > <source protocol='gluster'
name='Volume2/Image' transport='tcp'>
>>> >> > <host name='example.org'
port='6000'/>
>>> >> > </source>
>>> >> >
>>> >> > <source protocol='gluster'
name='Volume2/Image' transport='unix'>
>>> >> > <socket path='/path/to/sock'/>
>>> >> > </source>
>>> >> >
>>> >> > where hypothetically an ssh transport could have both host and
socket
>>> >> > sub-elements.
> > Hmm, I'm starting to think we are making it too complicated :-) Let's
see what
> > other think.
Oh, I'm fine with Harsh's v2. :) Just trying to propose something based
on your observations.
I meant his v3.
Paolo
12:58 p.m.
New subject: [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 10/26/2012 07:08 PM, Jiri Denemark wrote:
On Fri, Oct 26, 2012 at 15:06:19 +0530, Harsh Bora wrote:
> On 10/05/2012 05:27 PM, Paolo Bonzini wrote:
>> Il 04/10/2012 15:31, Harsh Prateek Bora ha scritto:
>>> Qemu accepts gluster protocol as supported storage backend beside others.
>>> This patch allows users to specify disks on gluster backends like this:
>>>
>>> <disk type='network' device='disk'>
>>> <driver name='qemu' type='raw'/>
>>> <source protocol='gluster'
name='volume/image'>
>>> <host name='example.org' port='6000'
transport='tcp'/>
>>> </source>
>>> <target dev='vda' bus='virtio'/>
>>> </disk>
>>>
>>> Note: In the <host> element above, transport is an optional attribute.
>>> Valid transport values are tcp, unix or rdma. If none specified, tcp is
assumed.
>>> If transport type is unix, host name specifies path to unix socket.
>>
>> I would rather add a new attribute "socket" than overload the host
name.
>> The host name for Unix sockets is really localhost.
>
> After looking into the URI infra, I realized, it is better to have the
> new attribute name as 'query' since socket=</path/to/socket> in
> qemu-gluster commandline is nothing but a query in the whole URI
> following by a '?' character.
>
> Since, the _virURI struct in libvirt also has a member 'query' for the
> query string in a URI, it makes sense to keep the XML attribute as
> generic as that so that it can be used by others for related purpose.
>
> Needless to say, users will have to specify sockets as
> <host name='example.org' port='6000' transport='unix'
> query='socket=/path/to/sock' />
Ah, this looks horrible. We really want to avoid specifying random garbage for
hostname and port in case of unix transport. What Paolo is suggesting is to
create
<host name='example.org' port='6000' transport='tcp'/>
or
<host socket='/path/to/sock' transport='unix'/>
However, I don't like this too much either. What if we add a general socket
element? In other words,
<host name='example.org' port='6000'
transport='tcp|rdma'/> or
<socket type='unix' path='/path/to/sock'/>
where the type attribute in socket element would determine what other
attributes can be used (path for unix sockets). Internally, both elements
could be described by a unified socket structure.
IIUC, a separate element for socket makes more sense if there will be
more than one socket utilized per connection (source/target pair), which
I am not really sure about.
Also, if there is a use case in future, we can always introduce a change
in schema as we are doing in this patch.
regards,
Harsh
Jirka
8:31 a.m.
New subject: [libvirt] [PATCH v2 2/2] tests: Add tests for gluster protocol based network disks support
Signed-off-by: Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
---
tests/qemuargv2xmltest.c | 1 +
.../qemuxml2argv-disk-drive-network-gluster.args | 1 +
.../qemuxml2argv-disk-drive-network-gluster.xml | 33 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
4 files changed, 37 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 5e51d32..bcea8fe 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -183,6 +183,7 @@ mymain(void)
DO_TEST("disk-drive-cache-directsync");
DO_TEST("disk-drive-cache-unsafe");
DO_TEST("disk-drive-network-nbd");
+ DO_TEST("disk-drive-network-gluster");
DO_TEST("disk-drive-network-rbd");
/* older format using CEPH_ARGS env var */
DO_TEST("disk-drive-network-rbd-ceph-env");
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args
new file mode 100644
index 0000000..72d4ded
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc
-no-kqemu -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi
-boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive
file=gluster+tcp://example.org:6000/Volume/Image,if=virtio,format=raw -net none -serial
none -parallel none -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml
new file mode 100644
index 0000000..fe56166
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source protocol='gluster' name='Volume/Image'>
+ <host name='example.org' port='6000'
transport='tcp'/>
+ </source>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0757e37..20ed7cb 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -469,6 +469,8 @@ mymain(void)
QEMU_CAPS_DRIVE_CACHE_UNSAFE, QEMU_CAPS_DRIVE_FORMAT);
DO_TEST("disk-drive-network-nbd",
QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
+ DO_TEST("disk-drive-network-gluster", false,
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
DO_TEST("disk-drive-network-rbd",
QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
DO_TEST("disk-drive-network-sheepdog",
--
1.7.11.4
4411
days inactive
4433
days old
26 comments
7 participants
participants (7)
-
Daniel P. Berrange -
Deepak C Shetty -
Eric Blake -
Harsh Bora -
Harsh Prateek Bora
-
Jiri Denemark -
Paolo Bonzini