[libvirt] [PATCH] add network disk support
This patch adds network disk support to libvirt/QEMU. The currently supported protcols are nbd, rbd, and sheepdog. The XML syntax is like this: <disk type="network" device="disk"> <driver name="qemu" type="raw" /> <source protocol='rbd|sheepdog|nbd' name="...some image identifier..."> <host name="mon1.example.org" port="6000"> <host name="mon2.example.org" port="6000"> <host name="mon3.example.org" port="6000"> </source> <target dev="vda" bus="virtio" /> </disk> Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> --- This patch addresses the discussion on https://www.redhat.com/archives/libvir-list/2010-November/msg00759.html Josh mentioned that the monitor hostnames of RBD can be set through the environment variables, but I couldn't find any documentations about it, so the monitors are not set in this patch. I hope someone who is familiar with RBD implements it. I appreciate any feedback. Thanks, Kazutaka docs/schemas/domain.rng | 31 +++++++++ src/conf/domain_conf.c | 68 +++++++++++++++++-- src/conf/domain_conf.h | 20 ++++++ src/qemu/qemu_conf.c | 169 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 279 insertions(+), 9 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index fb44335..81f4004 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -612,6 +612,37 @@ <ref name="diskspec"/> </interleave> </group> + <group> + <attribute name="type"> + <value>network</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="protocol"> + <choice> + <value>nbd</value> + <value>rbd</value> + <value>sheepdog</value> + </choice> + </attribute> + <attribute name="name"/> + <zeroOrMore> + <element name="host"> + <attribute name="name"> + <ref name="genericName"/> + </attribute> + <attribute name="port"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + <empty/> + </element> + </optional> + <ref name="diskspec"/> + </interleave> + </group> <ref name="diskspec"/> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f14cee..b9dbc61 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -113,7 +113,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", - "dir") + "dir", + "network") VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST, "disk", @@ -142,6 +143,11 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, "ignore", "enospace") +VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST, + "nbd", + "rbd", + "sheepdog") + VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", @@ -508,6 +514,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->serial); VIR_FREE(def->src); + VIR_FREE(def->hosts); VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); @@ -1574,13 +1581,15 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, int flags) { virDomainDiskDefPtr def; - xmlNodePtr cur; + xmlNodePtr cur, host; char *type = NULL; char *device = NULL; char *driverName = NULL; char *driverType = NULL; char *source = NULL; char *target = NULL; + char *protocol = NULL; + virDomainDiskHostDefPtr hosts = NULL; char *bus = NULL; char *cachetag = NULL; char *error_policy = NULL; @@ -1607,7 +1616,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if ((source == NULL) && + if ((source == NULL && hosts == NULL) && (xmlStrEqual(cur->name, BAD_CAST "source"))) { switch (def->type) { @@ -1620,6 +1629,30 @@ virDomainDiskDefParseXML(virCapsPtr caps, case VIR_DOMAIN_DISK_TYPE_DIR: source = virXMLPropString(cur, "dir"); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + protocol = virXMLPropString(cur, "protocol"); + if (protocol == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing protocol type")); + break; + } + def->protocol = virDomainDiskProtocolTypeFromString(protocol); + source = virXMLPropString(cur, "name"); + host = cur->children; + while (host != NULL) { + if (host->type == XML_ELEMENT_NODE && + xmlStrEqual(host->name, BAD_CAST "host")) { + if (VIR_REALLOC_N(hosts, def->nhosts + 1) < 0) { + virReportOOMError(); + goto error; + } + hosts[def->nhosts].name = virXMLPropString(host, "name"); + hosts[def->nhosts].port = virXMLPropString(host, "port"); + def->nhosts++; + } + host = host->next; + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), @@ -1685,7 +1718,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present */ - if (source == NULL && + if (source == NULL && hosts == NULL && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virDomainReportError(VIR_ERR_NO_SOURCE, @@ -1791,6 +1824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, source = NULL; def->dst = target; target = NULL; + def->hosts = hosts; + hosts = NULL; def->driverName = driverName; driverName = NULL; def->driverType = driverType; @@ -1819,6 +1854,8 @@ cleanup: VIR_FREE(type); VIR_FREE(target); VIR_FREE(source); + VIR_FREE(hosts); + VIR_FREE(protocol); VIR_FREE(device); VIR_FREE(driverType); VIR_FREE(driverName); @@ -5887,7 +5924,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferVSprintf(buf, "/>\n"); } - if (def->src) { + if (def->src || def->nhosts > 0) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: virBufferEscapeString(buf, " <source file='%s'/>\n", @@ -5901,6 +5938,27 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " <source dir='%s'/>\n", def->src); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + virBufferVSprintf(buf, " <source protocol='%s'", + virDomainDiskProtocolTypeToString(def->protocol)); + if (def->src) { + virBufferEscapeString(buf, " name='%s'", def->src); + } + if (def->nhosts == 0) { + virBufferVSprintf(buf, "/>\n"); + } else { + int i; + + virBufferVSprintf(buf, ">\n"); + for (i = 0; i < def->nhosts; i++) { + virBufferEscapeString(buf, " <host name='%s'", + def->hosts[i].name); + virBufferEscapeString(buf, " port='%s'/>\n", + def->hosts[i].port); + } + virBufferVSprintf(buf, " </source>\n"); + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 899b19f..6c97289 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -120,6 +120,7 @@ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, VIR_DOMAIN_DISK_TYPE_FILE, VIR_DOMAIN_DISK_TYPE_DIR, + VIR_DOMAIN_DISK_TYPE_NETWORK, VIR_DOMAIN_DISK_TYPE_LAST }; @@ -164,6 +165,21 @@ enum virDomainDiskErrorPolicy { VIR_DOMAIN_DISK_ERROR_POLICY_LAST }; +enum virDomainDiskProtocol { + VIR_DOMAIN_DISK_PROTOCOL_NBD, + VIR_DOMAIN_DISK_PROTOCOL_RBD, + VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG, + + VIR_DOMAIN_DISK_PROTOCOL_LAST +}; + +typedef struct _virDomainDiskHostDef virDomainDiskHostDef; +typedef virDomainDiskHostDef *virDomainDiskHostDefPtr; +struct _virDomainDiskHostDef { + char *name; + char *port; +}; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -173,6 +189,9 @@ struct _virDomainDiskDef { int bus; char *src; char *dst; + int protocol; + int nhosts; + virDomainDiskHostDefPtr hosts; char *driverName; char *driverType; char *serial; @@ -1237,6 +1256,7 @@ VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) +VIR_ENUM_DECL(virDomainDiskProtocol) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModel) VIR_ENUM_DECL(virDomainFS) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 35caccc..63abd75 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2714,7 +2714,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; } - if (disk->src) { + if (disk->src || disk->nhosts > 0) { if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ if (disk->driverType && @@ -2733,6 +2733,24 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); else virBufferVSprintf(&opt, "file=fat:%s,", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + virBufferVSprintf(&opt, "file=nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + virBufferVSprintf(&opt, "file=rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts > 0) + virBufferVSprintf(&opt, "file=sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + else + virBufferVSprintf(&opt, "file=sheepdog:%s,", disk->src); + break; + } } else { virBufferVSprintf(&opt, "file=%s,", disk->src); } @@ -4722,6 +4740,24 @@ int qemudBuildCommandLine(virConnectPtr conn, snprintf(file, PATH_MAX, "fat:floppy:%s", disk->src); else snprintf(file, PATH_MAX, "fat:%s", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + snprintf(file, PATH_MAX, "nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + snprintf(file, PATH_MAX, "rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts > 0) + snprintf(file, PATH_MAX, "sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + else + snprintf(file, PATH_MAX, "sheepdog:%s,", disk->src); + break; + } } else { snprintf(file, PATH_MAX, "%s", disk->src); } @@ -5794,7 +5830,67 @@ qemuParseCommandLineDisk(virCapsPtr caps, values[i] = NULL; if (STRPREFIX(def->src, "/dev/")) def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(def->src, "nbd:")) { + char *host, *port; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + host = def->src + strlen("nbd:"); + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), def->src); + goto cleanup; + } + *port++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = strdup(host); + def->hosts->port = strdup(port); + + VIR_FREE(def->src); + def->src = NULL; + } else if (STRPREFIX(def->src, "rbd:")) { + char *p = def->src; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("rbd:")); + + VIR_FREE(p); + } else if (STRPREFIX(def->src, "sheepdog:")) { + char *p = def->src; + char *port, *vdi; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("sheepdog:")); + + /* def->src must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(def->src, ':'); + if (port) { + *port++ = '\0'; + vdi = strchr(port, ':'); + if (!vdi) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sheepdog filename '%s'"), p); + goto cleanup; + } + *vdi++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = def->src; + def->hosts->port = strdup(port); + def->src = strdup(vdi); + } + + VIR_FREE(p); + } else def->type = VIR_DOMAIN_DISK_TYPE_FILE; } else { def->type = VIR_DOMAIN_DISK_TYPE_FILE; @@ -6731,7 +6827,19 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (STRPREFIX(val, "/dev/")) disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(val, "nbd:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; + val += strlen("nbd:"); + } else if (STRPREFIX(val, "rbd:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + val += strlen("rbd:"); + } else if (STRPREFIX(val, "sheepdog:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; + val += strlen("sheepdog:"); + } else disk->type = VIR_DOMAIN_DISK_TYPE_FILE; if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; @@ -6751,7 +6859,60 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, disk->dst = strdup(arg + 1); } disk->src = strdup(val); - if (!disk->src || + + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + char *host, *port; + + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + host = disk->src; + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), disk->src); + goto error; + } + *port++ = '\0'; + if (VIR_ALLOC(disk->hosts) < 0) { + virReportOOMError(); + goto error; + } + disk->nhosts = 1; + disk->hosts->name = host; + disk->hosts->port = strdup(port); + disk->src = NULL; + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + /* disk->src must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(disk->src, ':'); + if (port) { + char *vdi; + + *port++ = '\0'; + vdi = strchr(port, ':'); + if (!vdi) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sheepdog filename '%s'"), val); + goto error; + } + *vdi++ = '\0'; + if (VIR_ALLOC(disk->hosts) < 0) { + virReportOOMError(); + goto error; + } + disk->nhosts = 1; + disk->hosts->name = disk->src; + disk->hosts->port = strdup(port); + disk->src = strdup(vdi); + } + break; + } + } + + if (!(disk->src || disk->nhosts > 0) || !disk->dst) { virDomainDiskDefFree(disk); goto no_memory; -- 1.7.1
On Fri, Nov 26, 2010 at 05:49:56AM +0900, MORITA Kazutaka wrote:
This patch adds network disk support to libvirt/QEMU. The currently supported protcols are nbd, rbd, and sheepdog. The XML syntax is like this:
<disk type="network" device="disk"> <driver name="qemu" type="raw" /> <source protocol='rbd|sheepdog|nbd' name="...some image identifier..."> <host name="mon1.example.org" port="6000"> <host name="mon2.example.org" port="6000"> <host name="mon3.example.org" port="6000"> </source> <target dev="vda" bus="virtio" /> </disk>
This design looks good to me.
Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> ---
This patch addresses the discussion on https://www.redhat.com/archives/libvir-list/2010-November/msg00759.html
Josh mentioned that the monitor hostnames of RBD can be set through the environment variables, but I couldn't find any documentations about it, so the monitors are not set in this patch. I hope someone who is familiar with RBD implements it.
@@ -1620,6 +1629,30 @@ virDomainDiskDefParseXML(virCapsPtr caps, case VIR_DOMAIN_DISK_TYPE_DIR: source = virXMLPropString(cur, "dir"); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + protocol = virXMLPropString(cur, "protocol"); + if (protocol == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing protocol type")); + break; + } + def->protocol = virDomainDiskProtocolTypeFromString(protocol);
Should check for def->protocal < 0 & raise an error, which would indicate that 'protocol' was not one of the expected values.
+ source = virXMLPropString(cur, "name"); + host = cur->children; + while (host != NULL) { + if (host->type == XML_ELEMENT_NODE && + xmlStrEqual(host->name, BAD_CAST "host")) { + if (VIR_REALLOC_N(hosts, def->nhosts + 1) < 0) { + virReportOOMError(); + goto error; + } + hosts[def->nhosts].name = virXMLPropString(host, "name"); + hosts[def->nhosts].port = virXMLPropString(host, "port"); + def->nhosts++;
Ought to check for NULLs returned by virXMLPropString here I think.
+ } + host = host->next; + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), @@ -1685,7 +1718,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
/* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present */ - if (source == NULL && + if (source == NULL && hosts == NULL && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virDomainReportError(VIR_ERR_NO_SOURCE, @@ -1791,6 +1824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, source = NULL; def->dst = target; target = NULL; + def->hosts = hosts; + hosts = NULL; def->driverName = driverName; driverName = NULL; def->driverType = driverType; @@ -1819,6 +1854,8 @@ cleanup: VIR_FREE(type); VIR_FREE(target); VIR_FREE(source); + VIR_FREE(hosts);
I think you need to free the fields inside 'hosts' too, otherwise we'd leak memory for the port + host strings in the error path.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 35caccc..63abd75 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2714,7 +2714,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; }
- if (disk->src) { + if (disk->src || disk->nhosts > 0) {
If you check for nhosts > 0 here
if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ if (disk->driverType && @@ -2733,6 +2733,24 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); else virBufferVSprintf(&opt, "file=fat:%s,", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + virBufferVSprintf(&opt, "file=nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + virBufferVSprintf(&opt, "file=rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts > 0) + virBufferVSprintf(&opt, "file=sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + else + virBufferVSprintf(&opt, "file=sheepdog:%s,", disk->src); + break;
This else branch for sheepdog will never execute. So I think it is better to check the nhosts value in each of these conditionals. eg We should report an error if nhosts == 0, or nhosts > 1 for NBD since it only wants 1 host. Likewise for other protocols as nedeed.
@@ -5794,7 +5830,67 @@ qemuParseCommandLineDisk(virCapsPtr caps, values[i] = NULL; if (STRPREFIX(def->src, "/dev/")) def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(def->src, "nbd:")) { + char *host, *port; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + host = def->src + strlen("nbd:"); + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), def->src); + goto cleanup; + } + *port++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = strdup(host); + def->hosts->port = strdup(port);
Should check for NULL in both of these strdup cases.
+ + VIR_FREE(def->src); + def->src = NULL; + } else if (STRPREFIX(def->src, "rbd:")) { + char *p = def->src; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("rbd:"));
And this one, and a few more strdup's later in the patch. Regards, Daniel
This patch adds network disk support to libvirt/QEMU. The currently supported protocols are nbd, rbd, and sheepdog. The XML syntax is like this: <disk type="network" device="disk"> <driver name="qemu" type="raw" /> <source protocol='rbd|sheepdog|nbd' name="...some image identifier..."> <host name="mon1.example.org" port="6000"> <host name="mon2.example.org" port="6000"> <host name="mon3.example.org" port="6000"> </source> <target dev="vda" bus="virtio" /> </disk> Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> --- Hi, Thanks for your comments, Daniel. Here is a fixed version. Changes from v1 to v2 are: - check whether the XML input is valid or not more strictly - fix memory leak in the error path - add NULL check of the return value of strdup() Thanks, Kazutaka docs/schemas/domain.rng | 31 +++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++- src/conf/domain_conf.h | 20 ++++ src/qemu/qemu_conf.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 358 insertions(+), 9 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 08ebefb..3b76c9f 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -612,6 +612,37 @@ <ref name="diskspec"/> </interleave> </group> + <group> + <attribute name="type"> + <value>network</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="protocol"> + <choice> + <value>nbd</value> + <value>rbd</value> + <value>sheepdog</value> + </choice> + </attribute> + <attribute name="name"/> + <zeroOrMore> + <element name="host"> + <attribute name="name"> + <ref name="genericName"/> + </attribute> + <attribute name="port"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + <empty/> + </element> + </optional> + <ref name="diskspec"/> + </interleave> + </group> <ref name="diskspec"/> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9516427..1350e22 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -112,7 +112,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", - "dir") + "dir", + "network") VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST, "disk", @@ -141,6 +142,11 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, "ignore", "enospace") +VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST, + "nbd", + "rbd", + "sheepdog") + VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", @@ -507,6 +513,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->serial); VIR_FREE(def->src); + VIR_FREE(def->hosts); VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); @@ -1573,13 +1580,16 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, int flags) { virDomainDiskDefPtr def; - xmlNodePtr cur; + xmlNodePtr cur, host; char *type = NULL; char *device = NULL; char *driverName = NULL; char *driverType = NULL; char *source = NULL; char *target = NULL; + char *protocol = NULL; + virDomainDiskHostDefPtr hosts = NULL; + int nhosts = 0; char *bus = NULL; char *cachetag = NULL; char *error_policy = NULL; @@ -1606,7 +1616,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if ((source == NULL) && + if ((source == NULL && hosts == NULL) && (xmlStrEqual(cur->name, BAD_CAST "source"))) { switch (def->type) { @@ -1619,6 +1629,49 @@ virDomainDiskDefParseXML(virCapsPtr caps, case VIR_DOMAIN_DISK_TYPE_DIR: source = virXMLPropString(cur, "dir"); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + protocol = virXMLPropString(cur, "protocol"); + if (protocol == NULL) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing protocol type")); + goto error; + } + def->protocol = virDomainDiskProtocolTypeFromString(protocol); + if (def->protocol < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown protocol type '%s'"), + protocol); + goto error; + } + source = virXMLPropString(cur, "name"); + host = cur->children; + while (host != NULL) { + if (host->type == XML_ELEMENT_NODE && + xmlStrEqual(host->name, BAD_CAST "host")) { + if (VIR_REALLOC_N(hosts, nhosts + 1) < 0) { + virReportOOMError(); + goto error; + } + hosts[nhosts].name = NULL; + hosts[nhosts].port = NULL; + nhosts++; + + hosts[nhosts - 1].name = virXMLPropString(host, "name"); + if (!hosts[nhosts - 1].name) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing name for host")); + goto error; + } + hosts[nhosts - 1].port = virXMLPropString(host, "port"); + if (!hosts[nhosts - 1].port) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing port for host")); + goto error; + } + } + host = host->next; + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), @@ -1684,7 +1737,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present */ - if (source == NULL && + if (source == NULL && hosts == NULL && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virDomainReportError(VIR_ERR_NO_SOURCE, @@ -1790,6 +1843,10 @@ virDomainDiskDefParseXML(virCapsPtr caps, source = NULL; def->dst = target; target = NULL; + def->hosts = hosts; + hosts = NULL; + def->nhosts = nhosts; + nhosts = 0; def->driverName = driverName; driverName = NULL; def->driverType = driverType; @@ -1818,6 +1875,13 @@ cleanup: VIR_FREE(type); VIR_FREE(target); VIR_FREE(source); + while (nhosts > 0) { + VIR_FREE(hosts[nhosts - 1].name); + VIR_FREE(hosts[nhosts - 1].port); + nhosts--; + } + VIR_FREE(hosts); + VIR_FREE(protocol); VIR_FREE(device); VIR_FREE(driverType); VIR_FREE(driverName); @@ -5886,7 +5950,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferVSprintf(buf, "/>\n"); } - if (def->src) { + if (def->src || def->nhosts > 0) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: virBufferEscapeString(buf, " <source file='%s'/>\n", @@ -5900,6 +5964,27 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " <source dir='%s'/>\n", def->src); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + virBufferVSprintf(buf, " <source protocol='%s'", + virDomainDiskProtocolTypeToString(def->protocol)); + if (def->src) { + virBufferEscapeString(buf, " name='%s'", def->src); + } + if (def->nhosts == 0) { + virBufferVSprintf(buf, "/>\n"); + } else { + int i; + + virBufferVSprintf(buf, ">\n"); + for (i = 0; i < def->nhosts; i++) { + virBufferEscapeString(buf, " <host name='%s'", + def->hosts[i].name); + virBufferEscapeString(buf, " port='%s'/>\n", + def->hosts[i].port); + } + virBufferVSprintf(buf, " </source>\n"); + } + break; default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 899b19f..6c97289 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -120,6 +120,7 @@ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, VIR_DOMAIN_DISK_TYPE_FILE, VIR_DOMAIN_DISK_TYPE_DIR, + VIR_DOMAIN_DISK_TYPE_NETWORK, VIR_DOMAIN_DISK_TYPE_LAST }; @@ -164,6 +165,21 @@ enum virDomainDiskErrorPolicy { VIR_DOMAIN_DISK_ERROR_POLICY_LAST }; +enum virDomainDiskProtocol { + VIR_DOMAIN_DISK_PROTOCOL_NBD, + VIR_DOMAIN_DISK_PROTOCOL_RBD, + VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG, + + VIR_DOMAIN_DISK_PROTOCOL_LAST +}; + +typedef struct _virDomainDiskHostDef virDomainDiskHostDef; +typedef virDomainDiskHostDef *virDomainDiskHostDefPtr; +struct _virDomainDiskHostDef { + char *name; + char *port; +}; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -173,6 +189,9 @@ struct _virDomainDiskDef { int bus; char *src; char *dst; + int protocol; + int nhosts; + virDomainDiskHostDefPtr hosts; char *driverName; char *driverType; char *serial; @@ -1237,6 +1256,7 @@ VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) +VIR_ENUM_DECL(virDomainDiskProtocol) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModel) VIR_ENUM_DECL(virDomainFS) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 925585a..1296cef 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2726,7 +2726,9 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; } - if (disk->src) { + /* disk->src is NULL when we use nbd disks */ + if (disk->src || (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && + disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) { if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ if (disk->driverType && @@ -2745,6 +2747,31 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); else virBufferVSprintf(&opt, "file=fat:%s,", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + if (disk->nhosts != 1) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NBD accepts only one host")); + goto error; + } + virBufferVSprintf(&opt, "file=nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + /* TODO: set monitor hostnames */ + virBufferVSprintf(&opt, "file=rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts == 0) + virBufferVSprintf(&opt, "file=sheepdog:%s,", disk->src); + else + /* only one host is supported now */ + virBufferVSprintf(&opt, "file=sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + break; + } } else { virBufferVSprintf(&opt, "file=%s,", disk->src); } @@ -4636,6 +4663,30 @@ qemudBuildCommandLine(virConnectPtr conn, snprintf(file, PATH_MAX, "fat:floppy:%s", disk->src); else snprintf(file, PATH_MAX, "fat:%s", disk->src); + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + if (disk->nhosts != 1) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NBD accepts only one host")); + goto error; + } + snprintf(file, PATH_MAX, "nbd:%s:%s,", + disk->hosts->name, disk->hosts->port); + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + snprintf(file, PATH_MAX, "rbd:%s,", disk->src); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts == 0) + snprintf(file, PATH_MAX, "sheepdog:%s,", disk->src); + else + /* only one host is supported now */ + snprintf(file, PATH_MAX, "sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src); + break; + } } else { snprintf(file, PATH_MAX, "%s", disk->src); } @@ -5649,7 +5700,91 @@ qemuParseCommandLineDisk(virCapsPtr caps, values[i] = NULL; if (STRPREFIX(def->src, "/dev/")) def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(def->src, "nbd:")) { + char *host, *port; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + host = def->src + strlen("nbd:"); + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), def->src); + goto cleanup; + } + *port++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = strdup(host); + if (!def->hosts->name) { + virReportOOMError(); + goto cleanup; + } + def->hosts->port = strdup(port); + if (!def->hosts->port) { + virReportOOMError(); + goto cleanup; + } + + VIR_FREE(def->src); + def->src = NULL; + } else if (STRPREFIX(def->src, "rbd:")) { + char *p = def->src; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("rbd:")); + if (!def->src) { + virReportOOMError(); + goto cleanup; + } + + VIR_FREE(p); + } else if (STRPREFIX(def->src, "sheepdog:")) { + char *p = def->src; + char *port, *vdi; + + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->src = strdup(p + strlen("sheepdog:")); + if (!def->src) { + virReportOOMError(); + goto cleanup; + } + + /* def->src must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(def->src, ':'); + if (port) { + *port++ = '\0'; + vdi = strchr(port, ':'); + if (!vdi) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sheepdog filename '%s'"), p); + goto cleanup; + } + *vdi++ = '\0'; + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto cleanup; + } + def->nhosts = 1; + def->hosts->name = def->src; + def->hosts->port = strdup(port); + if (!def->hosts->port) { + virReportOOMError(); + goto cleanup; + } + def->src = strdup(vdi); + if (!def->src) { + virReportOOMError(); + goto cleanup; + } + } + + VIR_FREE(p); + } else def->type = VIR_DOMAIN_DISK_TYPE_FILE; } else { def->type = VIR_DOMAIN_DISK_TYPE_FILE; @@ -6586,7 +6721,19 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (STRPREFIX(val, "/dev/")) disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else + else if (STRPREFIX(val, "nbd:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; + val += strlen("nbd:"); + } else if (STRPREFIX(val, "rbd:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + val += strlen("rbd:"); + } else if (STRPREFIX(val, "sheepdog:")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; + val += strlen("sheepdog:"); + } else disk->type = VIR_DOMAIN_DISK_TYPE_FILE; if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; @@ -6606,7 +6753,73 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, disk->dst = strdup(arg + 1); } disk->src = strdup(val); - if (!disk->src || + + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + char *host, *port; + + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + host = disk->src; + port = strchr(host, ':'); + if (!port) { + def = NULL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), disk->src); + goto error; + } + *port++ = '\0'; + if (VIR_ALLOC(disk->hosts) < 0) { + virReportOOMError(); + goto error; + } + disk->nhosts = 1; + disk->hosts->name = host; + disk->hosts->port = strdup(port); + if (!disk->hosts->port) { + virReportOOMError(); + goto error; + } + disk->src = NULL; + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + /* TODO: set monitor hostnames */ + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + /* disk->src must be [vdiname] or [host]:[port]:[vdiname] */ + port = strchr(disk->src, ':'); + if (port) { + char *vdi; + + *port++ = '\0'; + vdi = strchr(port, ':'); + if (!vdi) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sheepdog filename '%s'"), val); + goto error; + } + *vdi++ = '\0'; + if (VIR_ALLOC(disk->hosts) < 0) { + virReportOOMError(); + goto error; + } + disk->nhosts = 1; + disk->hosts->name = disk->src; + disk->hosts->port = strdup(port); + if (!disk->hosts->port) { + virReportOOMError(); + goto error; + } + disk->src = strdup(vdi); + if (!disk->src) { + virReportOOMError(); + goto error; + } + } + break; + } + } + + if (!(disk->src || disk->nhosts > 0) || !disk->dst) { virDomainDiskDefFree(disk); goto no_memory; -- 1.7.1
Here are patches on top of Kazutaka's v2 to add RBD support and fix some general network disk problems. There's also a test for each type of network disk. Josh Durgin (2): qemu: Add RBD support and some network disk fixes tests: Add tests for network disks docs/schemas/domain.rng | 11 ++- src/conf/domain_conf.c | 25 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_conf.c | 143 ++++++++++++++++++-- tests/qemuargv2xmltest.c | 3 + .../qemuxml2argv-disk-drive-network-nbd.args | 1 + .../qemuxml2argv-disk-drive-network-nbd.xml | 32 +++++ .../qemuxml2argv-disk-drive-network-rbd.args | 1 + .../qemuxml2argv-disk-drive-network-rbd.xml | 34 +++++ .../qemuxml2argv-disk-drive-network-sheepdog.args | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.xml | 32 +++++ tests/qemuxml2argvtest.c | 6 + 12 files changed, 275 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml
Changes common to all network disks: -Make source name optional in the domain schema, since NBD doesn't use it -Add a hostName type to the domain schema, and use it instead of genericName, which doesn't include . -Don't leak host names or ports -Set the source protocol in qemuParseCommandline Signed-off-by: Josh Durgin <joshd@hq.newdream.net> --- docs/schemas/domain.rng | 11 +++- src/conf/domain_conf.c | 25 +++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_conf.c | 143 ++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 165 insertions(+), 15 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 4463884..51aae14 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -626,11 +626,13 @@ <value>sheepdog</value> </choice> </attribute> - <attribute name="name"/> + <optional> + <attribute name="name"/> + </optional> <zeroOrMore> <element name="host"> <attribute name="name"> - <ref name="genericName"/> + <ref name="hostName"/> </attribute> <attribute name="port"> <ref name="unsignedInt"/> @@ -2024,6 +2026,11 @@ <param name="minInclusive">1</param> </data> </define> + <define name="hostName"> + <data type="string"> + <param name="pattern">[a-zA-Z0-9\.\-]+</param> + </data> + </define> <define name="PortNumber"> <data type="short"> <param name="minInclusive">-1</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5e2422b..6b4320a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -508,21 +508,34 @@ void virDomainInputDefFree(virDomainInputDefPtr def) void virDomainDiskDefFree(virDomainDiskDefPtr def) { + unsigned int i; + if (!def) return; VIR_FREE(def->serial); VIR_FREE(def->src); - VIR_FREE(def->hosts); VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); virStorageEncryptionFree(def->encryption); virDomainDeviceInfoClear(&def->info); + for (i = 0 ; i < def->nhosts ; i++) + virDomainDiskHostDefFree(&def->hosts[i]); + VIR_FREE(def); } +void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->name); + VIR_FREE(def->port); +} + void virDomainControllerDefFree(virDomainControllerDefPtr def) { if (!def) @@ -1643,7 +1656,12 @@ virDomainDiskDefParseXML(virCapsPtr caps, protocol); goto error; } - source = virXMLPropString(cur, "name"); + if (!(source = virXMLPropString(cur, "name")) && + def->protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("missing name for disk source")); + goto error; + } host = cur->children; while (host != NULL) { if (host->type == XML_ELEMENT_NODE && @@ -1876,8 +1894,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); while (nhosts > 0) { - VIR_FREE(hosts[nhosts - 1].name); - VIR_FREE(hosts[nhosts - 1].port); + virDomainDiskHostDefFree(&hosts[nhosts - 1]); nhosts--; } VIR_FREE(hosts); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6c97289..c1e39ba 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1070,6 +1070,7 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); +void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def); void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 55e193f..d1368dc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -4010,6 +4010,8 @@ qemudBuildCommandLine(virConnectPtr conn, int last_good_net = -1; bool hasHwVirt = false; virCommandPtr cmd; + bool has_rbd_hosts = false; + virBuffer rbd_hosts = VIR_BUFFER_INITIALIZER; uname_normalize(&ut); @@ -4550,6 +4552,7 @@ qemudBuildCommandLine(virConnectPtr conn, int bootable = 0; virDomainDiskDefPtr disk = def->disks[i]; int withDeviceArg = 0; + int j; /* Unless we have -device, then USB disks need special handling */ @@ -4599,6 +4602,27 @@ qemudBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, optstr); VIR_FREE(optstr); + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && + disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + for (j = 0 ; j < disk->nhosts ; j++) { + if (!has_rbd_hosts) { + virBufferAddLit(&rbd_hosts, "-m "); + has_rbd_hosts = true; + } else { + virBufferAddLit(&rbd_hosts, ","); + } + virDomainDiskHostDefPtr host = &disk->hosts[j]; + if (host->port) { + virBufferVSprintf(&rbd_hosts, "%s:%s", + host->name, + host->port); + } else { + virBufferVSprintf(&rbd_hosts, "%s", + host->name); + } + } + } + if (withDeviceArg) { if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { virCommandAddArg(cmd, "-global"); @@ -4621,6 +4645,7 @@ qemudBuildCommandLine(virConnectPtr conn, char dev[NAME_MAX]; char file[PATH_MAX]; virDomainDiskDefPtr disk = def->disks[i]; + int j; if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -4684,6 +4709,23 @@ qemudBuildCommandLine(virConnectPtr conn, break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: snprintf(file, PATH_MAX, "rbd:%s,", disk->src); + for (j = 0 ; j < disk->nhosts ; j++) { + if (!has_rbd_hosts) { + virBufferAddLit(&rbd_hosts, "-m "); + has_rbd_hosts = true; + } else { + virBufferAddLit(&rbd_hosts, ","); + } + virDomainDiskHostDefPtr host = &disk->hosts[j]; + if (host->port) { + virBufferVSprintf(&rbd_hosts, "%s:%s", + host->name, + host->port); + } else { + virBufferVSprintf(&rbd_hosts, "%s", + host->name); + } + } break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk->nhosts == 0) @@ -4703,6 +4745,13 @@ qemudBuildCommandLine(virConnectPtr conn, } } + if (virBufferError(&rbd_hosts)) { + virBufferFreeAndReset(&rbd_hosts); + goto no_memory; + } + if (has_rbd_hosts) + virCommandAddEnvPair(cmd, "CEPH_ARGS", virBufferContentAndReset(&rbd_hosts)); + if (qemuCmdFlags & QEMUD_CMD_FLAG_FSDEV) { for (i = 0 ; i < def->nfss ; i++) { char *optstr; @@ -5468,6 +5517,7 @@ static int qemuStringToArgvEnv(const char *args, int envend; int i; const char *curr = args; + const char *start; const char **progenv = NULL; const char **progargv = NULL; @@ -5475,14 +5525,22 @@ static int qemuStringToArgvEnv(const char *args, while (curr && *curr != '\0') { char *arg; const char *next; - if (*curr == '\'') { - curr++; - next = strchr(curr, '\''); - } else if (*curr == '"') { - curr++; - next = strchr(curr, '"'); + + start = curr; + /* accept a space in CEPH_ARGS */ + if (STRPREFIX(curr, "CEPH_ARGS=-m ")) { + start += strlen("CEPH_ARGS=-m "); + } + if (*start == '\'') { + if (start == curr) + curr++; + next = strchr(start + 1, '\''); + } else if (*start == '"') { + if (start == curr) + curr++; + next = strchr(start + 1, '"'); } else { - next = strchr(curr, ' '); + next = strchr(start, ' '); } if (!next) next = strchr(curr, '\n'); @@ -5712,6 +5770,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, char *host, *port; def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; host = def->src + strlen("nbd:"); port = strchr(host, ':'); if (!port) { @@ -5743,6 +5802,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, char *p = def->src; def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; def->src = strdup(p + strlen("rbd:")); if (!def->src) { virReportOOMError(); @@ -5755,6 +5815,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, char *port, *vdi; def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; def->src = strdup(p + strlen("sheepdog:")); if (!def->src) { virReportOOMError(); @@ -5870,7 +5931,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, } if (!def->src && - def->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + def->device == VIR_DOMAIN_DISK_DEVICE_DISK && + def->type != VIR_DOMAIN_DISK_TYPE_NETWORK) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("missing file parameter in drive '%s'"), val); virDomainDiskDefFree(def); @@ -6790,7 +6852,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, disk->src = NULL; break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: - /* TODO: set monitor hostnames */ + /* handled later since the hosts for all disks are in CEPH_ARGS */ break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: /* disk->src must be [vdiname] or [host]:[port]:[vdiname] */ @@ -7117,6 +7179,69 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } #undef WANT_VALUE + if (def->ndisks > 0) { + const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS"); + if (ceph_args) { + char *hosts, *port, *saveptr, *token; + virDomainDiskDefPtr first_rbd_disk = NULL; + for (i = 0 ; i < def->ndisks ; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && + disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + first_rbd_disk = disk; + break; + } + } + + if (!first_rbd_disk) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("CEPH_ARGS was set without an rbd disk")); + goto error; + } + + /* CEPH_ARGS should be: -m host1[:port1][,host2[:port2]]... */ + if (!STRPREFIX(ceph_args, "-m ")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("could not parse CEPH_ARGS '%s'"), ceph_args); + goto error; + } + hosts = strdup(strchr(ceph_args, ' ') + 1); + if (!hosts) + goto no_memory; + first_rbd_disk->nhosts = 0; + token = strtok_r(hosts, ",", &saveptr); + while (token != NULL) { + if (VIR_REALLOC_N(first_rbd_disk->hosts, first_rbd_disk->nhosts + 1) < 0) { + VIR_FREE(hosts); + goto no_memory; + } + port = strchr(token, ':'); + if (port) { + *port++ = '\0'; + port = strdup(port); + if (!port) { + VIR_FREE(hosts); + goto no_memory; + } + } + first_rbd_disk->hosts[first_rbd_disk->nhosts].port = port; + first_rbd_disk->hosts[first_rbd_disk->nhosts].name = strdup(token); + if (!first_rbd_disk->hosts[first_rbd_disk->nhosts].name) { + VIR_FREE(hosts); + goto no_memory; + } + first_rbd_disk->nhosts++; + token = strtok_r(NULL, ",", &saveptr); + } + VIR_FREE(hosts); + + if (first_rbd_disk->nhosts == 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("found no rbd hosts in CEPH_ARGS '%s'"), ceph_args); + goto error; + } + } + } if (!nographics && def->ngraphics == 0) { virDomainGraphicsDefPtr sdl; -- 1.5.6.5
On Tue, Dec 07, 2010 at 11:56:34AM -0800, Josh Durgin wrote:
Changes common to all network disks: -Make source name optional in the domain schema, since NBD doesn't use it -Add a hostName type to the domain schema, and use it instead of genericName, which doesn't include . -Don't leak host names or ports -Set the source protocol in qemuParseCommandline
Signed-off-by: Josh Durgin <joshd@hq.newdream.net> --- docs/schemas/domain.rng | 11 +++- src/conf/domain_conf.c | 25 +++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_conf.c | 143 ++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 165 insertions(+), 15 deletions(-)
ACK Daniel
Signed-off-by: Josh Durgin <joshd@hq.newdream.net> --- tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-disk-drive-network-nbd.args | 1 + .../qemuxml2argv-disk-drive-network-nbd.xml | 32 ++++++++++++++++++ .../qemuxml2argv-disk-drive-network-rbd.args | 1 + .../qemuxml2argv-disk-drive-network-rbd.xml | 34 ++++++++++++++++++++ .../qemuxml2argv-disk-drive-network-sheepdog.args | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.xml | 32 ++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 8 files changed, 110 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index d941b0b..adff05a 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -173,6 +173,9 @@ mymain(int argc, char **argv) DO_TEST("disk-drive-cache-v2-wt"); DO_TEST("disk-drive-cache-v2-wb"); DO_TEST("disk-drive-cache-v2-none"); + DO_TEST("disk-drive-network-nbd"); + DO_TEST("disk-drive-network-rbd"); + DO_TEST("disk-drive-network-sheepdog"); DO_TEST("disk-usb"); DO_TEST("graphics-vnc"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args new file mode 100644 index 0000000..75d74ab --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -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=nbd:example.org:6000,if=virtio,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml new file mode 100644 index 0000000..863165d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>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' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='nbd'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args new file mode 100644 index 0000000..7e19beb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test CEPH_ARGS=-m mon1.example.org:6321,mon2.example.org:6322,mon3.example.org:6322 /usr/bin/qemu -S -M pc -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=rbd:pool/image,if=virtio,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml new file mode 100644 index 0000000..118d890 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>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' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/image'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args new file mode 100644 index 0000000..147378f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -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=sheepdog:example.org:6000:image,if=virtio,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml new file mode 100644 index 0000000..5313d75 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>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' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='sheepdog' name='image'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5fe91f1..bb184fc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -302,6 +302,12 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_DRIVE_FORMAT, false); DO_TEST("disk-drive-cache-v2-none", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_DRIVE_FORMAT, false); + DO_TEST("disk-drive-network-nbd", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DRIVE_FORMAT, false); + DO_TEST("disk-drive-network-rbd", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DRIVE_FORMAT, false); + DO_TEST("disk-drive-network-sheepdog", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DRIVE_FORMAT, false); DO_TEST("disk-usb", 0, false); DO_TEST("disk-usb-device", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false); -- 1.5.6.5
On Tue, Dec 07, 2010 at 11:57:33AM -0800, Josh Durgin wrote:
Signed-off-by: Josh Durgin <joshd@hq.newdream.net> --- tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-disk-drive-network-nbd.args | 1 + .../qemuxml2argv-disk-drive-network-nbd.xml | 32 ++++++++++++++++++ .../qemuxml2argv-disk-drive-network-rbd.args | 1 + .../qemuxml2argv-disk-drive-network-rbd.xml | 34 ++++++++++++++++++++ .../qemuxml2argv-disk-drive-network-sheepdog.args | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.xml | 32 ++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 8 files changed, 110 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml
ACK Daniel
On 12/09/2010 04:10 AM, Daniel P. Berrange wrote:
On Tue, Dec 07, 2010 at 11:57:33AM -0800, Josh Durgin wrote:
Signed-off-by: Josh Durgin <joshd@hq.newdream.net> --- tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-disk-drive-network-nbd.args | 1 + .../qemuxml2argv-disk-drive-network-nbd.xml | 32 ++++++++++++++++++ .../qemuxml2argv-disk-drive-network-rbd.args | 1 + .../qemuxml2argv-disk-drive-network-rbd.xml | 34 ++++++++++++++++++++ .../qemuxml2argv-disk-drive-network-sheepdog.args | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.xml | 32 ++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 8 files changed, 110 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml
ACK
Both patches are now pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Dec 06, 2010 at 04:24:09PM +0900, MORITA Kazutaka wrote:
This patch adds network disk support to libvirt/QEMU. The currently supported protocols are nbd, rbd, and sheepdog. The XML syntax is like this:
<disk type="network" device="disk"> <driver name="qemu" type="raw" /> <source protocol='rbd|sheepdog|nbd' name="...some image identifier..."> <host name="mon1.example.org" port="6000"> <host name="mon2.example.org" port="6000"> <host name="mon3.example.org" port="6000"> </source> <target dev="vda" bus="virtio" /> </disk>
Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> ---
Hi,
Thanks for your comments, Daniel. Here is a fixed version.
Changes from v1 to v2 are: - check whether the XML input is valid or not more strictly - fix memory leak in the error path - add NULL check of the return value of strdup()
Thanks,
Kazutaka
docs/schemas/domain.rng | 31 +++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++- src/conf/domain_conf.h | 20 ++++ src/qemu/qemu_conf.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 358 insertions(+), 9 deletions(-)
ACK Daniel
On 12/09/2010 04:09 AM, Daniel P. Berrange wrote:
On Mon, Dec 06, 2010 at 04:24:09PM +0900, MORITA Kazutaka wrote:
This patch adds network disk support to libvirt/QEMU. The currently supported protocols are nbd, rbd, and sheepdog. The XML syntax is like this:
<disk type="network" device="disk"> <driver name="qemu" type="raw" /> <source protocol='rbd|sheepdog|nbd' name="...some image identifier..."> <host name="mon1.example.org" port="6000"> <host name="mon2.example.org" port="6000"> <host name="mon3.example.org" port="6000"> </source> <target dev="vda" bus="virtio" /> </disk>
docs/schemas/domain.rng | 31 +++++++
Hmm; missing a corresponding update to docs/formatdomain.html.in to document the new XML.
ACK
NACK. After applying this patch, I get 'make check' failures: TEST: vmx2xmltest /bin/sh: line 5: 15285 Segmentation fault (core dumped) abs_top_builddir=`cd '..'; pwd` abs_top_srcdir=`cd '..'; pwd` abs_builddir=`cd '.'; pwd` abs_srcdir=`cd '.'; pwd` CONFIG_HEADER="`cd '..'; pwd`/config.h" PATH="$abs_top_builddir/src:$abs_top_builddir/daemon:$abs_top_builddir/tools:$PATH" SHELL="/bin/sh" LIBVIRT_DRIVER_DIR="/home/remote/eblake/libvirt/src/.libs" LC_ALL=C ${dir}$tst FAIL: vmx2xmltest TEST: xml2vmxtest ...!!!!!!!!!!...............!!!!!!!!!.. 39 FAIL I haven't looked into root cause. Kazutaka, would you mind preparing a v3? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 12/09/2010 12:13 PM, Eric Blake wrote:
On 12/09/2010 04:09 AM, Daniel P. Berrange wrote:
On Mon, Dec 06, 2010 at 04:24:09PM +0900, MORITA Kazutaka wrote:
This patch adds network disk support to libvirt/QEMU. The currently supported protocols are nbd, rbd, and sheepdog. The XML syntax is like this:
ACK
NACK. After applying this patch, I get 'make check' failures:
TEST: vmx2xmltest /bin/sh: line 5: 15285 Segmentation fault (core dumped)
Weird. After 'make clean', I was no longer able to reproduce the failure; I then did a complete fresh checkout to verify on a pristine tree. I must have had some bad cruft lying around in my tree, and wonder what it was about your patch to expose that cruft? At any rate, given that your patch does not fail after all, I will go ahead and push it. However, we still need a follow-up patch:
Hmm; missing a corresponding update to docs/formatdomain.html.in to document the new XML.
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Josh Durgin -
MORITA Kazutaka