I made clear why it create iscsi+iser, not only that the system have many place expect iscsi+iser type of strings, like qemuParseDriveURIString. but actually for qemu command line, iser://example.org... can work while iscsi+iser will not, how should I do? 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
On Fri, Dec 15, 2017 at 11:47 AM, Charles Kelimod <lichs...@gmail.com> wrote: > 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, >> > + _("virDomainHostdevSubsysSCSI >> iSCSIDefParseXML")); >> > + >> >> 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 @@ qemuBlockStorageSourceBuildJSO >> NSocketAddress(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