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 <pkre...@redhat.com> wrote: > On Thu, Dec 14, 2017 at 18:14:36 +0800, lichs...@gmail.com wrote: > > From: zhangshengyu <zhangshen...@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, > > + _("virDomainHostdevSubsysSCSIiSCS > IDefParseXML")); > > + > > 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 = virStorageNetHostTransportType > FromString(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. >
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list