[libvirt] [PATCH 00/12] qemu: Fully populate JSON formatters for the protocol layer (blockdev-add saga)
This adds the missing formatters for JSON properties for the storage. John Ferlan (1): qemu: block: Add JSON props generator for iSCSI protocol This patch was stolen from the iSCSI saga and fixed, since the formatter did not format the port number into the portal string. Peter Krempa (11): qemu: block: Use proper type for servers for VxHS disks qemu: process: Split out useful parts from qemuBuildNetworkDriveURI storage: Don't store leading '/' in image name when splitting out volume storage: Store RBD image name as pool and image name qemu: block: Add JSON props generator for 'curl' based storage backends qemu: block: Add JSON props generator for NBD storage backing qemu: block: Add JSON props generator for RBD storage backing qemu: block: Add JSON props generator for sheepdog storage backing qemu: block: Add JSON props generator for ssh storage backing qemu: block: Add node-names to JSON backing storage strings tests: Add testing of storage backend JSON props formatter src/conf/domain_conf.c | 17 +- src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_block.c | 403 ++++++++++++++++++++- src/qemu/qemu_block.h | 4 + src/qemu/qemu_command.c | 40 +- src/util/virstoragefile.c | 26 +- src/xenconfig/xen_xl.c | 2 +- tests/Makefile.am | 14 +- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 2 +- tests/qemublocktest.c | 189 ++++++++++ .../qemuxml2argv-disk-drive-network-gluster.args | 2 +- ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 8 +- .../qemuxml2argv-disk-drive-network-vxhs.args | 4 +- tests/virstoragetest.c | 3 +- 15 files changed, 640 insertions(+), 78 deletions(-) create mode 100644 tests/qemublocktest.c -- 2.14.3
Original implementation used 'SocketAddress' equivalent from qemu for the disk server field, while qemu documentation specifies 'InetSocketAddress'. The backing store parser uses the correct parsing function but the formatter used the incorrect one (and also with the legacy mode enabled which was wrong). --- src/qemu/qemu_block.c | 36 +++++++++++++++++++++- ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 8 ++--- .../qemuxml2argv-disk-drive-network-vxhs.args | 4 +-- tests/virstoragetest.c | 3 +- 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 0761f8991..96db19226 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -501,6 +501,40 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, } +/** + * qemuBlockStorageSourceBuildJSONInetSocketAddress + * @host: the virStorageNetHostDefPtr definition to build + * + * Formats @hosts into a json object conforming to the 'InetSocketAddress' type + * in qemu. + * + * Returns a virJSONValuePtr for a single server. + */ +static virJSONValuePtr +qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) +{ + virJSONValuePtr ret = NULL; + char *port = NULL; + + if (host->transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only TCP protocol can be converted to InetSocketAddress")); + return NULL; + } + + if (virAsprintf(&port, "%u", host->port) < 0) + return NULL; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:host", host->name, + "s:port", port, + NULL)); + + VIR_FREE(port); + return ret; +} + + static virJSONValuePtr qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) { @@ -540,7 +574,7 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) return NULL; } - if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true))) + if (!(server = qemuBlockStorageSourceBuildJSONInetSocketAddress(&src->hosts[0]))) return NULL; /* VxHS disk specification example: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args index a75272454..eaa8699a9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args @@ -23,7 +23,7 @@ server,nowait \ -object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/libvirt-vxhs,\ endpoint=client,verify-peer=yes \ -drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\ -file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,file.server.type=tcp,\ +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\ file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\ id=drive-virtio-disk0,cache=none \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ @@ -31,13 +31,13 @@ id=virtio-disk0 \ -object tls-creds-x509,id=objvirtio-disk1_tls0,dir=/etc/pki/libvirt-vxhs,\ endpoint=client,verify-peer=yes \ -drive file.driver=vxhs,file.tls-creds=objvirtio-disk1_tls0,\ -file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,file.server.type=tcp,\ +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\ file.server.host=192.168.0.2,file.server.port=9999,format=raw,if=none,\ id=drive-virtio-disk1,cache=none \ -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ id=virtio-disk1 \ -drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc253,\ -file.server.type=tcp,file.server.host=192.168.0.3,file.server.port=9999,\ -format=raw,if=none,id=drive-virtio-disk2,cache=none \ +file.server.host=192.168.0.3,file.server.port=9999,format=raw,if=none,\ +id=drive-virtio-disk2,cache=none \ -device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args index b62ace3de..1747dc80f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args @@ -21,7 +21,7 @@ server,nowait \ -boot c \ -usb \ -drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\ -file.server.type=tcp,file.server.host=192.168.0.1,file.server.port=9999,\ -format=raw,if=none,id=drive-virtio-disk0,cache=none \ +file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\ +id=drive-virtio-disk0,cache=none \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ id=virtio-disk0 diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index e1d875172..78f24ad78 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1601,8 +1601,7 @@ mymain(void) "</source>\n"); TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"vxhs\"," "\"vdisk-id\":\"c6718f6b-0401-441d-a8c3-1f0064d75ee0\"," - "\"server\": { \"type\":\"tcp\"," - "\"host\":\"example.com\"," + "\"server\": { \"host\":\"example.com\"," "\"port\":\"9999\"" "}" "}" -- 2.14.3
Extract the part formatting the basic URI part so that it can be reused to format JSON backing definitions. Parts specific to the command line format will remain in qemuBuildNetworkDriveURI. The new function is called qemuBlockStorageSourceGetURI. --- src/qemu/qemu_block.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 4 ++++ src/qemu/qemu_command.c | 38 +------------------------------- 3 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 96db19226..018d7c7ec 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -390,6 +390,64 @@ qemuBlockGetNodeData(virJSONValuePtr data) } +/** + * qemuBlockStorageSourceGetURI: + * @src: disk storage source + * + * Formats a URI from a virStorageSource */ +virURIPtr +qemuBlockStorageSourceGetURI(virStorageSourcePtr src) +{ + virURIPtr uri = NULL; + virURIPtr 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) { + uri->port = src->hosts->port; + + 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 (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 (VIR_STRDUP(uri->server, src->hosts->name) < 0) + goto cleanup; + + VIR_STEAL_PTR(ret, uri); + + cleanup: + virURIFree(uri); + return ret; +} + + /** * qemuBlockStorageSourceBuildJSONSocketAddress * @host: the virStorageNetHostDefPtr definition to build diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index f0a2c9aa7..b9ee97f48 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -26,6 +26,7 @@ # include "virhash.h" # include "virjson.h" +# include "viruri.h" typedef struct qemuBlockNodeNameBackingChainData qemuBlockNodeNameBackingChainData; typedef qemuBlockNodeNameBackingChainData *qemuBlockNodeNameBackingChainDataPtr; @@ -56,4 +57,7 @@ qemuBlockGetNodeData(virJSONValuePtr data); virJSONValuePtr qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src); +virURIPtr +qemuBlockStorageSourceGetURI(virStorageSourcePtr src); + #endif /* __QEMU_BLOCK_H__ */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b1cfafa79..25d5fdf18 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -829,41 +829,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, virURIPtr uri = NULL; char *ret = NULL; - if (src->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("protocol '%s' accepts only one host"), - virStorageNetProtocolTypeToString(src->protocol)); + if (!(uri = qemuBlockStorageSourceGetURI(src))) goto cleanup; - } - - if (VIR_ALLOC(uri) < 0) - goto cleanup; - - if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { - uri->port = src->hosts->port; - - 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 (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) @@ -872,9 +839,6 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) goto cleanup; - if (VIR_STRDUP(uri->server, src->hosts->name) < 0) - goto cleanup; - ret = virURIFormat(uri); cleanup: -- 2.14.3
On Fri, Nov 03, 2017 at 03:29:19PM +0100, Peter Krempa wrote:
--- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -390,6 +390,64 @@ qemuBlockGetNodeData(virJSONValuePtr data) }
+/** + * qemuBlockStorageSourceGetURI: + * @src: disk storage source + * + * Formats a URI from a virStorageSource */
Please put the */ on a separate line.
+virURIPtr +qemuBlockStorageSourceGetURI(virStorageSourcePtr src) +{
Jan
Libvirt historically stores storage source path including the volume as one string in the XML, but that is not really flexible enough when dealing with the fields in the code. Previously we'd store the slash separating the two as part of the image name. This was fine for gluster but it's not necessary and does not scale well when converting other protocols. Don't store the slash as part of the path. The resulting change from absolute to relative path within the gluster driver should be okay, as the root directory is the default when accessing gluster. --- src/conf/domain_conf.c | 4 ++-- src/qemu/qemu_block.c | 2 +- src/util/virstoragefile.c | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 77c20c697..01cd3811c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8398,7 +8398,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, src->volume = src->path; - if (VIR_STRDUP(src->path, tmp) < 0) + if (VIR_STRDUP(src->path, tmp + 1) < 0) goto cleanup; tmp[0] = '\0'; @@ -22158,7 +22158,7 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virStorageNetProtocolTypeToString(src->protocol)); if (src->volume) { - if (virAsprintf(&path, "%s%s", src->volume, src->path) < 0) + if (virAsprintf(&path, "%s/%s", src->volume, src->path) < 0) return -1; } diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 018d7c7ec..08020d797 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -426,7 +426,7 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src) if (src->path) { if (src->volume) { - if (virAsprintf(&uri->path, "/%s%s", + if (virAsprintf(&uri->path, "/%s/%s", src->volume, src->path) < 0) goto cleanup; } else { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3a2d2aa05..d48358abb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2450,7 +2450,7 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src, src->volume = src->path; - if (VIR_STRDUP(src->path, tmp) < 0) + if (VIR_STRDUP(src->path, tmp + 1) < 0) goto cleanup; tmp[0] = '\0'; @@ -2931,7 +2931,7 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src, src->protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER; if (VIR_STRDUP(src->volume, volume) < 0 || - virAsprintf(&src->path, "/%s", path) < 0) + VIR_STRDUP(src->path, path) < 0) return -1; nservers = virJSONValueArraySize(server); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args index 990616955..6cfbe36fe 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args @@ -27,7 +27,7 @@ id=virtio-disk0 \ format=raw,if=none,id=drive-virtio-disk1' \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\ id=virtio-disk1 \ --drive file.driver=gluster,file.volume=Volume3,file.path=/Image.qcow2,\ +-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,file.debug=4,\ -- 2.14.3
Similarly to how we store gluster names, split the name into a pool and image portions when paring the XML and store them separately. --- src/conf/domain_conf.c | 13 +++++++------ src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_command.c | 2 +- src/util/virstoragefile.c | 22 +++++++++++----------- src/xenconfig/xen_xl.c | 2 +- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 2 +- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01cd3811c..a7fe1ac85 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8383,16 +8383,17 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, src->tlsFromConfig = !!tlsCfgVal; } - /* for historical reasons the volume name for gluster volume is stored - * as a part of the path. This is hard to work with when dealing with - * relative names. Split out the volume into a separate variable */ - if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { + /* for historical reasons we store the volume and image name in one XML + * element although it complicates thing when attempting to access them. */ + if (src->path && + (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER || + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { char *tmp; if (!(tmp = strchr(src->path, '/')) || tmp == src->path) { virReportError(VIR_ERR_XML_ERROR, - _("missing volume name or file name in " - "gluster source path '%s'"), src->path); + _("can't split path '%s' into pool name and image " + "name"), src->path); goto cleanup; } diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index ecbabfc79..63397e94c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -657,7 +657,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, goto cleanup; } - virBufferStrcat(&buf, "rbd:", src->path, NULL); + virBufferStrcat(&buf, "rbd:", src->volume, "/", src->path, NULL); if (username) { virBufferEscape(&buf, '\\', ":", ":id=%s", username); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25d5fdf18..0dcdc8976 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -951,7 +951,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, goto cleanup; } - virBufferStrcat(&buf, "rbd:", src->path, NULL); + virBufferStrcat(&buf, "rbd:", src->volume, "/", src->path, NULL); if (src->snapshot) virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d48358abb..9cee64312 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2545,6 +2545,14 @@ virStorageSourceParseRBDColonString(const char *rbdstr, *p = '\0'; } + /* pool vs. image name */ + if ((p = strchr(src->path, '/'))) { + VIR_STEAL_PTR(src->volume, src->path); + if (VIR_STRDUP(src->path, p + 1) < 0) + goto error; + *p = '\0'; + } + /* options */ if (!options) return 0; /* all done */ @@ -3178,7 +3186,6 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, const char *conf = virJSONValueObjectGetString(json, "conf"); const char *snapshot = virJSONValueObjectGetString(json, "snapshot"); virJSONValuePtr servers = virJSONValueObjectGetArray(json, "server"); - char *fullname = NULL; size_t nservers; size_t i; int ret = -1; @@ -3197,17 +3204,12 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, return -1; } - /* currently we need to store the pool name and image name together, since - * the rest of the code is not prepared for it */ - if (virAsprintf(&fullname, "%s/%s", pool, image) < 0) - return -1; - - if (VIR_STRDUP(src->snapshot, snapshot) < 0 || + if (VIR_STRDUP(src->volume, pool) < 0 || + VIR_STRDUP(src->path, image) < 0 || + VIR_STRDUP(src->snapshot, snapshot) < 0 || VIR_STRDUP(src->configFile, conf) < 0) goto cleanup; - VIR_STEAL_PTR(src->path, fullname); - if (servers) { nservers = virJSONValueArraySize(servers); @@ -3225,8 +3227,6 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, ret = 0; cleanup: - VIR_FREE(fullname); - return ret; } diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 8acbfe3f6..a61f1d7d5 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -1040,7 +1040,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src) goto cleanup; } - virBufferStrcat(&buf, "rbd:", src->path, NULL); + virBufferStrcat(&buf, "rbd:", src->volume, "/", src->path, NULL); virBufferAddLit(&buf, ":auth_supported=none"); diff --git a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml index aa1522a45..cf5ea0814 100644 --- a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml @@ -24,7 +24,7 @@ </source> </disk> <disk name='hdh' snapshot='external' type='network'> - <source protocol='rbd' name='name'> + <source protocol='rbd' name='vol/name'> <host name='host' port='1234'/> <host name='host2' port='1234' transport='rdma'/> <host name='host3' port='1234'/> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index c2e77d7ac..0e81f35c6 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -23,7 +23,7 @@ </source> </disk> <disk name='hdh' snapshot='external' type='network'> - <source protocol='rbd' name='name'> + <source protocol='rbd' name='vol/name'> <host name='host' port='1234'/> <host name='host2' port='1234' transport='rdma'/> <host name='host3' port='1234'/> -- 2.14.3
QEMU uses curl for accessing files using http(s) and ftp(s). They share common options so let's generate them in one helper. --- src/qemu/qemu_block.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 08020d797..dedb92fd5 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -652,6 +652,57 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + const char *passwordalias = NULL; + const char *username = NULL; + virJSONValuePtr ret = NULL; + virURIPtr uri = NULL; + char *uristr = NULL; + const char *driver; + + /** + * Common options: + * url, readahead, timeout, username, password-secret, proxy-username, + * proxy-password secret + * + * Options for http transport: + * cookie, cookie-secret + * + * Options for secure transport (ftps, https): + * sslverify + */ + + driver = virStorageNetProtocolTypeToString(src->protocol); + + if (!(uri = qemuBlockStorageSourceGetURI(src))) + goto cleanup; + + if (!(uristr = virURIFormat(uri))) + goto cleanup; + + if (src->auth) { + username = src->auth->username; + passwordalias = srcPriv->secinfo->s.aes.alias; + } + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", driver, + "s:url", uristr, + "S:username", username, + "S:password-secret", passwordalias, + NULL)); + + cleanup: + virURIFree(uri); + VIR_FREE(uristr); + + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -692,15 +743,19 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) return NULL; break; - case VIR_STORAGE_NET_PROTOCOL_NBD: - case VIR_STORAGE_NET_PROTOCOL_RBD: - case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_HTTP: case VIR_STORAGE_NET_PROTOCOL_HTTPS: case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + if (!(fileprops = qemuBlockStorageSourceGetCURLProps(src))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: -- 2.14.3
On Fri, Nov 03, 2017 at 03:29:22PM +0100, Peter Krempa wrote:
--- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -652,6 +652,57 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) }
+static virJSONValuePtr +qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + const char *passwordalias = NULL; + const char *username = NULL; + virJSONValuePtr ret = NULL; + virURIPtr uri = NULL; + char *uristr = NULL; + const char *driver; + + /** + * Common options: + * url, readahead, timeout, username, password-secret, proxy-username, + * proxy-password secret + *
proxy-password-secret Jan
+ * Options for http transport: + * cookie, cookie-secret + *
From: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_block.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index dedb92fd5..4e588c724 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -703,6 +703,70 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + char *target = NULL; + char *lunStr = NULL; + char *username = NULL; + char *objalias = NULL; + char *portal = NULL; + unsigned int lun = 0; + virJSONValuePtr ret = NULL; + + /* { driver:"iscsi", + * transport:"tcp", ("iser" also possible) + * portal:"example.com", + * target:"iqn.2017-04.com.example:iscsi-disks", + * lun:1, + * user:"username", + * password-secret:"secret-alias", + * } + */ + + if (VIR_STRDUP(target, src->path) < 0) + goto cleanup; + + /* Separate the target and lun */ + if ((lunStr = strchr(target, '/'))) { + *(lunStr++) = '\0'; + if (virStrToLong_ui(lunStr, NULL, 10, &lun) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse target for lunStr '%s'"), + target); + goto cleanup; + } + } + + /* combine host and port into portal */ + if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + + if (src->auth) { + username = src->auth->username; + objalias = srcPriv->secinfo->s.aes.alias; + } + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", protocol, + "s:portal", portal, + "s:target", target, + "u:lun", lun, + "s:transport", "tcp", + "S:user", username, + "S:password-secret", objalias, + NULL)); + goto cleanup; + + cleanup: + VIR_FREE(target); + VIR_FREE(portal); + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -752,10 +816,14 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) return NULL; break; + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + if (!(fileprops = qemuBlockStorageSourceGetISCSIProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: -- 2.14.3
On Fri, Nov 03, 2017 at 03:29:23PM +0100, Peter Krempa wrote:
From: John Ferlan <jferlan@redhat.com>
--- +static virJSONValuePtr +qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) +{
[...]
+ if (VIR_STRDUP(target, src->path) < 0) + goto cleanup; + + /* Separate the target and lun */ + if ((lunStr = strchr(target, '/'))) { + *(lunStr++) = '\0'; + if (virStrToLong_ui(lunStr, NULL, 10, &lun) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse target for lunStr '%s'"), + target); + goto cleanup; + } + } + + /* combine host and port into portal */ + if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup;
Can src->hosts[0].name possibly be a literal IPv6 address? Jan
On Sun, Nov 05, 2017 at 16:03:25 +0100, Ján Tomko wrote:
On Fri, Nov 03, 2017 at 03:29:23PM +0100, Peter Krempa wrote:
From: John Ferlan <jferlan@redhat.com>
--- +static virJSONValuePtr +qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) +{
[...]
+ if (VIR_STRDUP(target, src->path) < 0) + goto cleanup; + + /* Separate the target and lun */ + if ((lunStr = strchr(target, '/'))) { + *(lunStr++) = '\0'; + if (virStrToLong_ui(lunStr, NULL, 10, &lun) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse target for lunStr '%s'"), + target); + goto cleanup; + } + } + + /* combine host and port into portal */ + if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup;
Can src->hosts[0].name possibly be a literal IPv6 address?
Yes, you are right. How about the following diff squashed in? diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index ffe2892ab..428c0b465 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -742,8 +742,15 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) } /* combine host and port into portal */ - if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) - goto cleanup; + if (virSocketAddrNumericFamily(src->hosts[0].name) == AF_INET6) { + if (virAsprintf(&portal, "[%s]:%u", + src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + } else { + if (virAsprintf(&portal, "%s:%u", + src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + } if (src->auth) { username = src->auth->username; Also our parser is buggy. I'll send patches separately.
On Tue, Nov 07, 2017 at 04:04:02PM +0100, Peter Krempa wrote:
On Sun, Nov 05, 2017 at 16:03:25 +0100, Ján Tomko wrote:
On Fri, Nov 03, 2017 at 03:29:23PM +0100, Peter Krempa wrote:
From: John Ferlan <jferlan@redhat.com>
--- +static virJSONValuePtr +qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) +{
[...]
+ if (VIR_STRDUP(target, src->path) < 0) + goto cleanup; + + /* Separate the target and lun */ + if ((lunStr = strchr(target, '/'))) { + *(lunStr++) = '\0'; + if (virStrToLong_ui(lunStr, NULL, 10, &lun) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse target for lunStr '%s'"), + target); + goto cleanup; + } + } + + /* combine host and port into portal */ + if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup;
Can src->hosts[0].name possibly be a literal IPv6 address?
Yes, you are right. How about the following diff squashed in?
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index ffe2892ab..428c0b465 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -742,8 +742,15 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) }
/* combine host and port into portal */ - if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) - goto cleanup; + if (virSocketAddrNumericFamily(src->hosts[0].name) == AF_INET6) { + if (virAsprintf(&portal, "[%s]:%u", + src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + } else { + if (virAsprintf(&portal, "%s:%u", + src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + }
if (src->auth) { username = src->auth->username;
ACK
Also our parser is buggy. I'll send patches separately.
Thanks, patches are welcome. Jan
--- src/qemu/qemu_block.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4e588c724..451d04694 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -767,6 +767,34 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) +{ + virJSONValuePtr serverprops; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nbd protocol accepts only one host")); + return NULL; + } + + serverprops = qemuBlockStorageSourceBuildJSONSocketAddress(&src->hosts[0], + false); + if (!serverprops) + return NULL; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "nbd", + "a:server", serverprops, + "S:export", src->path, + "S:tls-creds", src->tlsAlias, + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -822,6 +850,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; case VIR_STORAGE_NET_PROTOCOL_NBD: + if (!(fileprops = qemuBlockStorageSourceGetNBDProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH: -- 2.14.3
On 11/03/2017 10:29 AM, Peter Krempa wrote:
--- src/qemu/qemu_block.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4e588c724..451d04694 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -767,6 +767,34 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) }
+static virJSONValuePtr +qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) +{ + virJSONValuePtr serverprops; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nbd protocol accepts only one host")); + return NULL; + } + + serverprops = qemuBlockStorageSourceBuildJSONSocketAddress(&src->hosts[0], + false); + if (!serverprops) + return NULL; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "nbd", + "a:server", serverprops, + "S:export", src->path, + "S:tls-creds", src->tlsAlias, + NULL));
Coverity notes that serverprops will be leaked on failure Actually it REALLY doesn't like the "a:server" consumption, but I already have quite a few of those in my false positive list. John
+ + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -822,6 +850,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break;
case VIR_STORAGE_NET_PROTOCOL_NBD: + if (!(fileprops = qemuBlockStorageSourceGetNBDProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH:
--- src/qemu/qemu_block.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 451d04694..8a1ce8262 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -593,6 +593,47 @@ qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) } +/** + * qemuBlockStorageSourceBuildHostsJSONInetSocketAddress: + * @src: disk storage source + * + * Formats src->hosts into a json object conforming to the 'InetSocketAddress' + * type in qemu. + */ +static virJSONValuePtr +qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) +{ + virJSONValuePtr servers = NULL; + virJSONValuePtr server = NULL; + virJSONValuePtr ret = NULL; + virStorageNetHostDefPtr host; + size_t i; + + if (!(servers = virJSONValueNewArray())) + goto cleanup; + + for (i = 0; i < src->nhosts; i++) { + host = src->hosts + i; + + if (!(server = qemuBlockStorageSourceBuildJSONInetSocketAddress(host))) + goto cleanup; + + if (virJSONValueArrayAppend(servers, server) < 0) + goto cleanup; + + server = NULL; + } + + VIR_STEAL_PTR(ret, servers); + + cleanup: + virJSONValueFree(servers); + virJSONValueFree(server); + + return ret; +} + + static virJSONValuePtr qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) { @@ -795,6 +836,35 @@ qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + virJSONValuePtr servers = NULL; + virJSONValuePtr ret = NULL; + const char *username = NULL; + + if (src->nhosts > 0 && + !(servers = qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(src))) + return NULL; + + if (src->auth) + username = srcPriv->secinfo->s.aes.username; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "rbd", + "s:pool", src->volume, + "s:image", src->path, + "S:snapshot", src->snapshot, + "S:conf", src->configFile, + "A:server", servers, + "S:user", username, + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -855,6 +925,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; case VIR_STORAGE_NET_PROTOCOL_RBD: + if (!(fileprops = qemuBlockStorageSourceGetRBDProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_NONE: -- 2.14.3
On Fri, Nov 03, 2017 at 03:29:25PM +0100, Peter Krempa wrote:
--- src/qemu/qemu_block.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 451d04694..8a1ce8262 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -593,6 +593,47 @@ qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) +static virJSONValuePtr +qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) +{ + virJSONValuePtr servers = NULL; + virJSONValuePtr server = NULL; + virJSONValuePtr ret = NULL; + virStorageNetHostDefPtr host; + size_t i; + + if (!(servers = virJSONValueNewArray())) + goto cleanup; + + for (i = 0; i < src->nhosts; i++) { + host = src->hosts + i; + + if (!(server = qemuBlockStorageSourceBuildJSONInetSocketAddress(host))) + goto cleanup;
Indentation is off here. Jan
+ + if (virJSONValueArrayAppend(servers, server) < 0) + goto cleanup;
--- src/qemu/qemu_block.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8a1ce8262..5f28c4dd6 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -865,6 +865,33 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) +{ + virJSONValuePtr serverprops; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("sheepdog protocol accepts only one host")); + return NULL; + } + + serverprops = qemuBlockStorageSourceBuildJSONSocketAddress(&src->hosts[0], + false); + if (!serverprops) + return NULL; + + /* libvirt does not support the 'snap-id' and 'tag' properties */ + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "sheepdog", + "a:server", serverprops, + "s:vdi", src->path, + NULL)); + + return ret; +} + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -930,6 +957,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + if (!(fileprops = qemuBlockStorageSourceGetSheepdogProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: -- 2.14.3
On 11/03/2017 10:29 AM, Peter Krempa wrote:
--- src/qemu/qemu_block.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8a1ce8262..5f28c4dd6 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -865,6 +865,33 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) }
+static virJSONValuePtr +qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) +{ + virJSONValuePtr serverprops; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("sheepdog protocol accepts only one host")); + return NULL; + } + + serverprops = qemuBlockStorageSourceBuildJSONSocketAddress(&src->hosts[0], + false); + if (!serverprops) + return NULL; + + /* libvirt does not support the 'snap-id' and 'tag' properties */ + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "sheepdog", + "a:server", serverprops, + "s:vdi", src->path, + NULL));
Again Coverity here w/ serverprops being leaked on failure. John
+ + return ret; +} + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -930,6 +957,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break;
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + if (!(fileprops = qemuBlockStorageSourceGetSheepdogProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST:
--- src/qemu/qemu_block.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 5f28c4dd6..6f6d294bf 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -892,6 +892,38 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) return ret; } + +static virJSONValuePtr +qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) +{ + virJSONValuePtr serverprops; + virJSONValuePtr ret = NULL; + const char *username = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("sheepdog protocol accepts only one host")); + return NULL; + } + + serverprops = qemuBlockStorageSourceBuildJSONInetSocketAddress(&src->hosts[0]); + if (!serverprops) + return NULL; + + if (src->auth) + username = src->auth->username; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "ssh", + "s:path", src->path, + "a:server", serverprops, + "S:user", username, + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -962,6 +994,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; case VIR_STORAGE_NET_PROTOCOL_SSH: + if (!(fileprops = qemuBlockStorageSourceGetSshProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: break; -- 2.14.3
Format out the node-name if it was assigned for JSON-based storage specification. --- src/qemu/qemu_block.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6f6d294bf..6df0dc0fb 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1005,5 +1005,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; } + if (virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) { + virJSONValueFree(fileprops); + return NULL; + } + return fileprops; } -- 2.14.3
On 11/03/2017 10:29 AM, Peter Krempa wrote:
Format out the node-name if it was assigned for JSON-based storage specification. --- src/qemu/qemu_block.c | 5 +++++ 1 file changed, 5 insertions(+)
And Coverity also notes that for virStorageType) actualType == VIR_STORAGE_TYPE_VOLUME ((NONE and LAST, too), that fileprops will be NULL and thus the virJSONValueObjectAdd won't be happy... John
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6f6d294bf..6df0dc0fb 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1005,5 +1005,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; }
+ if (virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) { + virJSONValueFree(fileprops); + return NULL; + } + return fileprops; }
Add a new test program called 'qemublocktest' to test the block layer related stuff and test storage source to JSON generator by comparing it to the JSON parser. --- tests/Makefile.am | 14 +++- tests/qemublocktest.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 tests/qemublocktest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 0b2305d70..939acb0a1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -282,7 +282,9 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ qemuagenttest qemucapabilitiestest qemucaps2xmltest \ qemumemlocktest \ - qemucommandutiltest + qemucommandutiltest \ + qemublocktest \ + $(NULL) test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ libqemutestdriver.la \ @@ -668,6 +670,15 @@ qemuhotplugtest_SOURCES = \ $(NULL) qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) +qemublocktest_SOURCES = \ + qemublocktest.c testutils.h testutils.c +qemublocktest_LDADD = $(LDADDS) \ + ../src/libvirt_conf.la \ + ../src/libvirt_util.la \ + $(qemu_LDADDS) \ + ../gnulib/lib/libgnu.la \ + $(NULL) + domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h @@ -686,6 +697,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuagenttest.c qemucapabilitiestest.c \ qemucaps2xmltest.c qemucommandutiltest.c \ qemumemlocktest.c qemucpumock.c testutilshostcpus.h \ + qemublocktest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif ! WITH_QEMU diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c new file mode 100644 index 000000000..52ecbde36 --- /dev/null +++ b/tests/qemublocktest.c @@ -0,0 +1,189 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include <stdlib.h> + +#include "testutils.h" +#include "virstoragefile.h" +#include "virstring.h" +#include "virlog.h" +#include "qemu/qemu_block.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("tests.storagetest"); + +struct testBackingXMLjsonXMLdata { + int type; + const char *xml; +}; + +static int +testBackingXMLjsonXML(const void *args) +{ + const struct testBackingXMLjsonXMLdata *data = args; + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virStorageSourcePtr xmlsrc = NULL; + virStorageSourcePtr jsonsrc = NULL; + virJSONValuePtr backendprops = NULL; + virJSONValuePtr wrapper = NULL; + char *propsstr = NULL; + char *protocolwrapper = NULL; + char *actualxml = NULL; + int ret = -1; + + if (VIR_ALLOC(xmlsrc) < 0) + return -1; + + xmlsrc->type = data->type; + + if (!(xml = virXMLParseStringCtxt(data->xml, "(test storage source XML)", &ctxt))) + goto cleanup; + + if (virDomainDiskSourceParse(ctxt->node, ctxt, xmlsrc, 0) < 0) { + fprintf(stderr, "failed to parse disk source xml\n"); + goto cleanup; + } + + if (!(backendprops = qemuBlockStorageSourceGetBackendProps(xmlsrc))) { + fprintf(stderr, "failed to format disk source json\n"); + goto cleanup; + } + + if (virJSONValueObjectCreate(&wrapper, "a:file", backendprops, NULL) < 0) + goto cleanup; + + backendprops = NULL; + + if (!(propsstr = virJSONValueToString(wrapper, false))) + goto cleanup; + + if (virAsprintf(&protocolwrapper, "json:%s", propsstr) < 0) + goto cleanup; + + if (!(jsonsrc = virStorageSourceNewFromBackingAbsolute(protocolwrapper))) { + fprintf(stderr, "failed to parse disk json\n"); + goto cleanup; + } + + if (virDomainDiskSourceFormat(&buf, jsonsrc, 0, 0) < 0 || + !(actualxml = virBufferContentAndReset(&buf))) { + fprintf(stderr, "failed to format disk source xml\n"); + goto cleanup; + } + + if (STRNEQ(actualxml, data->xml)) { + fprintf(stderr, "\n expected storage source xml:\n'%s'\n" + "actual storage source xml:\n%s\n" + "intermediate json:\n%s\n", + data->xml, actualxml, protocolwrapper); + goto cleanup; + } + + ret = 0; + + cleanup: + virStorageSourceFree(xmlsrc); + virStorageSourceFree(jsonsrc); + VIR_FREE(propsstr); + VIR_FREE(protocolwrapper); + VIR_FREE(actualxml); + virJSONValueFree(backendprops); + virJSONValueFree(wrapper); + virBufferFreeAndReset(&buf); + xmlFreeDoc(xml); + + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + struct testBackingXMLjsonXMLdata data; + + virTestCounterReset("qemu storage source xml->json->xml "); + +#define TEST_JSON_FORMAT(tpe, xmlstr) \ + do { \ + data.type = tpe; \ + data.xml = xmlstr; \ + if (virTestRun(virTestCounterNext(), testBackingXMLjsonXML, &data) < 0) \ + ret = -1; \ + } while (0) + +#define TEST_JSON_FORMAT_NET(xmlstr)\ + TEST_JSON_FORMAT(VIR_STORAGE_TYPE_NETWORK, xmlstr) + + TEST_JSON_FORMAT(VIR_STORAGE_TYPE_FILE, "<source file='/path/to/file'/>\n"); + + /* type VIR_STORAGE_TYPE_BLOCK is not tested since it parses back to 'file' */ + /* type VIR_STORAGE_TYPE_DIR it is a 'format' driver in qemu */ + + TEST_JSON_FORMAT_NET("<source protocol='http' name='file'>\n" + " <host name='example.com' port='80'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='https' name='file'>\n" + " <host name='example.com' port='432'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='gluster' name='vol/file'>\n" + " <host name='example.com' port='24007'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='gluster' name='testvol/img.qcow2'>\n" + " <host name='example.com' port='1234'/>\n" + " <host transport='unix' socket='/path/socket'/>\n" + " <host name='example.com' port='24007'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='nbd'>\n" + " <host transport='unix' socket='/path/to/socket'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='nbd' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='nbd'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='ssh' name='blah'>\n" + " <host name='example.org' port='6000'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='rbd' name='libvirt/test'>\n" + " <host name='example.com' port='1234'/>\n" + " <host name='example2.com'/>\n" + " <snapshot name='snapshotname'/>\n" + " <config file='/path/to/conf'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/0'>\n" + " <host name='test.org' port='3260'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/6'>\n" + " <host name='test.org' port='1234'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='sheepdog' name='test'>\n" + " <host name='example.com' port='321'/>\n" + "</source>\n"); + TEST_JSON_FORMAT_NET("<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n" + " <host name='example.com' port='9999'/>\n" + "</source>\n"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) -- 2.14.3
On Fri, Nov 03, 2017 at 03:29:29PM +0100, Peter Krempa wrote:
Add a new test program called 'qemublocktest' to test the block layer related stuff and test storage source to JSON generator by comparing it to the JSON parser. --- tests/Makefile.am | 14 +++- tests/qemublocktest.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 tests/qemublocktest.c
[...]
+static int +testBackingXMLjsonXML(const void *args) +{
[...]
+ + ret = 0; + + cleanup: + virStorageSourceFree(xmlsrc); + virStorageSourceFree(jsonsrc); + VIR_FREE(propsstr); + VIR_FREE(protocolwrapper); + VIR_FREE(actualxml); + virJSONValueFree(backendprops); + virJSONValueFree(wrapper); + virBufferFreeAndReset(&buf);
xmlXPathFreeContext(ctxt); Jan
+ xmlFreeDoc(xml); + + return ret; +}
On Fri, Nov 03, 2017 at 03:29:17PM +0100, Peter Krempa wrote:
This adds the missing formatters for JSON properties for the storage.
John Ferlan (1): qemu: block: Add JSON props generator for iSCSI protocol
This patch was stolen from the iSCSI saga and fixed, since the formatter did not format the port number into the portal string.
Peter Krempa (11): qemu: block: Use proper type for servers for VxHS disks qemu: process: Split out useful parts from qemuBuildNetworkDriveURI storage: Don't store leading '/' in image name when splitting out volume storage: Store RBD image name as pool and image name qemu: block: Add JSON props generator for 'curl' based storage backends qemu: block: Add JSON props generator for NBD storage backing qemu: block: Add JSON props generator for RBD storage backing qemu: block: Add JSON props generator for sheepdog storage backing qemu: block: Add JSON props generator for ssh storage backing qemu: block: Add node-names to JSON backing storage strings tests: Add testing of storage backend JSON props formatter
src/conf/domain_conf.c | 17 +- src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_block.c | 403 ++++++++++++++++++++- src/qemu/qemu_block.h | 4 + src/qemu/qemu_command.c | 40 +- src/util/virstoragefile.c | 26 +- src/xenconfig/xen_xl.c | 2 +- tests/Makefile.am | 14 +- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 2 +- tests/qemublocktest.c | 189 ++++++++++ .../qemuxml2argv-disk-drive-network-gluster.args | 2 +- ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 8 +- .../qemuxml2argv-disk-drive-network-vxhs.args | 4 +- tests/virstoragetest.c | 3 +- 15 files changed, 640 insertions(+), 78 deletions(-) create mode 100644 tests/qemublocktest.c
ACK series Jan
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa