<div dir="ltr">I made clear why it create iscsi+iser, not only that the system have many place expect iscsi+iser type of strings, like qemuParseDriveURIString.<div>but actually for qemu command line, iser://example.org... can work while iscsi+iser will not, how should I do?</div><div>Currently for json type of command line in qemu can work for me, if need to use "iser://" and let disk-drive-network-iser.args to work, need to modify many place like qemuParseDriveURIString</div></div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Dec 15, 2017 at 11:47 AM, Charles Kelimod <<a href="mailto:lichstor@gmail.com" target="_blank">lichstor@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thank you for your patience, I regret for my first time submit here that brings so may mistakes. <div> </div><div>I have done almost changes that you have mentioned. Now I have a question:</div><div>I created drive-file-network-args.xml in test, and there is: -drive file=iser://<a href="http://example.org:6000/iqn.1992-01.com.example/1,format=raw" target="_blank">example.org:6000/iqn.1992-01.com.example/1,format=raw</a></div><div>but the system create -drive file=iscsi+iser://<a href="http://example.org:6000/iqn.1992-01.com.example/1,format=raw" target="_blank">example.org:6000/iqn.1992-01.com.example/1,format=raw</a></div><div>I doubt why it create iscsi+iser?</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"> <div class="gmail_quote">On Thu, Dec 14, 2017 at 7:35 PM, Peter Krempa <<a href="mailto:pkrempa@redhat.com" target="_blank">pkrempa@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, Dec 14, 2017 at 18:14:36 +0800, <a href="mailto:lichstor@gmail.com" target="_blank">lichstor@gmail.com</a> wrote: > From: zhangshengyu <<a href="mailto:zhangshengyu@fusionstack.cn" target="_blank">zhangshengyu@fusionstack.cn</a>> 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: <a href="https://libvirt.org/hacking.html" rel="noreferrer" target="_blank">https://libvirt.org/hacking.html</a> 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. <div><div class="m_-8116138281067736694h5"> > + > 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; </div></div>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. </blockquote></div> </div> </div></div></blockquote></div> </div>