Thank you for your patience, I regret for my first time submit here that brings so may mistakes.I have done almost changes that you have mentioned. Now I have a question:I created drive-file-network-args.xml in test, and there is: -drive file=iser://example.org:6000/iqn.1992-01.com.example/1, format=raw but the system create -drive file=iscsi+iser://example.org:6000/iqn.1992-01.com.example/ 1,format=raw I doubt why it create iscsi+iser?On Thu, Dec 14, 2017 at 7:35 PM, Peter Krempa <pkrempa@redhat.com> wrote:On Thu, Dec 14, 2017 at 18:14:36 +0800, lichstor@gmail.com wrote:
> From: zhangshengyu <zhangshengyu@fusionstack.cn>
As pointed out last time, please follow the contributor guidelines. You
did not run make syntax-check. Also the contributor guidelines state
that you should write a commit message. I asked last time for that too.
Please note that I will NOT review any other version until you follow
the contributor guidelines at:
https://libvirt.org/hacking.html
See below for reasons.
>
> ---
> src/conf/domain_conf.c | 13 ++++++++
> src/qemu/qemu_block.c | 17 +++++++++-
> src/util/virstoragefile.c | 3 +-
> src/util/virstoragefile.h | 1 +
> tests/qemuxml2argvdata/disk-drive-network-iser.xml | 37 ++++++++++++++++++++++ Is this leftover debugging? it's certainly aligned wrongly and does not
> 5 files changed, 69 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/disk-drive-network-iser.xml
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 66e21c4bd..bf20cfd0c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7201,6 +7201,9 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
> iscsisrc->src->type = VIR_STORAGE_TYPE_NETWORK;
> iscsisrc->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
>
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("virDomainHostdevSubsysSCSIiSCSIDefParseXML"));
> +
make sense at all.
> if (!(iscsisrc->src->path = virXMLPropString(sourcenode, "name"))) {
> virReportError(VIR_ERR_XML_ERROR, "%s", It will be missing for all other protocols. You can't just do that.
> _("missing iSCSI hostdev source path name"));
> @@ -8416,6 +8419,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> unsigned int flags)
> {
> char *protocol = NULL;
> + char *transport = NULL;
> char *haveTLS = NULL;
> char *tlsCfg = NULL;
> int tlsCfgVal;
> @@ -8427,6 +8431,10 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> goto cleanup;
> }
>
> + if (!(transport = virXMLPropString(node, "transport"))) {
> + VIR_WARN("missing network source transport type");
> + }
> +
> if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) <= 0) { This is plain wrong. virDomainStorageNetworkParseHo
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown protocol type '%s'"), protocol);
> @@ -8495,6 +8503,9 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> if (virDomainStorageNetworkParseHosts(node, &src->hosts, &src->nhosts) < 0)
> goto cleanup;
>
> + if(src->hosts)
> + src->hosts->transport = virStorageNetHostTransportTypeFromString(transport);
sts should parse
this.
> +
> virStorageSourceNetworkAssignDefaultPorts(src); What?!?! How is this even supposed to work? Did you even bother running
>
> ret = 0;
> @@ -22326,6 +22337,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
>
> VIR_FREE(path);
>
> + virBufferEscapeString(attrBuf, " transport='%s'", "iser");
make check?!?! This breaks 9 test suites.
Missing line break here. Please follow the coding style in the rest of
> +
> if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT &&
> !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
> src->tlsFromConfig))
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 585f0255e..dcd7c6a5e 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -506,6 +506,20 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHo stDefPtr host,
> goto cleanup;
> break;
>
> + case VIR_STORAGE_NET_HOST_TRANS_ISER:
> + transport = "iser";
> + if (virAsprintf(&port, "%u", host->port) < 0)
> + goto cleanup;
> +
> + if (virJSONValueObjectCreate(&server,
> + "s:type", transport,
> + "s:host", host->name,
> + "s:port", port,
> + NULL) < 0)
> + goto cleanup;
> +
> +
> + break;
the function.
> case VIR_STORAGE_NET_HOST_TRANS_UNIX: [...]
> if (virJSONValueObjectCreate(&server,
> "s:type", "unix",
> diff --git a/tests/qemuxml2argvdata/disk-drive-network-iser.xml b/tests/qemuxml2argvdata/disk- You are missing the output file and change to qemuxml2argvtest.c, thusdrive-network-iser.xml
> new file mode 100644
> index 000000000..b3f4f9bfb
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/disk-drive-network-iser.xml
this is testing nothing. I also think that it will not work for most
cases unless you tweak qemuDiskSourceNeedsProps.
Also this fails to compile when the gluster driver is enabled:
storage/storage_backend_gluster.c: In function 'virStorageFileBackendGlusterI nitServer':
storage/storage_backend_gluster.c:600:5: error: enumeration value 'VIR_STORAGE_NET_HOST_TRANS_IS ER' not handled in switch [-Werror=switch]
switch ((virStorageNetHostTransport) host->transport) {
^~~~~~
Do not send any other version until you fix the tests. You also need to
run make syntax-check and fix any problems it points out.