[libvirt] [PATCH 00/25] Add support for multi-host gluster drives
This is a updated take based on stuff I had laying around and parts from https://www.redhat.com/archives/libvir-list/2016-July/msg00872.html This addresses the backing store parser, adds and improves bits to the JSON->commandline generator prior to plugging in the gluster support. This series does not yet address block jobs (snapshot/block copy) using multi-host gluster volumes. Peter Krempa (24): tests: qemuxml2xml: Avoid crash when processing an XML that fails to parse tests: Add testing of backing store string parser util: storage: Add parser for qemu's "json" backing pseudo-protocol util: storage: Add support for host device backing specified via JSON util: storage: Add support for URI based backing volumes in qemu's JSON pseudo-protocol util: storage: Add json pseudo protocol support for gluster volumes util: storage: Add json pseudo protocol support for iSCSI volumes util: storage: Add JSON backing volume parser for 'nbd' protocol util: storage: Add JSON backing store parser for 'sheepdog' protocol util: storage: Add 'ssh' network storage protocol util: storage: Add JSON backing volume parser for 'ssh' protocol util: json: Make first argument of virJSONValueObjectForeachKeyValue const util: qemu: Add wrapper for JSON -> commandline conversion util: qemu: Add support for user-passed strings in JSON->commandline util: qemu: Allow nested objects in JSON -> commandline generator util: qemu: Allow for different approaches to format JSON arrays util: qemu: Don't generate any extra commas in virQEMUBuildCommandLineJSON qemu: command: Rename qemuBuildNetworkDriveURI to qemuBuildNetworkDriveStr qemu: command: Split out network disk URI building qemu: command: Extract drive source command line formatter qemu: command: Refactor code extracted to qemuBuildDriveSourceStr storage: gluster: Support multiple hosts in backend functions util: qemu: Add support for numbered array members qemu: command: Add infrastructure for object specified disk sources Prasanna Kumar Kalever (1): qemu: command: Add support for multi-host gluster disks docs/formatdomain.html.in | 2 +- src/libvirt_private.syms | 5 + src/libxl/libxl_conf.c | 1 + src/qemu/qemu_command.c | 428 +++++++++++++++------ src/qemu/qemu_driver.c | 3 + src/qemu/qemu_parse_command.c | 1 + src/storage/storage_backend_gluster.c | 82 ++-- src/util/virjson.c | 2 +- src/util/virjson.h | 2 +- src/util/virqemu.c | 219 +++++++++-- src/util/virqemu.h | 16 + src/util/virstoragefile.c | 375 +++++++++++++++++- src/util/virstoragefile.h | 4 + src/xenconfig/xen_xl.c | 1 + tests/qemucommandutiltest.c | 68 +++- .../qemuxml2argv-disk-drive-network-gluster.args | 9 +- .../qemuxml2argv-disk-drive-network-gluster.xml | 9 + .../qemuxml2xmlout-disk-drive-network-gluster.xml | 10 + tests/qemuxml2xmltest.c | 9 +- tests/virstoragetest.c | 150 ++++++++ 20 files changed, 1169 insertions(+), 227 deletions(-) -- 2.9.0
Failure to parse a XML that was not supposed to fail would result into a crash in the test suite as the vcpu bitmap would not be filled prior to the active XML->XML test. Skip formatting of the vcpu snippet in the fake status XML formatter in such case to avoid the crash. The test would fail anyways. --- tests/qemuxml2xmltest.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 8a184d2..5f04b8b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -112,9 +112,12 @@ testGetStatuXMLPrefixVcpus(virBufferPtr buf, virBufferAddLit(buf, "<vcpus>\n"); virBufferAdjustIndent(buf, 2); - while ((vcpuid = virBitmapNextSetBit(data->activeVcpus, vcpuid)) >= 0) - virBufferAsprintf(buf, "<vcpu id='%zd' pid='%zd'/>\n", - vcpuid, vcpuid + 3803519); + /* Make sure we can format the fake vcpu list. The test will fail regardles. */ + if (data->activeVcpus) { + while ((vcpuid = virBitmapNextSetBit(data->activeVcpus, vcpuid)) >= 0) + virBufferAsprintf(buf, "<vcpu id='%zd' pid='%zd'/>\n", + vcpuid, vcpuid + 3803519); + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</vcpus>\n"); -- 2.9.0
As we already test that the extraction of the backing store string works well additional tests for the backing store string parser can be made simpler. Export virStorageSourceNewFromBackingAbsolute and use it to parse the backing store strings, format them using virDomainDiskSourceFormat and match them against expected XMLs. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 2 +- src/util/virstoragefile.h | 3 ++ tests/virstoragetest.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9396c4e..9bc8d37 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2366,6 +2366,7 @@ virStorageSourceIsBlockLocal; virStorageSourceIsEmpty; virStorageSourceIsLocalStorage; virStorageSourceNewFromBacking; +virStorageSourceNewFromBackingAbsolute; virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 16de603..0fa9681 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2514,7 +2514,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, } -static virStorageSourcePtr +virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path) { virStorageSourcePtr ret; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 78beaf4..1a76fad 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -380,4 +380,7 @@ int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int virStorageFileCheckCompat(const char *compat); + +virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path); + #endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 580065e..6016a3b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -665,6 +665,58 @@ testPathRelative(const void *args) } +struct testBackingParseData { + const char *backing; + const char *expect; +}; + +static int +testBackingParse(const void *args) +{ + const struct testBackingParseData *data = args; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virStorageSourcePtr src = NULL; + char *xml = NULL; + int ret = -1; + + if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) { + if (!data->expect) + ret = 0; + + goto cleanup; + } + + if (src && !data->expect) { + fprintf(stderr, "parsing of backing store string '%s' should " + "have failed\n", data->backing); + goto cleanup; + } + + if (virDomainDiskSourceFormat(&buf, src, 0, 0) < 0 || + !(xml = virBufferContentAndReset(&buf))) { + fprintf(stderr, "failed to format disk source xml\n"); + goto cleanup; + } + + if (!STREQ(xml, data->expect)) { + fprintf(stderr, "\n backing store string '%s'\n" + "expected storage source xml:\n%s\n" + "actual storage source xml:\n%s\n", + data->backing, data->expect, xml); + goto cleanup; + } + + ret = 0; + + cleanup: + virStorageSourceFree(src); + virBufferFreeAndReset(&buf); + VIR_FREE(xml); + + return ret; +} + + static int mymain(void) { @@ -674,6 +726,7 @@ mymain(void) struct testLookupData data2; struct testPathCanonicalizeData data3; struct testPathRelativeBacking data4; + struct testBackingParseData data5; virStorageSourcePtr chain = NULL; virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ @@ -1276,6 +1329,33 @@ mymain(void) TEST_RELATIVE_BACKING(21, backingchain[10], backingchain[11], "../../../../blah/image4"); TEST_RELATIVE_BACKING(22, backingchain[11], backingchain[11], "../blah/image4"); + + virTestCounterReset("Backing store parse "); + +#define TEST_BACKING_PARSE(bck, xml) \ + do { \ + data5.backing = bck; \ + data5.expect = xml; \ + if (virTestRun(virTestCounterNext(), \ + testBackingParse, &data5) < 0) \ + ret = -1; \ + } while (0) + + TEST_BACKING_PARSE("path", "<source file='path'/>\n"); + TEST_BACKING_PARSE("://", NULL); + TEST_BACKING_PARSE("http://example.com/file", + "<source protocol='http' name='file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("rbd:testshare:id=asdf:mon_host=example.com", + "<source protocol='rbd' name='testshare'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("nbd:example.org:6000:exportname=blah", + "<source protocol='nbd' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); + cleanup: /* Final cleanup */ virStorageSourceFree(chain); -- 2.9.0
Add a modular parser that will allow to parse 'json' backing definitions that are supported by qemu. The initial implementation adds support for the 'file' driver. --- src/util/virstoragefile.c | 87 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 8 +++++ 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0fa9681..826e4ba 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -43,6 +43,7 @@ #include "viruri.h" #include "dirname.h" #include "virbuffer.h" +#include "virjson.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -2514,10 +2515,80 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, + virJSONValuePtr json, + int type) +{ + const char *path; + + if (!(path = virJSONValueObjectGetString(json, "file.filename"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'filename' field in JSON backing volume " + "definition")); + return -1; + } + + if (VIR_STRDUP(src->path, path) < 0) + return -1; + + src->type = type; + return 0; +} + + +struct virStorageSourceJSONDriverParser { + const char *drvname; + int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); + int opaque; +}; + +static const struct virStorageSourceJSONDriverParser jsonParsers[] = { + {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, +}; + + +static int +virStorageSourceParseBackingJSON(virStorageSourcePtr src, + const char *json) +{ + virJSONValuePtr root; + const char *drvname; + size_t i; + int ret = -1; + + if (!(root = virJSONValueFromString(json))) + return -1; + + if (!(drvname = virJSONValueObjectGetString(root, "file.driver"))) { + virReportError(VIR_ERR_INVALID_ARG, _("JSON backing volume defintion " + "'%s' lacks driver name"), json); + goto cleanup; + } + + for (i = 0; i < ARRAY_CARDINALITY(jsonParsers); i++) { + if (STREQ(drvname, jsonParsers[i].drvname)) { + ret = jsonParsers[i].func(src, root, jsonParsers[i].opaque); + goto cleanup; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing parser implementation for JSON backing volume " + "driver '%s'"), drvname); + + cleanup: + virJSONValueFree(root); + return ret; +} + + virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path) { + const char *json; virStorageSourcePtr ret; + int rc; if (VIR_ALLOC(ret) < 0) return NULL; @@ -2531,13 +2602,15 @@ virStorageSourceNewFromBackingAbsolute(const char *path) ret->type = VIR_STORAGE_TYPE_NETWORK; /* handle URI formatted backing stores */ - if (strstr(path, "://")) { - if (virStorageSourceParseBackingURI(ret, path) < 0) - goto error; - } else { - if (virStorageSourceParseBackingColon(ret, path) < 0) - goto error; - } + if ((json = STRSKIP(path, "json:"))) + rc = virStorageSourceParseBackingJSON(ret, json); + else if (strstr(path, "://")) + rc = virStorageSourceParseBackingURI(ret, path); + else + rc = virStorageSourceParseBackingColon(ret, path); + + if (rc < 0) + goto error; } return ret; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 6016a3b..46bbfb5 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1355,6 +1355,14 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:", NULL); + TEST_BACKING_PARSE("json:asdgsdfg", NULL); + TEST_BACKING_PARSE("json:{}", NULL); + TEST_BACKING_PARSE("json: { \"file.driver\":\"blah\"}", NULL); + TEST_BACKING_PARSE("json:{\"file.driver\":\"file\"}", NULL); + TEST_BACKING_PARSE("json:{\"file.driver\":\"file\", " + "\"file.filename\":\"/path/to/file\"}", + "<source file='/path/to/file'/>\n"); cleanup: /* Final cleanup */ -- 2.9.0
Why the quotes? Jan On Mon, Jul 25, 2016 at 08:11:48PM +0200, Peter Krempa wrote:
Add a modular parser that will allow to parse 'json' backing definitions that are supported by qemu. The initial implementation adds support for the 'file' driver. --- src/util/virstoragefile.c | 87 +++++++++++++++++++++++++++++++++++++++++++---- tests/virstoragetest.c | 8 +++++ 2 files changed, 88 insertions(+), 7 deletions(-)
JSON pseudo protocol for qemu allows to explicitly specify devices. Add convertor to the internal type. --- src/util/virstoragefile.c | 2 ++ tests/virstoragetest.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 826e4ba..2e47bdb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2545,6 +2545,8 @@ struct virStorageSourceJSONDriverParser { static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, + {"host_device", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, + {"host_cdrom", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 46bbfb5..552b4d0 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1363,6 +1363,12 @@ mymain(void) TEST_BACKING_PARSE("json:{\"file.driver\":\"file\", " "\"file.filename\":\"/path/to/file\"}", "<source file='/path/to/file'/>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"host_device\", " + "\"file.filename\":\"/path/to/dev\"}", + "<source dev='/path/to/dev'/>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"host_cdrom\", " + "\"file.filename\":\"/path/to/cdrom\"}", + "<source dev='/path/to/cdrom'/>\n"); cleanup: /* Final cleanup */ -- 2.9.0
http(s), ftp(s) and tftp use URIs for volume definitions in the JSON pseudo protocol so it's pretty straightforward to add support for them. --- src/util/virstoragefile.c | 43 +++++++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 8 ++++++++ 2 files changed, 51 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2e47bdb..852d7f6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2537,6 +2537,44 @@ virStorageSourceParseBackingJSONPath(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONUriStr(virStorageSourcePtr src, + const char *uri, + int protocol) +{ + if (virStorageSourceParseBackingURI(src, uri) < 0) + return -1; + + if (src->protocol != protocol) { + virReportError(VIR_ERR_INVALID_ARG, + _("expected protocol '%s' but got '%s' in URI JSON volume " + "definition"), + virStorageNetProtocolTypeToString(protocol), + virStorageNetProtocolTypeToString(src->protocol)); + return -1; + } + + return 0; +} + + +static int +virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, + virJSONValuePtr json, + int protocol) +{ + const char *uri; + + if (!(uri = virJSONValueObjectGetString(json, "file.uri"))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing URI in JSON backing volume definition")); + return -1; + } + + return virStorageSourceParseBackingJSONUriStr(src, uri, protocol); +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2547,6 +2585,11 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE}, {"host_device", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, {"host_cdrom", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_BLOCK}, + {"http", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_HTTP}, + {"https", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_HTTPS}, + {"ftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP}, + {"ftps", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, + {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 552b4d0..364e359 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1369,6 +1369,14 @@ mymain(void) TEST_BACKING_PARSE("json:{\"file.driver\":\"host_cdrom\", " "\"file.filename\":\"/path/to/cdrom\"}", "<source dev='/path/to/cdrom'/>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"http\", " + "\"file.uri\":\"http://example.com/file\"}", + "<source protocol='http' name='file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"ftp\", " + "\"file.uri\":\"http://example.com/file\"}", + NULL); cleanup: /* Final cleanup */ -- 2.9.0
Along with the legacy URI based syntax add support for the brand-new fully object based syntax. --- src/util/virstoragefile.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 25 +++++++++++ 2 files changed, 133 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 852d7f6..5fc1fe7 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2575,6 +2575,113 @@ virStorageSourceParseBackingJSONUri(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONGlusterHost(virStorageNetHostDefPtr host, + virJSONValuePtr json) +{ + const char *type = virJSONValueObjectGetString(json, "type"); + const char *hostname = virJSONValueObjectGetString(json, "host"); + const char *port = virJSONValueObjectGetString(json, "port"); + const char *socket = virJSONValueObjectGetString(json, "socket"); + int transport; + + if ((transport = virStorageNetHostTransportTypeFromString(type)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown backing store transport protocol '%s'"), type); + return -1; + } + + host->transport = transport; + + switch ((virStorageNetHostTransport) transport) { + case VIR_STORAGE_NET_HOST_TRANS_TCP: + if (!hostname) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing hostname for tcp backing server in " + "JSON backing definition for gluster volume")); + return -1; + } + + if (VIR_STRDUP(host->name, hostname) < 0 || + VIR_STRDUP(host->port, port) < 0) + return -1; + break; + + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + if (!socket) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing socket path for udp backing server in " + "JSON backing definition for gluster volume")); + return -1; + } + + + if (VIR_STRDUP(host->socket, socket) < 0) + return -1; + break; + + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("backing store protocol '%s' is not yet supported"), + type); + return -1; + } + + return 0; +} + + +static int +virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *uri = virJSONValueObjectGetString(json, "file.filename"); + const char *volume = virJSONValueObjectGetString(json, "file.volume"); + const char *path = virJSONValueObjectGetString(json, "file.path"); + virJSONValuePtr server = virJSONValueObjectGetArray(json, "file.server"); + size_t nservers; + size_t i; + + /* legacy URI based syntax passed via 'filename' option */ + if (uri) + return virStorageSourceParseBackingJSONUriStr(src, uri, + VIR_STORAGE_NET_PROTOCOL_GLUSTER); + + if (!volume || !path || !server) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'volume', 'path' or 'server' attribute in " + "JSON backing definition for gluster volume")); + return -1; + } + + if (VIR_STRDUP(src->volume, volume) < 0 || + virAsprintf(&src->path, "/%s", path) < 0) + return -1; + + nservers = virJSONValueArraySize(server); + + if (nservers < 1) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("at least 1 server is necessary in " + "JSON backing definition for gluster volume")); + } + + if (VIR_ALLOC_N(src->hosts, nservers) < 0) + return -1; + src->nhosts = nservers; + + for (i = 0; i < nservers; i++) { + if (virStorageSourceParseBackingJSONGlusterHost(src->hosts + i, + virJSONValueArrayGet(server, i)) < 0) + return -1; + } + + return 0; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2590,6 +2697,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"ftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP}, {"ftps", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, + {"gluster", virStorageSourceParseBackingJSONGluster, 0}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 364e359..4ebc1c5 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1377,6 +1377,31 @@ mymain(void) TEST_BACKING_PARSE("json:{\"file.driver\":\"ftp\", " "\"file.uri\":\"http://example.com/file\"}", NULL); + TEST_BACKING_PARSE("json:{\"file.driver\":\"gluster\", " + "\"file.filename\":\"gluster://example.com/vol/file\"}", + "<source protocol='gluster' name='vol/file'>\n" + " <host name='example.com'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"gluster\"," + "\"file.volume\":\"testvol\"," + "\"file.path\":\"img.qcow2\"," + "\"file.server\":[ { \"type\":\"tcp\"," + "\"host\":\"example.com\"," + "\"port\":\"1234\"" + "}," + "{ \"type\":\"unix\"," + "\"socket\":\"/path/socket\"" + "}," + "{ \"type\":\"tcp\"," + "\"host\":\"example.com\"" + "}" + "]" + "}", + "<source protocol='none' name='testvol/img.qcow2'>\n" + " <host name='example.com' port='1234'/>\n" + " <host transport='unix' socket='/path/socket'/>\n" + " <host name='example.com'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.9.0
iSCSI is a bit odd in this aspect since it only supports URIs but using the 'filename' property and does not have any alternative syntax. --- src/util/virstoragefile.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 5fc1fe7..9fc260b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2682,6 +2682,26 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *uri; + + /* legacy URI based syntax passed via 'filename' option */ + if ((uri = virJSONValueObjectGetString(json, "file.filename"))) + return virStorageSourceParseBackingJSONUriStr(src, uri, + VIR_STORAGE_NET_PROTOCOL_ISCSI); + + /* iSCSI currently supports only URI syntax passed in as filename */ + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing iSCSI URI in JSON backing volume definition")); + + return -1; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2698,6 +2718,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"ftps", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTPS}, {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, {"gluster", virStorageSourceParseBackingJSONGluster, 0}, + {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, }; -- 2.9.0
--- src/util/virstoragefile.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 14 ++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9fc260b..c4dcd48 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2702,6 +2702,50 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *path = virJSONValueObjectGetString(json, "file.path"); + const char *host = virJSONValueObjectGetString(json, "file.host"); + const char *port = virJSONValueObjectGetString(json, "file.port"); + const char *export = virJSONValueObjectGetString(json, "file.export"); + + if (!path && !host) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing path or host of NBD server in JSON backing " + "volume definition")); + return -1; + } + + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_NBD; + + if (VIR_STRDUP(src->path, export) < 0) + return -1; + + if (VIR_ALLOC_N(src->hosts, 1) < 0) + return -1; + src->nhosts = 1; + + if (path) { + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; + if (VIR_STRDUP(src->hosts[0].socket, path) < 0) + return -1; + } else { + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + if (VIR_STRDUP(src->hosts[0].name, host) < 0) + return -1; + + if (VIR_STRDUP(src->hosts[0].port, port) < 0) + return -1; + } + + return 0; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2719,6 +2763,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, {"gluster", virStorageSourceParseBackingJSONGluster, 0}, {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, + {"nbd", virStorageSourceParseBackingJSONNbd, 0}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 4ebc1c5..220143e 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1402,6 +1402,20 @@ mymain(void) " <host transport='unix' socket='/path/socket'/>\n" " <host name='example.com'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"nbd\"," + "\"file.path\":\"/path/to/socket\"" + "}", + "<source protocol='nbd'>\n" + " <host transport='unix' socket='/path/to/socket'/>\n" + "</source>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"nbd\"," + "\"file.export\":\"blah\"," + "\"file.host\":\"example.org\"," + "\"file.port\":\"6000\"" + "}", + "<source protocol='nbd' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.9.0
--- src/util/virstoragefile.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c4dcd48..5b896ff 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2746,6 +2746,30 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *filename; + + /* legacy URI based syntax passed via 'filename' option */ + if ((filename = virJSONValueObjectGetString(json, "file.filename"))) { + if (strstr(filename, "://")) + return virStorageSourceParseBackingJSONUriStr(src, filename, + VIR_STORAGE_NET_PROTOCOL_SHEEPDOG); + + /* libvirt doesn't implement a parser for the legacy non-URI syntax */ + } + + /* Sheepdog currently supports only URI and legacy syntax passed in as filename */ + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing sheepdog URI in JSON backing volume definition")); + + return -1; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2764,6 +2788,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"gluster", virStorageSourceParseBackingJSONGluster, 0}, {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, {"nbd", virStorageSourceParseBackingJSONNbd, 0}, + {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, }; -- 2.9.0
Allow using 'ssh' protocol in backing chains and later for disks themselves. --- src/libxl/libxl_conf.c | 1 + src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_driver.c | 3 +++ src/qemu/qemu_parse_command.c | 1 + src/util/virstoragefile.c | 4 +++- src/util/virstoragefile.h | 1 + src/xenconfig/xen_xl.c | 1 + 7 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 146e08a..2d6d5da 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -581,6 +581,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4558b9f..3d6e4b5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -480,6 +480,9 @@ qemuNetworkDriveGetPort(int protocol, case VIR_STORAGE_NET_PROTOCOL_NBD: return 10809; + case VIR_STORAGE_NET_PROTOCOL_SSH: + return 22; + case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: /* no default port specified */ @@ -878,6 +881,10 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, ret = virBufferContentAndReset(&buf); break; + case VIR_STORAGE_NET_PROTOCOL_SSH: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'ssh' protocol is not yet supported")); + goto cleanup; case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0501003..dd5a9ff 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13372,6 +13372,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " @@ -13434,6 +13435,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external active snapshots are not supported on " @@ -13578,6 +13580,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("internal inactive snapshots are not supported on " diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 3f7e445..82d1621 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -2014,6 +2014,7 @@ qemuParseCommandLine(virCapsPtr caps, case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: /* ignored for now */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 5b896ff..41fbc96 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -84,7 +84,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST, "https", "ftp", "ftps", - "tftp") + "tftp", + "ssh") VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST, "tcp", @@ -2501,6 +2502,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_SSH: virReportError(VIR_ERR_INTERNAL_ERROR, _("malformed backing store path for protocol %s"), protocol); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1a76fad..3ea3a60 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -132,6 +132,7 @@ typedef enum { VIR_STORAGE_NET_PROTOCOL_FTP, VIR_STORAGE_NET_PROTOCOL_FTPS, VIR_STORAGE_NET_PROTOCOL_TFTP, + VIR_STORAGE_NET_PROTOCOL_SSH, VIR_STORAGE_NET_PROTOCOL_LAST } virStorageNetProtocol; diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index dcd4849..25a3621 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -727,6 +727,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src) case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, -- 2.9.0
--- src/util/virstoragefile.c | 38 ++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 9 +++++++++ 2 files changed, 47 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 41fbc96..92e0006 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2772,6 +2772,43 @@ virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src, } +static int +virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *path = virJSONValueObjectGetString(json, "file.path"); + const char *host = virJSONValueObjectGetString(json, "file.host"); + const char *port = virJSONValueObjectGetString(json, "file.port"); + + if (!host || !path) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing host or path of SSH JSON backing " + "volume definition")); + return -1; + } + + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_SSH; + + if (VIR_STRDUP(src->path, path) < 0) + return -1; + + if (VIR_ALLOC_N(src->hosts, 1) < 0) + return -1; + src->nhosts = 1; + + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + if (VIR_STRDUP(src->hosts[0].name, host) < 0) + return -1; + + if (VIR_STRDUP(src->hosts[0].port, port) < 0) + return -1; + + return 0; +} + + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -2791,6 +2828,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, {"nbd", virStorageSourceParseBackingJSONNbd, 0}, {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, + {"ssh", virStorageSourceParseBackingJSONSSH, 0}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 220143e..0309b33 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1416,6 +1416,15 @@ mymain(void) "<source protocol='nbd' name='blah'>\n" " <host name='example.org' port='6000'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"ssh\"," + "\"file.host\":\"example.org\"," + "\"file.port\":\"6000\"," + "\"file.path\":\"blah\"," + "\"file.user\":\"user\"" + "}", + "<source protocol='ssh' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); cleanup: /* Final cleanup */ -- 2.9.0
The iterator function (second argument) already requires that the object is handled as 'const' thus we won't modify the object itself. --- src/util/virjson.c | 2 +- src/util/virjson.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 1022cfc..afc98e3 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1220,7 +1220,7 @@ virJSONValueObjectIsNull(virJSONValuePtr object, * during iteration and -1 on generic errors. */ int -virJSONValueObjectForeachKeyValue(virJSONValuePtr object, +virJSONValueObjectForeachKeyValue(const virJSONValue *object, virJSONValueObjectIteratorFunc cb, void *opaque) { diff --git a/src/util/virjson.h b/src/util/virjson.h index 66ed48a..a5aef39 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -167,7 +167,7 @@ typedef int (*virJSONValueObjectIteratorFunc)(const char *key, const virJSONValue *value, void *opaque); -int virJSONValueObjectForeachKeyValue(virJSONValuePtr object, +int virJSONValueObjectForeachKeyValue(const virJSONValue *object, virJSONValueObjectIteratorFunc cb, void *opaque); -- 2.9.0
Refactor the command line generator by adding a wrapper (with documentation) that will handle the outermost object iteration. This patch also renames the functions and tweaks the error message for nested arrays to be more universal. The new function is then reused to simplify qemucommandutiltest. --- src/libvirt_private.syms | 1 + src/util/virqemu.c | 45 +++++++++++++++++++++++++++++++++------------ src/util/virqemu.h | 3 +++ tests/qemucommandutiltest.c | 22 ++++++++++++++-------- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9bc8d37..0ce95b2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2206,6 +2206,7 @@ virProcessWait; # util/virqemu.h virQEMUBuildBufferEscapeComma; +virQEMUBuildCommandLineJSON; virQEMUBuildLuksOpts; virQEMUBuildObjectCommandlineFromJSON; diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 7d181e1..a5d5385 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -33,10 +33,10 @@ VIR_LOG_INIT("util.qemu"); static int -virQEMUBuildObjectCommandLinePropsInternal(const char *key, - const virJSONValue *value, - virBufferPtr buf, - bool nested) +virQEMUBuildCommandLineJSONRecurse(const char *key, + const virJSONValue *value, + virBufferPtr buf, + bool nested) { virJSONValuePtr elem; virBitmapPtr bitmap = NULL; @@ -64,7 +64,8 @@ virQEMUBuildObjectCommandLinePropsInternal(const char *key, case VIR_JSON_TYPE_ARRAY: if (nested) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("nested -object property arrays are not supported")); + _("nested JSON array to commandline conversion is " + "not supported")); return -1; } @@ -87,8 +88,7 @@ virQEMUBuildObjectCommandLinePropsInternal(const char *key, elem = virJSONValueArrayGet((virJSONValuePtr)value, i); /* recurse to avoid duplicating code */ - if (virQEMUBuildObjectCommandLinePropsInternal(key, elem, buf, - true) < 0) + if (virQEMUBuildCommandLineJSONRecurse(key, elem, buf, true) < 0) return -1; } } @@ -108,11 +108,34 @@ virQEMUBuildObjectCommandLinePropsInternal(const char *key, static int -virQEMUBuildObjectCommandLineProps(const char *key, +virQEMUBuildCommandLineJSONIterate(const char *key, const virJSONValue *value, void *opaque) { - return virQEMUBuildObjectCommandLinePropsInternal(key, value, opaque, false); + return virQEMUBuildCommandLineJSONRecurse(key, value, opaque, false); +} + + +/** + * virQEMUBuildCommandLineJSON: + * @value: json object containing the value + * @buf: otuput buffer + * + * Formats JSON value object into command line parameters suitable for use with + * qemu. + * + * Returns 0 on success -1 on error. + */ +int +virQEMUBuildCommandLineJSON(const virJSONValue *value, + virBufferPtr buf) +{ + if (virJSONValueObjectForeachKeyValue(value, + virQEMUBuildCommandLineJSONIterate, + buf) < 0) + return -1; + + return 0; } @@ -126,9 +149,7 @@ virQEMUBuildObjectCommandlineFromJSON(const char *type, virBufferAsprintf(&buf, "%s,id=%s", type, alias); - if (virJSONValueObjectForeachKeyValue(props, - virQEMUBuildObjectCommandLineProps, - &buf) < 0) + if (virQEMUBuildCommandLineJSON(props, &buf) < 0) goto cleanup; if (virBufferCheckError(&buf) < 0) diff --git a/src/util/virqemu.h b/src/util/virqemu.h index eb75d1b..efc6c42 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -29,6 +29,9 @@ # include "virjson.h" # include "virstorageencryption.h" +int virQEMUBuildCommandLineJSON(const virJSONValue *value, + virBufferPtr buf); + char *virQEMUBuildObjectCommandlineFromJSON(const char *type, const char *alias, virJSONValuePtr props); diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index c02d1db..21fef1c 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -33,11 +33,12 @@ typedef struct } testQemuCommandBuildObjectFromJSONData; static int -testQemuCommandBuildObjectFromJSON(const void *opaque) +testQemuCommandBuildFromJSON(const void *opaque) { const testQemuCommandBuildObjectFromJSONData *data = opaque; virJSONValuePtr val = NULL; char *expect = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; char *result = NULL; int ret = -1; @@ -46,13 +47,18 @@ testQemuCommandBuildObjectFromJSON(const void *opaque) return -1; } - if (virAsprintf(&expect, "testobject,id=testalias%s%s", - data->expectprops ? "," : "", - data->expectprops ? data->expectprops : "") < 0) + if (data->expectprops && + virAsprintf(&expect, ",%s", data->expectprops) < 0) return -1; - result = virQEMUBuildObjectCommandlineFromJSON("testobject", - "testalias", val); + if (virQEMUBuildCommandLineJSON(val, &buf) < 0) { + fprintf(stderr, + "\nvirQEMUBuildCommandlineJSON failed process JSON:\n%s\n", + data->props); + goto cleanup; + } + + result = virBufferContentAndReset(&buf); if (STRNEQ_NULLABLE(expect, result)) { fprintf(stderr, "\nFailed to create object string. " @@ -80,14 +86,14 @@ mymain(void) return EXIT_AM_SKIP; #endif - virTestCounterReset("testQemuCommandBuildObjectFromJSON"); + virTestCounterReset("testQemuCommandBuildFromJSON"); #define DO_TEST_COMMAND_OBJECT_FROM_JSON(PROPS, EXPECT) \ do { \ data1.props = PROPS; \ data1.expectprops = EXPECT; \ if (virTestRun(virTestCounterNext(), \ - testQemuCommandBuildObjectFromJSON, \ + testQemuCommandBuildFromJSON, \ &data1) < 0) \ ret = -1; \ } while (0) -- 2.9.0
Until now the JSON->commandline convertor was used only for objects created by qemu. To allow reusing it with disk formatter we'll need to escape ',' as usual in qemu commandlines. --- src/util/virqemu.c | 3 ++- tests/qemucommandutiltest.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index a5d5385..99c14c2 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -46,7 +46,8 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, switch ((virJSONType) value->type) { case VIR_JSON_TYPE_STRING: - virBufferAsprintf(buf, ",%s=%s", key, value->data.string); + virBufferAsprintf(buf, ",%s=", key); + virQEMUBuildBufferEscapeComma(buf, value->data.string); break; case VIR_JSON_TYPE_NUMBER: diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index 21fef1c..8299462 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -100,6 +100,7 @@ mymain(void) DO_TEST_COMMAND_OBJECT_FROM_JSON("{}", NULL); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"string\":\"qwer\"}", "string=qwer"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"string\":\"qw,e,r\"}", "string=qw,,e,,r"); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"number\":1234}", "number=1234"); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"boolean\":true}", "boolean=yes"); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"boolean\":false}", "boolean=no"); -- 2.9.0
Move the iterator of objects to the recursive function so that nested objects are supported by flattening the structure with '.' delimiters. --- src/util/virqemu.c | 70 ++++++++++++++++++++++++++++++++++----------- tests/qemucommandutiltest.c | 8 ++++++ 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 99c14c2..df665ad 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -26,11 +26,49 @@ #include "virerror.h" #include "virlog.h" #include "virqemu.h" +#include "virstring.h" +#include "viralloc.h" #define VIR_FROM_THIS VIR_FROM_NONE VIR_LOG_INIT("util.qemu"); +struct virQEMUCommandLineJSONIteratorData { + const char *prefix; + virBufferPtr buf; +}; + + +static int +virQEMUBuildCommandLineJSONRecurse(const char *key, + const virJSONValue *value, + virBufferPtr buf, + bool nested); + +/* internal iterator to handle nested object formatting */ +static int +virQEMUBuildCommandLineJSONIterate(const char *key, + const virJSONValue *value, + void *opaque) +{ + struct virQEMUCommandLineJSONIteratorData *data = opaque; + char *tmpkey = NULL; + int ret = -1; + + if (data->prefix) { + if (virAsprintf(&tmpkey, "%s.%s", data->prefix, key) < 0) + return -1; + + ret = virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf, false); + + VIR_FREE(tmpkey); + } else { + ret = virQEMUBuildCommandLineJSONRecurse(key, value, data->buf, false); + } + + return ret; +} + static int virQEMUBuildCommandLineJSONRecurse(const char *key, @@ -38,12 +76,19 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, virBufferPtr buf, bool nested) { + struct virQEMUCommandLineJSONIteratorData data = { key, buf }; virJSONValuePtr elem; virBitmapPtr bitmap = NULL; ssize_t pos = -1; ssize_t end; size_t i; + if (!key && value->type != VIR_JSON_TYPE_OBJECT) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only JSON objects can be top level")); + return -1; + } + switch ((virJSONType) value->type) { case VIR_JSON_TYPE_STRING: virBufferAsprintf(buf, ",%s=", key); @@ -96,10 +141,15 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, break; case VIR_JSON_TYPE_OBJECT: + if (virJSONValueObjectForeachKeyValue(value, + virQEMUBuildCommandLineJSONIterate, + &data) < 0) + return -1; + break; + case VIR_JSON_TYPE_NULL: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("NULL and OBJECT JSON types can't be converted to " - "commandline string")); + _("NULL JSON type can't be converted to commandline")); return -1; } @@ -108,15 +158,6 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, } -static int -virQEMUBuildCommandLineJSONIterate(const char *key, - const virJSONValue *value, - void *opaque) -{ - return virQEMUBuildCommandLineJSONRecurse(key, value, opaque, false); -} - - /** * virQEMUBuildCommandLineJSON: * @value: json object containing the value @@ -131,12 +172,7 @@ int virQEMUBuildCommandLineJSON(const virJSONValue *value, virBufferPtr buf) { - if (virJSONValueObjectForeachKeyValue(value, - virQEMUBuildCommandLineJSONIterate, - buf) < 0) - return -1; - - return 0; + return virQEMUBuildCommandLineJSONRecurse(NULL, value, buf, false); } diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index 8299462..4872ea3 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -117,6 +117,14 @@ mymain(void) "array=bleah,array=qwerty,array=1"); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"boolean\":true,\"hyphen-name\":1234,\"some_string\":\"bleah\"}", "boolean=yes,hyphen-name=1234,some_string=bleah"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"nest\": {\"boolean\":true," + "\"hyphen-name\":1234," + "\"some_string\":\"bleah\"," + "\"bleah\":\"bl,eah\"" + "}" + "}", + "nest.boolean=yes,nest.hyphen-name=1234," + "nest.some_string=bleah,nest.bleah=bl,,eah"); return ret; -- 2.9.0
For use with memory hotplug virQEMUBuildCommandLineJSONRecurse attempted to format JSON arrays as bitmap on the command line. Make the formatter function configurable so that it can be reused with different syntaxes of arrays such as numbered arrays for use with disk sources. This patch extracts the code and adds a parameter for the function that will allow to plug in different formatters. --- src/libvirt_private.syms | 1 + src/util/virqemu.c | 73 ++++++++++++++++++++++++++++++--------------- src/util/virqemu.h | 10 ++++++- tests/qemucommandutiltest.c | 3 +- 4 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0ce95b2..9937200 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2207,6 +2207,7 @@ virProcessWait; # util/virqemu.h virQEMUBuildBufferEscapeComma; virQEMUBuildCommandLineJSON; +virQEMUBuildCommandLineJSONArrayBitmap; virQEMUBuildLuksOpts; virQEMUBuildObjectCommandlineFromJSON; diff --git a/src/util/virqemu.c b/src/util/virqemu.c index df665ad..3cc59e7 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -36,6 +36,7 @@ VIR_LOG_INIT("util.qemu"); struct virQEMUCommandLineJSONIteratorData { const char *prefix; virBufferPtr buf; + virQEMUBuildCommandLineJSONArrayFormatFunc arrayFunc; }; @@ -43,8 +44,41 @@ static int virQEMUBuildCommandLineJSONRecurse(const char *key, const virJSONValue *value, virBufferPtr buf, + virQEMUBuildCommandLineJSONArrayFormatFunc arrayFunc, bool nested); + + +int +virQEMUBuildCommandLineJSONArrayBitmap(const char *key, + const virJSONValue *array, + virBufferPtr buf) +{ + ssize_t pos = -1; + ssize_t end; + virBitmapPtr bitmap = NULL; + + if (virJSONValueGetArrayAsBitmap(array, &bitmap) < 0) + return -1; + + while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { + if ((end = virBitmapNextClearBit(bitmap, pos)) < 0) + end = virBitmapLastSetBit(bitmap) + 1; + + if (end - 1 > pos) { + virBufferAsprintf(buf, ",%s=%zd-%zd", key, pos, end - 1); + pos = end; + } else { + virBufferAsprintf(buf, ",%s=%zd", key, pos); + } + } + + virBitmapFree(bitmap); + + return 0; +} + + /* internal iterator to handle nested object formatting */ static int virQEMUBuildCommandLineJSONIterate(const char *key, @@ -59,11 +93,13 @@ virQEMUBuildCommandLineJSONIterate(const char *key, if (virAsprintf(&tmpkey, "%s.%s", data->prefix, key) < 0) return -1; - ret = virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf, false); + ret = virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf, + data->arrayFunc, false); VIR_FREE(tmpkey); } else { - ret = virQEMUBuildCommandLineJSONRecurse(key, value, data->buf, false); + ret = virQEMUBuildCommandLineJSONRecurse(key, value, data->buf, + data->arrayFunc, false); } return ret; @@ -74,13 +110,11 @@ static int virQEMUBuildCommandLineJSONRecurse(const char *key, const virJSONValue *value, virBufferPtr buf, + virQEMUBuildCommandLineJSONArrayFormatFunc arrayFunc, bool nested) { - struct virQEMUCommandLineJSONIteratorData data = { key, buf }; + struct virQEMUCommandLineJSONIteratorData data = { key, buf, arrayFunc }; virJSONValuePtr elem; - virBitmapPtr bitmap = NULL; - ssize_t pos = -1; - ssize_t end; size_t i; if (!key && value->type != VIR_JSON_TYPE_OBJECT) { @@ -115,26 +149,15 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, return -1; } - if (virJSONValueGetArrayAsBitmap(value, &bitmap) == 0) { - while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { - if ((end = virBitmapNextClearBit(bitmap, pos)) < 0) - end = virBitmapLastSetBit(bitmap) + 1; - - if (end - 1 > pos) { - virBufferAsprintf(buf, ",%s=%zd-%zd", key, pos, end - 1); - pos = end; - } else { - virBufferAsprintf(buf, ",%s=%zd", key, pos); - } - } - } else { + if (!arrayFunc || arrayFunc(key, value, buf) < 0) { /* fallback, treat the array as a non-bitmap, adding the key * for each member */ for (i = 0; i < virJSONValueArraySize(value); i++) { elem = virJSONValueArrayGet((virJSONValuePtr)value, i); /* recurse to avoid duplicating code */ - if (virQEMUBuildCommandLineJSONRecurse(key, elem, buf, true) < 0) + if (virQEMUBuildCommandLineJSONRecurse(key, elem, buf, + arrayFunc, true) < 0) return -1; } } @@ -153,7 +176,6 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, return -1; } - virBitmapFree(bitmap); return 0; } @@ -162,6 +184,7 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, * virQEMUBuildCommandLineJSON: * @value: json object containing the value * @buf: otuput buffer + * @arrayFunc: array formatter function to allow for different syntax * * Formats JSON value object into command line parameters suitable for use with * qemu. @@ -170,9 +193,10 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, */ int virQEMUBuildCommandLineJSON(const virJSONValue *value, - virBufferPtr buf) + virBufferPtr buf, + virQEMUBuildCommandLineJSONArrayFormatFunc array) { - return virQEMUBuildCommandLineJSONRecurse(NULL, value, buf, false); + return virQEMUBuildCommandLineJSONRecurse(NULL, value, buf, array, false); } @@ -186,7 +210,8 @@ virQEMUBuildObjectCommandlineFromJSON(const char *type, virBufferAsprintf(&buf, "%s,id=%s", type, alias); - if (virQEMUBuildCommandLineJSON(props, &buf) < 0) + if (virQEMUBuildCommandLineJSON(props, &buf, + virQEMUBuildCommandLineJSONArrayBitmap) < 0) goto cleanup; if (virBufferCheckError(&buf) < 0) diff --git a/src/util/virqemu.h b/src/util/virqemu.h index efc6c42..801c35b 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -29,8 +29,16 @@ # include "virjson.h" # include "virstorageencryption.h" +typedef int (*virQEMUBuildCommandLineJSONArrayFormatFunc)(const char *key, + const virJSONValue *array, + virBufferPtr buf); +int virQEMUBuildCommandLineJSONArrayBitmap(const char *key, + const virJSONValue *array, + virBufferPtr buf); + int virQEMUBuildCommandLineJSON(const virJSONValue *value, - virBufferPtr buf); + virBufferPtr buf, + virQEMUBuildCommandLineJSONArrayFormatFunc array); char *virQEMUBuildObjectCommandlineFromJSON(const char *type, const char *alias, diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index 4872ea3..a5e3153 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -51,7 +51,8 @@ testQemuCommandBuildFromJSON(const void *opaque) virAsprintf(&expect, ",%s", data->expectprops) < 0) return -1; - if (virQEMUBuildCommandLineJSON(val, &buf) < 0) { + if (virQEMUBuildCommandLineJSON(val, &buf, + virQEMUBuildCommandLineJSONArrayBitmap) < 0) { fprintf(stderr, "\nvirQEMUBuildCommandlineJSON failed process JSON:\n%s\n", data->props); -- 2.9.0
The function would generate a leading comma. Let the callers properly add commas by formatting the commas at the end and trimming the trailing one. --- src/util/virqemu.c | 22 ++++++++++++++-------- tests/qemucommandutiltest.c | 10 ++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 3cc59e7..8babe36 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -66,10 +66,10 @@ virQEMUBuildCommandLineJSONArrayBitmap(const char *key, end = virBitmapLastSetBit(bitmap) + 1; if (end - 1 > pos) { - virBufferAsprintf(buf, ",%s=%zd-%zd", key, pos, end - 1); + virBufferAsprintf(buf, "%s=%zd-%zd,", key, pos, end - 1); pos = end; } else { - virBufferAsprintf(buf, ",%s=%zd", key, pos); + virBufferAsprintf(buf, "%s=%zd,", key, pos); } } @@ -125,19 +125,20 @@ virQEMUBuildCommandLineJSONRecurse(const char *key, switch ((virJSONType) value->type) { case VIR_JSON_TYPE_STRING: - virBufferAsprintf(buf, ",%s=", key); + virBufferAsprintf(buf, "%s=", key); virQEMUBuildBufferEscapeComma(buf, value->data.string); + virBufferAddLit(buf, ","); break; case VIR_JSON_TYPE_NUMBER: - virBufferAsprintf(buf, ",%s=%s", key, value->data.number); + virBufferAsprintf(buf, "%s=%s,", key, value->data.number); break; case VIR_JSON_TYPE_BOOLEAN: if (value->data.boolean) - virBufferAsprintf(buf, ",%s=yes", key); + virBufferAsprintf(buf, "%s=yes,", key); else - virBufferAsprintf(buf, ",%s=no", key); + virBufferAsprintf(buf, "%s=no,", key); break; @@ -196,7 +197,12 @@ virQEMUBuildCommandLineJSON(const virJSONValue *value, virBufferPtr buf, virQEMUBuildCommandLineJSONArrayFormatFunc array) { - return virQEMUBuildCommandLineJSONRecurse(NULL, value, buf, array, false); + if (virQEMUBuildCommandLineJSONRecurse(NULL, value, buf, array, false) < 0) + return -1; + + virBufferTrim(buf, ",", -1); + + return 0; } @@ -208,7 +214,7 @@ virQEMUBuildObjectCommandlineFromJSON(const char *type, virBuffer buf = VIR_BUFFER_INITIALIZER; char *ret = NULL; - virBufferAsprintf(&buf, "%s,id=%s", type, alias); + virBufferAsprintf(&buf, "%s,id=%s,", type, alias); if (virQEMUBuildCommandLineJSON(props, &buf, virQEMUBuildCommandLineJSONArrayBitmap) < 0) diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index a5e3153..0bf0351 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -37,7 +37,6 @@ testQemuCommandBuildFromJSON(const void *opaque) { const testQemuCommandBuildObjectFromJSONData *data = opaque; virJSONValuePtr val = NULL; - char *expect = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *result = NULL; int ret = -1; @@ -47,10 +46,6 @@ testQemuCommandBuildFromJSON(const void *opaque) return -1; } - if (data->expectprops && - virAsprintf(&expect, ",%s", data->expectprops) < 0) - return -1; - if (virQEMUBuildCommandLineJSON(val, &buf, virQEMUBuildCommandLineJSONArrayBitmap) < 0) { fprintf(stderr, @@ -61,10 +56,10 @@ testQemuCommandBuildFromJSON(const void *opaque) result = virBufferContentAndReset(&buf); - if (STRNEQ_NULLABLE(expect, result)) { + if (STRNEQ_NULLABLE(data->expectprops, result)) { fprintf(stderr, "\nFailed to create object string. " "\nExpected:\n'%s'\nGot:\n'%s'", - NULLSTR(expect), NULLSTR(result)); + NULLSTR(data->expectprops), NULLSTR(result)); goto cleanup; } @@ -72,7 +67,6 @@ testQemuCommandBuildFromJSON(const void *opaque) cleanup: virJSONValueFree(val); VIR_FREE(result); - VIR_FREE(expect); return ret; } -- 2.9.0
The function builds also non-uri strings for the various protocols. --- src/qemu/qemu_command.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3d6e4b5..96cc202 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -691,7 +691,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf, #define QEMU_DEFAULT_NBD_PORT "10809" static char * -qemuBuildNetworkDriveURI(virStorageSourcePtr src, +qemuBuildNetworkDriveStr(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo) { char *ret = NULL; @@ -926,7 +926,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, break; case VIR_STORAGE_TYPE_NETWORK: - if (!(*source = qemuBuildNetworkDriveURI(src, secinfo))) + if (!(*source = qemuBuildNetworkDriveStr(src, secinfo))) goto cleanup; break; @@ -4534,7 +4534,7 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) src.nhosts = iscsisrc->nhosts; /* Rather than pull what we think we want - use the network disk code */ - source = qemuBuildNetworkDriveURI(&src, hostdevPriv->secinfo); + source = qemuBuildNetworkDriveStr(&src, hostdevPriv->secinfo); return source; } -- 2.9.0
Extract the code so that it can be called from multiple places. This also removes a tricky fallthrough in the large switch in qemuBuildNetworkDriveStr. --- src/qemu/qemu_command.c | 122 +++++++++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 54 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 96cc202..c55456f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -690,13 +690,76 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf, #define QEMU_DEFAULT_NBD_PORT "10809" + +static char * +qemuBuildNetworkDriveURI(virStorageSourcePtr src, + qemuDomainSecretInfoPtr secinfo) +{ + virURIPtr uri = NULL; + char *ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("protocol '%s' accepts only one host"), + virStorageNetProtocolTypeToString(src->protocol)); + goto cleanup; + } + + if (VIR_ALLOC(uri) < 0) + goto cleanup; + + if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + if (VIR_STRDUP(uri->scheme, + virStorageNetProtocolTypeToString(src->protocol)) < 0) + goto cleanup; + } else { + if (virAsprintf(&uri->scheme, "%s+%s", + virStorageNetProtocolTypeToString(src->protocol), + virStorageNetHostTransportTypeToString(src->hosts->transport)) < 0) + goto cleanup; + } + + if ((uri->port = qemuNetworkDriveGetPort(src->protocol, + src->hosts->port)) < 0) + goto cleanup; + + if (src->path) { + if (src->volume) { + if (virAsprintf(&uri->path, "/%s%s", + src->volume, src->path) < 0) + goto cleanup; + } else { + if (virAsprintf(&uri->path, "%s%s", + src->path[0] == '/' ? "" : "/", + src->path) < 0) + goto cleanup; + } + } + + if (src->hosts->socket && + virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0) + goto cleanup; + + if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) + goto cleanup; + + if (VIR_STRDUP(uri->server, src->hosts->name) < 0) + goto cleanup; + + ret = virURIFormat(uri); + + cleanup: + virURIFree(uri); + return ret; +} + + static char * qemuBuildNetworkDriveStr(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo) { char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - virURIPtr uri = NULL; size_t i; switch ((virStorageNetProtocol) src->protocol) { @@ -752,8 +815,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, ret = virBufferContentAndReset(&buf); goto cleanup; } - /* fallthrough */ - /* NBD code uses same formatting scheme as others in some cases */ + /* NBD code uses URI formatting scheme as others in some cases */ + ret = qemuBuildNetworkDriveURI(src, secinfo); + break; case VIR_STORAGE_NET_PROTOCOL_HTTP: case VIR_STORAGE_NET_PROTOCOL_HTTPS: @@ -762,56 +826,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - if (src->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("protocol '%s' accepts only one host"), - virStorageNetProtocolTypeToString(src->protocol)); - goto cleanup; - } - - if (VIR_ALLOC(uri) < 0) - goto cleanup; - - if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { - if (VIR_STRDUP(uri->scheme, - virStorageNetProtocolTypeToString(src->protocol)) < 0) - goto cleanup; - } else { - if (virAsprintf(&uri->scheme, "%s+%s", - virStorageNetProtocolTypeToString(src->protocol), - virStorageNetHostTransportTypeToString(src->hosts->transport)) < 0) - goto cleanup; - } - - if ((uri->port = qemuNetworkDriveGetPort(src->protocol, - src->hosts->port)) < 0) - goto cleanup; - - if (src->path) { - if (src->volume) { - if (virAsprintf(&uri->path, "/%s%s", - src->volume, src->path) < 0) - goto cleanup; - } else { - if (virAsprintf(&uri->path, "%s%s", - src->path[0] == '/' ? "" : "/", - src->path) < 0) - goto cleanup; - } - } - - if (src->hosts->socket && - virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0) - goto cleanup; - - if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) - goto cleanup; - - if (VIR_STRDUP(uri->server, src->hosts->name) < 0) - goto cleanup; - - ret = virURIFormat(uri); - + ret = qemuBuildNetworkDriveURI(src, secinfo); break; case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -896,7 +911,6 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, cleanup: virBufferFreeAndReset(&buf); - virURIFree(uri); return ret; } -- 2.9.0
The disk source formatting code grew rather ugly and complex and it will get worse. Extract it into a separated function to contain the mess. --- src/qemu/qemu_command.c | 159 ++++++++++++++++++++++++++---------------------- 1 file changed, 87 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c55456f..ee7329c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1093,6 +1093,92 @@ qemuDiskBusNeedsDeviceArg(int bus) } +static int +qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, + virBufferPtr buf) +{ + int actualType = virStorageSourceGetActualType(disk->src); + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; + char *source = NULL; + int ret = -1; + + if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0) + goto cleanup; + + if (source && + !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || + disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { + + virBufferAddLit(buf, "file="); + + switch (actualType) { + case VIR_STORAGE_TYPE_DIR: + /* QEMU only supports magic FAT format for now */ + if (disk->src->format > 0 && + disk->src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(disk->src->format)); + goto cleanup; + } + + if (!disk->src->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto cleanup; + } + + virBufferAddLit(buf, "fat:"); + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + virBufferAddLit(buf, "floppy:"); + + break; + + case VIR_STORAGE_TYPE_BLOCK: + if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + disk->src->type == VIR_STORAGE_TYPE_VOLUME ? + _("tray status 'open' is invalid for block type volume") : + _("tray status 'open' is invalid for block type disk")); + goto cleanup; + } + + break; + + default: + break; + } + + virQEMUBuildBufferEscapeComma(buf, source); + virBufferAddLit(buf, ","); + + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + virBufferAsprintf(buf, "password-secret=%s,", + secinfo->s.aes.alias); + } + + if (encinfo) + virQEMUBuildLuksOpts(buf, &disk->src->encryption->encinfo, + encinfo->s.aes.alias); + + if (disk->src->format > 0 && + disk->src->type != VIR_STORAGE_TYPE_DIR) + virBufferAsprintf(buf, "format=%s,", + virStorageFileFormatTypeToString(disk->src->format)); + } + + ret = 0; + + cleanup: + VIR_FREE(source); + return ret; +} + + char * qemuBuildDriveStr(virDomainDiskDefPtr disk, bool bootable, @@ -1104,11 +1190,6 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virDomainDiskGeometryTransTypeToString(disk->geometry.trans); int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1; - char *source = NULL; - int actualType = virStorageSourceGetActualType(disk->src); - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; - qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); if (idx < 0) { @@ -1191,74 +1272,9 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; } - if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0) + if (qemuBuildDriveSourceStr(disk, &opt) < 0) goto error; - if (source && - !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || - disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && - disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { - - virBufferAddLit(&opt, "file="); - - switch (actualType) { - case VIR_STORAGE_TYPE_DIR: - /* QEMU only supports magic FAT format for now */ - if (disk->src->format > 0 && - disk->src->format != VIR_STORAGE_FILE_FAT) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->src->format)); - goto error; - } - - if (!disk->src->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto error; - } - - virBufferAddLit(&opt, "fat:"); - - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAddLit(&opt, "floppy:"); - - break; - - case VIR_STORAGE_TYPE_BLOCK: - if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - disk->src->type == VIR_STORAGE_TYPE_VOLUME ? - _("tray status 'open' is invalid for block type volume") : - _("tray status 'open' is invalid for block type disk")); - goto error; - } - - break; - - default: - break; - } - - virQEMUBuildBufferEscapeComma(&opt, source); - virBufferAddLit(&opt, ","); - - if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { - virBufferAsprintf(&opt, "password-secret=%s,", - secinfo->s.aes.alias); - } - - if (encinfo) - virQEMUBuildLuksOpts(&opt, &disk->src->encryption->encinfo, - encinfo->s.aes.alias); - - if (disk->src->format > 0 && - disk->src->type != VIR_STORAGE_TYPE_DIR) - virBufferAsprintf(&opt, "format=%s,", - virStorageFileFormatTypeToString(disk->src->format)); - } - VIR_FREE(source); - if (emitDeviceSyntax) virBufferAddLit(&opt, "if=none"); else @@ -1581,7 +1597,6 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, return virBufferContentAndReset(&opt); error: - VIR_FREE(source); virBufferFreeAndReset(&opt); return NULL; } -- 2.9.0
Avoid a large block by tweaking the condition skipping empty drives and split up the switch containing two branches having different purpose. --- src/qemu/qemu_command.c | 99 +++++++++++++++++++++++-------------------------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee7329c..3b42b73 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1107,70 +1107,63 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0) goto cleanup; - if (source && - !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || - disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && - disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { - - virBufferAddLit(buf, "file="); - - switch (actualType) { - case VIR_STORAGE_TYPE_DIR: - /* QEMU only supports magic FAT format for now */ - if (disk->src->format > 0 && - disk->src->format != VIR_STORAGE_FILE_FAT) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->src->format)); - goto cleanup; - } - - if (!disk->src->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto cleanup; - } - - virBufferAddLit(buf, "fat:"); + /* nothing to format if the drive is empty */ + if (!source || + ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || + disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) + return 0; - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAddLit(buf, "floppy:"); + if (actualType == VIR_STORAGE_TYPE_BLOCK && + disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + disk->src->type == VIR_STORAGE_TYPE_VOLUME ? + _("tray status 'open' is invalid for block type volume") : + _("tray status 'open' is invalid for block type disk")); + goto cleanup; + } - break; + virBufferAddLit(buf, "file="); - case VIR_STORAGE_TYPE_BLOCK: - if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - disk->src->type == VIR_STORAGE_TYPE_VOLUME ? - _("tray status 'open' is invalid for block type volume") : - _("tray status 'open' is invalid for block type disk")); - goto cleanup; - } - - break; + /* for now the DIR based storage is handled by the magic FAT format */ + if (actualType == VIR_STORAGE_TYPE_DIR) { + if (disk->src->format > 0 && + disk->src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(disk->src->format)); + goto cleanup; + } - default: - break; + if (!disk->src->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto cleanup; } - virQEMUBuildBufferEscapeComma(buf, source); - virBufferAddLit(buf, ","); + virBufferAddLit(buf, "fat:"); - if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { - virBufferAsprintf(buf, "password-secret=%s,", - secinfo->s.aes.alias); - } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + virBufferAddLit(buf, "floppy:"); + } - if (encinfo) - virQEMUBuildLuksOpts(buf, &disk->src->encryption->encinfo, - encinfo->s.aes.alias); + virQEMUBuildBufferEscapeComma(buf, source); + virBufferAddLit(buf, ","); - if (disk->src->format > 0 && - disk->src->type != VIR_STORAGE_TYPE_DIR) - virBufferAsprintf(buf, "format=%s,", - virStorageFileFormatTypeToString(disk->src->format)); + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + virBufferAsprintf(buf, "password-secret=%s,", + secinfo->s.aes.alias); } + if (encinfo) + virQEMUBuildLuksOpts(buf, &disk->src->encryption->encinfo, + encinfo->s.aes.alias); + + if (disk->src->format > 0 && + disk->src->type != VIR_STORAGE_TYPE_DIR) + virBufferAsprintf(buf, "format=%s,", + virStorageFileFormatTypeToString(disk->src->format)); + ret = 0; cleanup: -- 2.9.0
On Mon, Jul 25, 2016 at 08:12:06PM +0200, Peter Krempa wrote:
Avoid a large block by tweaking the condition skipping empty drives and split up the switch containing two branches having different purpose. --- src/qemu/qemu_command.c | 99 +++++++++++++++++++++++-------------------------- 1 file changed, 46 insertions(+), 53 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee7329c..3b42b73 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1107,70 +1107,63 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0) goto cleanup;
- if (source && - !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || - disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && - disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { - - virBufferAddLit(buf, "file="); - - switch (actualType) { - case VIR_STORAGE_TYPE_DIR: - /* QEMU only supports magic FAT format for now */ - if (disk->src->format > 0 && - disk->src->format != VIR_STORAGE_FILE_FAT) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->src->format)); - goto cleanup; - } - - if (!disk->src->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto cleanup; - } - - virBufferAddLit(buf, "fat:"); + /* nothing to format if the drive is empty */ + if (!source || + ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || + disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) + return 0;
Looks like this would leak source if it's non-NULL. Jan
As gluster natively supports multiple hosts for failover reasons we can easily add the support to the storage driver code in libvirt. Extract the code setting an individual host into a separate function and call them in a loop. The new code also tries to keep the debug log entries sane. --- src/storage/storage_backend_gluster.c | 82 ++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 31 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index eda060d..5bcbef4 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -570,25 +570,55 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src) } static int -virStorageFileBackendGlusterInit(virStorageSourcePtr src) +virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPrivPtr priv, + virStorageNetHostDefPtr host) { - virStorageFileBackendGlusterPrivPtr priv = NULL; - virStorageNetHostDefPtr host = &(src->hosts[0]); - const char *hostname; + const char *transport = virStorageNetHostTransportTypeToString(host->transport); + const char *hoststr = NULL; int port = 0; - if (src->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected exactly 1 host for the gluster volume")); + switch ((virStorageNetHostTransport) host->transport) { + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_TCP: + hoststr = host->name; + + if (host->port && + virStrToLong_i(host->port, NULL, 10, &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + host->port); + return -1; + } + + break; + + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + hoststr = host->socket; + break; + + case VIR_STORAGE_NET_HOST_TRANS_LAST: + break; + } + + VIR_DEBUG("adding gluster host for %p: transport=%s host=%s port=%d", + priv, transport, hoststr, port); + + if (glfs_set_volfile_server(priv->vol, transport, hoststr, port) < 0) { + virReportSystemError(errno, + _("failed to set gluster volfile server '%s'"), + hoststr); return -1; } - hostname = host->name; + return 0; +} - VIR_DEBUG("initializing gluster storage file %p (gluster://%s:%s/%s%s)[%u:%u]", - src, hostname, host->port ? host->port : "0", - NULLSTR(src->volume), src->path, - (unsigned int)src->drv->uid, (unsigned int)src->drv->gid); + +static int +virStorageFileBackendGlusterInit(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = NULL; + size_t i; if (!src->volume) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -600,35 +630,25 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) if (VIR_ALLOC(priv) < 0) return -1; - if (host->port && - virStrToLong_i(host->port, NULL, 10, &port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse port number '%s'"), - host->port); - goto error; - } - - if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) - hostname = host->socket; + VIR_DEBUG("initializing gluster storage file %p " + "(priv='%p' volume='%s' path='%s') as [%u:%u]", + src, priv, src->volume, src->path, + (unsigned int)src->drv->uid, (unsigned int)src->drv->gid); if (!(priv->vol = glfs_new(src->volume))) { virReportOOMError(); goto error; } - if (glfs_set_volfile_server(priv->vol, - virStorageNetHostTransportTypeToString(host->transport), - hostname, port) < 0) { - virReportSystemError(errno, - _("failed to set gluster volfile server '%s'"), - hostname); - goto error; + for (i = 0; i < src->nhosts; i++) { + if (virStorageFileBackendGlusterInitServer(priv, src->hosts + i) < 0) + goto error; } if (glfs_init(priv->vol) < 0) { virReportSystemError(errno, - _("failed to initialize gluster connection to " - "server: '%s'"), hostname); + _("failed to initialize gluster connection " + "(src=%p priv=%p)"), src, priv); goto error; } -- 2.9.0
Add support for converting objects nested in arrays with a numbering discriminator on the command line. This syntax is used for the object-based specification of disk source properties. --- src/libvirt_private.syms | 1 + src/util/virqemu.c | 33 +++++++++++++++++++++++++++++++++ src/util/virqemu.h | 3 +++ tests/qemucommandutiltest.c | 36 +++++++++++++++++++++++++++++++++--- 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9937200..296b036 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2208,6 +2208,7 @@ virProcessWait; virQEMUBuildBufferEscapeComma; virQEMUBuildCommandLineJSON; virQEMUBuildCommandLineJSONArrayBitmap; +virQEMUBuildCommandLineJSONArrayNumbered; virQEMUBuildLuksOpts; virQEMUBuildObjectCommandlineFromJSON; diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 8babe36..20410f7 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -79,6 +79,39 @@ virQEMUBuildCommandLineJSONArrayBitmap(const char *key, } +int +virQEMUBuildCommandLineJSONArrayNumbered(const char *key, + const virJSONValue *array, + virBufferPtr buf) +{ + const virJSONValue *member; + size_t nelems = virJSONValueArraySize(array); + char *prefix = NULL; + size_t i; + int ret = 0; + + for (i = 0; i < nelems; i++) { + member = virJSONValueArrayGet((virJSONValuePtr) array, i); + + if (virAsprintf(&prefix, "%s.%zu", key, i) < 0) + goto cleanup; + + if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, + virQEMUBuildCommandLineJSONArrayNumbered, + true) < 0) + goto cleanup; + + VIR_FREE(prefix); + } + + ret = 0; + + cleanup: + VIR_FREE(prefix); + return ret; +} + + /* internal iterator to handle nested object formatting */ static int virQEMUBuildCommandLineJSONIterate(const char *key, diff --git a/src/util/virqemu.h b/src/util/virqemu.h index 801c35b..40cd9b8 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -35,6 +35,9 @@ typedef int (*virQEMUBuildCommandLineJSONArrayFormatFunc)(const char *key, int virQEMUBuildCommandLineJSONArrayBitmap(const char *key, const virJSONValue *array, virBufferPtr buf); +int virQEMUBuildCommandLineJSONArrayNumbered(const char *key, + const virJSONValue *array, + virBufferPtr buf); int virQEMUBuildCommandLineJSON(const virJSONValue *value, virBufferPtr buf, diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index 0bf0351..1985983 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -30,6 +30,7 @@ typedef struct { const char *props; const char *expectprops; + virQEMUBuildCommandLineJSONArrayFormatFunc arrayfunc; } testQemuCommandBuildObjectFromJSONData; static int @@ -46,8 +47,7 @@ testQemuCommandBuildFromJSON(const void *opaque) return -1; } - if (virQEMUBuildCommandLineJSON(val, &buf, - virQEMUBuildCommandLineJSONArrayBitmap) < 0) { + if (virQEMUBuildCommandLineJSON(val, &buf, data->arrayfunc) < 0) { fprintf(stderr, "\nvirQEMUBuildCommandlineJSON failed process JSON:\n%s\n", data->props); @@ -83,16 +83,23 @@ mymain(void) virTestCounterReset("testQemuCommandBuildFromJSON"); -#define DO_TEST_COMMAND_OBJECT_FROM_JSON(PROPS, EXPECT) \ +#define DO_TEST_COMMAND_FROM_JSON(PROPS, ARRAYFUNC, EXPECT) \ do { \ data1.props = PROPS; \ data1.expectprops = EXPECT; \ + data1.arrayfunc = ARRAYFUNC; \ if (virTestRun(virTestCounterNext(), \ testQemuCommandBuildFromJSON, \ &data1) < 0) \ ret = -1; \ } while (0) +#define DO_TEST_COMMAND_OBJECT_FROM_JSON(PROPS, EXPECT) \ + DO_TEST_COMMAND_FROM_JSON(PROPS, virQEMUBuildCommandLineJSONArrayBitmap, EXPECT) + +#define DO_TEST_COMMAND_DRIVE_FROM_JSON(PROPS, EXPECT) \ + DO_TEST_COMMAND_FROM_JSON(PROPS, virQEMUBuildCommandLineJSONArrayNumbered, EXPECT) + DO_TEST_COMMAND_OBJECT_FROM_JSON("{}", NULL); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"string\":\"qwer\"}", "string=qwer"); DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"string\":\"qw,e,r\"}", "string=qw,,e,,r"); @@ -120,6 +127,29 @@ mymain(void) "}", "nest.boolean=yes,nest.hyphen-name=1234," "nest.some_string=bleah,nest.bleah=bl,,eah"); + DO_TEST_COMMAND_DRIVE_FROM_JSON("{\"driver\":\"gluster\"," + "\"volume\":\"test\"," + "\"path\":\"img\"," + "\"server\":[ { \"type\":\"tcp\"," + "\"host\":\"example.com\"," + "\"port\":\"1234\"" + "}," + "{ \"type\":\"unix\"," + "\"socket\":\"/path/socket\"" + "}," + "{ \"type\":\"tcp\"," + "\"host\":\"example.com\"" + "}" + "]" + "}", + "driver=gluster,volume=test,path=img," + "server.0.type=tcp," + "server.0.host=example.com," + "server.0.port=1234," + "server.1.type=unix," + "server.1.socket=/path/socket," + "server.2.type=tcp," + "server.2.host=example.com"); return ret; -- 2.9.0
To allow richer definitions of disk sources add infrastructure that will allow to register functionst generating a JSON object based definition. This infrastructure will then convert the definition to the proper command line syntax and use it in cases where it's necessary. This will allow to keep legacy definitions for back-compat when possible and use the new definitions for the configurations requiring them. --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 83 ++++++++++++++++++++++++++++++++++++------------ src/util/virqemu.c | 21 ++++++++++++ src/util/virqemu.h | 2 ++ 4 files changed, 86 insertions(+), 21 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 296b036..4614a7b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2209,6 +2209,7 @@ virQEMUBuildBufferEscapeComma; virQEMUBuildCommandLineJSON; virQEMUBuildCommandLineJSONArrayBitmap; virQEMUBuildCommandLineJSONArrayNumbered; +virQEMUBuildDriveCommandlineFromJSON; virQEMUBuildLuksOpts; virQEMUBuildObjectCommandlineFromJSON; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3b42b73..5a5df7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -916,6 +916,34 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, } +static int +qemuGetDriveSourceProps(virStorageSourcePtr src, + virJSONValuePtr *props) +{ + int actualType = virStorageSourceGetActualType(src); + virJSONValuePtr fileprops = NULL; + + *props = NULL; + + switch ((virStorageType) actualType) { + case VIR_STORAGE_TYPE_BLOCK: + case VIR_STORAGE_TYPE_FILE: + case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NONE: + case VIR_STORAGE_TYPE_LAST: + case VIR_STORAGE_TYPE_NETWORK: + break; + } + + if (fileprops && + virJSONValueObjectCreate(props, "a:file", fileprops, NULL) < 0) + return -1; + + return 0; +} + + int qemuGetDriveSourceString(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo, @@ -1101,14 +1129,19 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; + virJSONValuePtr srcprops = NULL; char *source = NULL; int ret = -1; - if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0) + if (qemuGetDriveSourceProps(disk->src, &srcprops) < 0) + goto cleanup; + + if (!srcprops && + qemuGetDriveSourceString(disk->src, secinfo, &source) < 0) goto cleanup; /* nothing to format if the drive is empty */ - if (!source || + if (!(source || srcprops) || ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) @@ -1123,31 +1156,38 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, goto cleanup; } - virBufferAddLit(buf, "file="); + if (source) { + virBufferAddLit(buf, "file="); - /* for now the DIR based storage is handled by the magic FAT format */ - if (actualType == VIR_STORAGE_TYPE_DIR) { - if (disk->src->format > 0 && - disk->src->format != VIR_STORAGE_FILE_FAT) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->src->format)); - goto cleanup; - } + /* for now the DIR based storage is handled by the magic FAT format */ + if (actualType == VIR_STORAGE_TYPE_DIR) { + if (disk->src->format > 0 && + disk->src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(disk->src->format)); + goto cleanup; + } - if (!disk->src->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto cleanup; + if (!disk->src->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto cleanup; + } + + virBufferAddLit(buf, "fat:"); + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + virBufferAddLit(buf, "floppy:"); } - virBufferAddLit(buf, "fat:"); + virQEMUBuildBufferEscapeComma(buf, source); + } else { + if (!(source = virQEMUBuildDriveCommandlineFromJSON(srcprops))) + goto cleanup; - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAddLit(buf, "floppy:"); + virBufferAdd(buf, source, -1); } - - virQEMUBuildBufferEscapeComma(buf, source); virBufferAddLit(buf, ","); if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { @@ -1168,6 +1208,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, cleanup: VIR_FREE(source); + virJSONValueFree(srcprops); return ret; } diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 20410f7..a1ba562 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -264,6 +264,27 @@ virQEMUBuildObjectCommandlineFromJSON(const char *type, } +char * +virQEMUBuildDriveCommandlineFromJSON(const virJSONValue *srcdef) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *ret = NULL; + + if (virQEMUBuildCommandLineJSON(srcdef, &buf, + virQEMUBuildCommandLineJSONArrayNumbered) < 0) + goto cleanup; + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + ret = virBufferContentAndReset(&buf); + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} + + /** * virQEMUBuildBufferEscapeComma: * @buf: buffer to append the escaped string diff --git a/src/util/virqemu.h b/src/util/virqemu.h index 40cd9b8..f3c2b69 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -47,6 +47,8 @@ char *virQEMUBuildObjectCommandlineFromJSON(const char *type, const char *alias, virJSONValuePtr props); +char *virQEMUBuildDriveCommandlineFromJSON(const virJSONValue *src); + void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str); void virQEMUBuildLuksOpts(virBufferPtr buf, virStorageEncryptionInfoDefPtr enc, -- 2.9.0
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> To allow using failover with gluster it's necessary to specify multiple volume hosts. Add support for starting qemu with such configurations. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.html.in | 2 +- src/qemu/qemu_command.c | 102 +++++++++++++++++++++ .../qemuxml2argv-disk-drive-network-gluster.args | 9 +- .../qemuxml2argv-disk-drive-network-gluster.xml | 9 ++ .../qemuxml2xmlout-disk-drive-network-gluster.xml | 10 ++ 5 files changed, 130 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 59a8bb9..8efd6af 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2280,7 +2280,7 @@ <tr> <td> gluster </td> <td> a server running glusterd daemon </td> - <td> only one </td> + <td> one or more (<span class="since">Since 2.1.0</span>), just one prior to that </td> <td> 24007 </td> </tr> </table> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a5df7d..5a50fa1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -689,6 +689,101 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf, #define QEMU_DEFAULT_NBD_PORT "10809" +#define QEMU_DEFAULT_GLUSTER_PORT "24007" + +/* builds the hosts array */ +static virJSONValuePtr +qemuBuildGlusterDriveJSONHosts(virStorageSourcePtr src) +{ + virJSONValuePtr servers = NULL; + virJSONValuePtr server = NULL; + virJSONValuePtr ret = NULL; + virStorageNetHostDefPtr host; + const char *transport; + const char *portstr; + size_t i; + + if (!(servers = virJSONValueNewArray())) + goto cleanup; + + for (i = 0; i < src->nhosts; i++) { + host = src->hosts + i; + transport = virStorageNetHostTransportTypeToString(host->transport); + portstr = host->port; + + if (virJSONValueObjectCreate(&server, "s:type", transport, NULL) < 0) + goto cleanup; + + if (!portstr) + portstr = QEMU_DEFAULT_GLUSTER_PORT; + + switch ((virStorageNetHostTransport) host->transport) { + case VIR_STORAGE_NET_HOST_TRANS_TCP: + if (virJSONValueObjectAdd(server, + "s:host", host->name, + "s:port", portstr, + NULL) < 0) + goto cleanup; + break; + + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + if (virJSONValueObjectAdd(server, + "s:socket", host->socket, + NULL) < 0) + goto cleanup; + break; + + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("transport protocol '%s' is not yet supported"), + transport); + goto cleanup; + } + + if (virJSONValueArrayAppend(servers, server) < 0) + goto cleanup; + + server = NULL; + } + + ret = servers; + servers = NULL; + + cleanup: + virJSONValueFree(servers); + virJSONValueFree(server); + + return ret; +} + + +static virJSONValuePtr +qemuBuildGlusterDriveJSON(virStorageSourcePtr src) +{ + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + virJSONValuePtr servers = NULL; + virJSONValuePtr ret = NULL; + + if (!(servers = qemuBuildGlusterDriveJSONHosts(src))) + return NULL; + + /* { driver:"gluster", + * volume:"testvol", + * path:"/a.img", + * server :[{type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, ...], + * driver:"qcow2" } + */ + if (virJSONValueObjectCreate(&ret, + "s:driver", protocol, + "s:volume", src->volume, + "s:path", src->path, + "a:server", servers, NULL) < 0) + virJSONValueFree(servers); + + return ret; +} static char * @@ -932,7 +1027,14 @@ qemuGetDriveSourceProps(virStorageSourcePtr src, case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: + break; + case VIR_STORAGE_TYPE_NETWORK: + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && + src->nhosts > 1) { + if (!(fileprops = qemuBuildGlusterDriveJSON(src)) < 0) + return -1; + } break; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args index 5668f15..634ed75 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args @@ -24,4 +24,11 @@ id=virtio-disk0 \ -drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,format=raw,\ if=none,id=drive-virtio-disk1' \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\ -id=virtio-disk1 +id=virtio-disk1 \ +-drive file.driver=gluster,file.volume=Volume3,file.path=/Image.qcow2,\ +file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\ +file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\ +file.server.2.type=unix,file.server.2.socket=/path/to/sock,format=qcow2,\ +if=none,id=drive-virtio-disk2 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\ +id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml index 0c66e7f..ef30e8c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml @@ -28,6 +28,15 @@ </source> <target dev='vdb' bus='virtio'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume3/Image.qcow2'> + <host name='example.org' port='6000'/> + <host name='example.org'/> + <host transport='unix' socket='/path/to/sock'/> + </source> + <target dev='vdc' bus='virtio'/> + </disk> <controller type='usb' index='0'/> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml index 160fd9d..8e0add5 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml @@ -30,6 +30,16 @@ <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume3/Image.qcow2'> + <host name='example.org' port='6000'/> + <host name='example.org'/> + <host transport='unix' socket='/path/to/sock'/> + </source> + <target dev='vdc' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.9.0
On Mon, Jul 25, 2016 at 11:42 PM, Peter Krempa <pkrempa@redhat.com> wrote:
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
To allow using failover with gluster it's necessary to specify multiple volume hosts. Add support for starting qemu with such configurations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.html.in | 2 +- src/qemu/qemu_command.c | 102 +++++++++++++++++++++ .../qemuxml2argv-disk-drive-network-gluster.args | 9 +- .../qemuxml2argv-disk-drive-network-gluster.xml | 9 ++ .../qemuxml2xmlout-disk-drive-network-gluster.xml | 10 ++ 5 files changed, 130 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 59a8bb9..8efd6af 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2280,7 +2280,7 @@ <tr> <td> gluster </td> <td> a server running glusterd daemon </td> - <td> only one </td> + <td> one or more (<span class="since">Since 2.1.0</span>), just one prior to that </td> <td> 24007 </td> </tr> </table> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a5df7d..5a50fa1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -689,6 +689,101 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
#define QEMU_DEFAULT_NBD_PORT "10809" +#define QEMU_DEFAULT_GLUSTER_PORT "24007" + +/* builds the hosts array */ +static virJSONValuePtr +qemuBuildGlusterDriveJSONHosts(virStorageSourcePtr src) +{ + virJSONValuePtr servers = NULL; + virJSONValuePtr server = NULL; + virJSONValuePtr ret = NULL; + virStorageNetHostDefPtr host; + const char *transport; + const char *portstr; + size_t i; + + if (!(servers = virJSONValueNewArray())) + goto cleanup; + + for (i = 0; i < src->nhosts; i++) { + host = src->hosts + i; + transport = virStorageNetHostTransportTypeToString(host->transport); + portstr = host->port; + + if (virJSONValueObjectCreate(&server, "s:type", transport, NULL) < 0) + goto cleanup; + + if (!portstr) + portstr = QEMU_DEFAULT_GLUSTER_PORT; + + switch ((virStorageNetHostTransport) host->transport) { + case VIR_STORAGE_NET_HOST_TRANS_TCP: + if (virJSONValueObjectAdd(server, + "s:host", host->name, + "s:port", portstr, + NULL) < 0) + goto cleanup; + break; + + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + if (virJSONValueObjectAdd(server, + "s:socket", host->socket, + NULL) < 0) + goto cleanup; + break; + + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("transport protocol '%s' is not yet supported"), + transport); + goto cleanup; + } + + if (virJSONValueArrayAppend(servers, server) < 0) + goto cleanup; + + server = NULL; + } + + ret = servers; + servers = NULL; + + cleanup: + virJSONValueFree(servers); + virJSONValueFree(server); + + return ret; +} + + +static virJSONValuePtr +qemuBuildGlusterDriveJSON(virStorageSourcePtr src) +{ + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + virJSONValuePtr servers = NULL; + virJSONValuePtr ret = NULL; + + if (!(servers = qemuBuildGlusterDriveJSONHosts(src))) + return NULL; + + /* { driver:"gluster", + * volume:"testvol", + * path:"/a.img", + * server :[{type:"tcp", host:"1.2.3.4", port:24007}, + * {type:"unix", socket:"/tmp/glusterd.socket"}, ...], + * driver:"qcow2" } + */ + if (virJSONValueObjectCreate(&ret, + "s:driver", protocol, + "s:volume", src->volume, + "s:path", src->path, + "a:server", servers, NULL) < 0) + virJSONValueFree(servers); + + return ret; +}
static char * @@ -932,7 +1027,14 @@ qemuGetDriveSourceProps(virStorageSourcePtr src, case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: + break; + case VIR_STORAGE_TYPE_NETWORK: + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && + src->nhosts > 1) { + if (!(fileprops = qemuBuildGlusterDriveJSON(src)) < 0)
ouch! comparing boolean result with constant 0 -- Prasanna
+ return -1; + } break; }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args index 5668f15..634ed75 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args @@ -24,4 +24,11 @@ id=virtio-disk0 \ -drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,format=raw,\ if=none,id=drive-virtio-disk1' \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\ -id=virtio-disk1 +id=virtio-disk1 \ +-drive file.driver=gluster,file.volume=Volume3,file.path=/Image.qcow2,\ +file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\ +file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\ +file.server.2.type=unix,file.server.2.socket=/path/to/sock,format=qcow2,\ +if=none,id=drive-virtio-disk2 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\ +id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml index 0c66e7f..ef30e8c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml @@ -28,6 +28,15 @@ </source> <target dev='vdb' bus='virtio'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume3/Image.qcow2'> + <host name='example.org' port='6000'/> + <host name='example.org'/> + <host transport='unix' socket='/path/to/sock'/> + </source> + <target dev='vdc' bus='virtio'/> + </disk> <controller type='usb' index='0'/> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml index 160fd9d..8e0add5 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-gluster.xml @@ -30,6 +30,16 @@ <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume3/Image.qcow2'> + <host name='example.org' port='6000'/> + <host name='example.org'/> + <host transport='unix' socket='/path/to/sock'/> + </source> + <target dev='vdc' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.9.0
On Tue, Jul 26, 2016 at 17:21:37 +0530, Prasanna Kalever wrote:
On Mon, Jul 25, 2016 at 11:42 PM, Peter Krempa <pkrempa@redhat.com> wrote:
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
To allow using failover with gluster it's necessary to specify multiple volume hosts. Add support for starting qemu with such configurations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.html.in | 2 +- src/qemu/qemu_command.c | 102 +++++++++++++++++++++ .../qemuxml2argv-disk-drive-network-gluster.args | 9 +- .../qemuxml2argv-disk-drive-network-gluster.xml | 9 ++ .../qemuxml2xmlout-disk-drive-network-gluster.xml | 10 ++ 5 files changed, 130 insertions(+), 2 deletions(-)
[...]
@@ -932,7 +1027,14 @@ qemuGetDriveSourceProps(virStorageSourcePtr src, case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: + break; + case VIR_STORAGE_TYPE_NETWORK: + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && + src->nhosts > 1) { + if (!(fileprops = qemuBuildGlusterDriveJSON(src)) < 0)
ouch! comparing boolean result with constant 0
I've fixed it on my local branch. Thanks.
-- Prasanna
On Mon, Jul 25, 2016 at 08:11:45PM +0200, Peter Krempa wrote:
This is a updated take based on stuff I had laying around and parts from https://www.redhat.com/archives/libvir-list/2016-July/msg00872.html
This addresses the backing store parser, adds and improves bits to the JSON->commandline generator prior to plugging in the gluster support.
This series does not yet address block jobs (snapshot/block copy) using multi-host gluster volumes.
Peter Krempa (24): tests: qemuxml2xml: Avoid crash when processing an XML that fails to parse tests: Add testing of backing store string parser util: storage: Add parser for qemu's "json" backing pseudo-protocol util: storage: Add support for host device backing specified via JSON util: storage: Add support for URI based backing volumes in qemu's JSON pseudo-protocol util: storage: Add json pseudo protocol support for gluster volumes util: storage: Add json pseudo protocol support for iSCSI volumes util: storage: Add JSON backing volume parser for 'nbd' protocol util: storage: Add JSON backing store parser for 'sheepdog' protocol util: storage: Add 'ssh' network storage protocol util: storage: Add JSON backing volume parser for 'ssh' protocol util: json: Make first argument of virJSONValueObjectForeachKeyValue const util: qemu: Add wrapper for JSON -> commandline conversion util: qemu: Add support for user-passed strings in JSON->commandline util: qemu: Allow nested objects in JSON -> commandline generator util: qemu: Allow for different approaches to format JSON arrays util: qemu: Don't generate any extra commas in virQEMUBuildCommandLineJSON qemu: command: Rename qemuBuildNetworkDriveURI to qemuBuildNetworkDriveStr qemu: command: Split out network disk URI building qemu: command: Extract drive source command line formatter qemu: command: Refactor code extracted to qemuBuildDriveSourceStr storage: gluster: Support multiple hosts in backend functions util: qemu: Add support for numbered array members qemu: command: Add infrastructure for object specified disk sources
Prasanna Kumar Kalever (1): qemu: command: Add support for multi-host gluster disks
docs/formatdomain.html.in | 2 +- src/libvirt_private.syms | 5 + src/libxl/libxl_conf.c | 1 + src/qemu/qemu_command.c | 428 +++++++++++++++------ src/qemu/qemu_driver.c | 3 + src/qemu/qemu_parse_command.c | 1 + src/storage/storage_backend_gluster.c | 82 ++-- src/util/virjson.c | 2 +- src/util/virjson.h | 2 +- src/util/virqemu.c | 219 +++++++++-- src/util/virqemu.h | 16 + src/util/virstoragefile.c | 375 +++++++++++++++++- src/util/virstoragefile.h | 4 + src/xenconfig/xen_xl.c | 1 + tests/qemucommandutiltest.c | 68 +++- .../qemuxml2argv-disk-drive-network-gluster.args | 9 +- .../qemuxml2argv-disk-drive-network-gluster.xml | 9 + .../qemuxml2xmlout-disk-drive-network-gluster.xml | 10 + tests/qemuxml2xmltest.c | 9 +- tests/virstoragetest.c | 150 ++++++++ 20 files changed, 1169 insertions(+), 227 deletions(-)
ACK series Jan
On Tue, Jul 26, 2016 at 17:57:32 +0200, Ján Tomko wrote:
On Mon, Jul 25, 2016 at 08:11:45PM +0200, Peter Krempa wrote:
This is a updated take based on stuff I had laying around and parts from https://www.redhat.com/archives/libvir-list/2016-July/msg00872.html
This addresses the backing store parser, adds and improves bits to the JSON->commandline generator prior to plugging in the gluster support.
This series does not yet address block jobs (snapshot/block copy) using multi-host gluster volumes.
Peter Krempa (24): tests: qemuxml2xml: Avoid crash when processing an XML that fails to parse tests: Add testing of backing store string parser util: storage: Add parser for qemu's "json" backing pseudo-protocol util: storage: Add support for host device backing specified via JSON util: storage: Add support for URI based backing volumes in qemu's JSON pseudo-protocol util: storage: Add json pseudo protocol support for gluster volumes util: storage: Add json pseudo protocol support for iSCSI volumes util: storage: Add JSON backing volume parser for 'nbd' protocol util: storage: Add JSON backing store parser for 'sheepdog' protocol util: storage: Add 'ssh' network storage protocol util: storage: Add JSON backing volume parser for 'ssh' protocol util: json: Make first argument of virJSONValueObjectForeachKeyValue const util: qemu: Add wrapper for JSON -> commandline conversion util: qemu: Add support for user-passed strings in JSON->commandline util: qemu: Allow nested objects in JSON -> commandline generator util: qemu: Allow for different approaches to format JSON arrays util: qemu: Don't generate any extra commas in virQEMUBuildCommandLineJSON qemu: command: Rename qemuBuildNetworkDriveURI to qemuBuildNetworkDriveStr qemu: command: Split out network disk URI building qemu: command: Extract drive source command line formatter qemu: command: Refactor code extracted to qemuBuildDriveSourceStr storage: gluster: Support multiple hosts in backend functions util: qemu: Add support for numbered array members qemu: command: Add infrastructure for object specified disk sources
Prasanna Kumar Kalever (1): qemu: command: Add support for multi-host gluster disksa
[...]
ACK series
Thanks for the review. I've pushed this now since the freeze didn't happen yet along with the updated JSON part as per review. Peter
participants (3)
-
Ján Tomko -
Peter Krempa -
Prasanna Kalever