On Thu, Dec 14, 2017 at 18:14:36 +0800, lichstor(a)gmail.com wrote:
From: zhangshengyu <zhangshengyu(a)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 ++++++++++++++++++++++
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"));
+
Is this leftover debugging? it's certainly aligned wrongly and does not
make sense at all.
if (!(iscsisrc->src->path = virXMLPropString(sourcenode,
"name"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("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");
It will be missing for all other protocols. You can't just do that.
+ }
+
if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) <= 0) {
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);
This is plain wrong. virDomainStorageNetworkParseHosts should parse
this.
+
virStorageSourceNetworkAssignDefaultPorts(src);
ret = 0;
@@ -22326,6 +22337,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
VIR_FREE(path);
+ virBufferEscapeString(attrBuf, " transport='%s'",
"iser");
What?!?! How is this even supposed to work? Did you even bother running
make check?!?! This breaks 9 test suites.
+
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(virStorageNetHostDefPtr
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;
Missing line break here. Please follow the coding style in the rest of
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-drive-network-iser.xml
new file mode 100644
index 000000000..b3f4f9bfb
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-drive-network-iser.xml
You are missing the output file and change to qemuxml2argvtest.c, thus
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
'virStorageFileBackendGlusterInitServer':
storage/storage_backend_gluster.c:600:5: error: enumeration value
'VIR_STORAGE_NET_HOST_TRANS_ISER' 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.