Re: [libvirt] [PATCH 06/12] qemu: block: Add JSON props generator for iSCSI protocol

On Sun, Nov 05, 2017 at 16:03:25 +0100, Ján Tomko wrote: > On Fri, Nov 03, 2017 at 03:29:23PM +0100, Peter Krempa wrote: > > From: John Ferlan <jferlan(a)redhat.com&gt; > > > > --- > > +static virJSONValuePtr > > +qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) > > +{ > > [...] > > > + if (VIR_STRDUP(target, src->path) < 0) > > + goto cleanup; > > + > > + /* Separate the target and lun */ > > + if ((lunStr = strchr(target, '/'))) { > > + *(lunStr++) = '\0'; > > + if (virStrToLong_ui(lunStr, NULL, 10, &lun) < 0) { > > + virReportError(VIR_ERR_INTERNAL_ERROR, > > + _("cannot parse target for lunStr '%s'"), > > + target); > > + goto cleanup; > > + } > > + } > > + > > + /* combine host and port into portal */ > > + if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) > > + goto cleanup; > > Can src->hosts[0].name possibly be a literal IPv6 address? Yes, you are right. How about the following diff squashed in? diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index ffe2892ab..428c0b465 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -742,8 +742,15 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) } /* combine host and port into portal */ - if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) - goto cleanup; + if (virSocketAddrNumericFamily(src->hosts[0].name) == AF_INET6) { + if (virAsprintf(&portal, "[%s]:%u", + src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + } else { + if (virAsprintf(&portal, "%s:%u", + src->hosts[0].name, src->hosts[0].port) < 0) + goto cleanup; + } if (src->auth) { username = src->auth->username;

Also our parser is buggy. I'll send patches separately.