Re: [libvirt] [PATCH 05/13] qemu: support NBD with Unix sockets

Il 11/03/2013 10:34, Osier Yang ha scritto: >> >> + if (STRPREFIX(host, "unix:/")) { >> + src = strchr(host + strlen("unix:"), ':'); >> + if (src) >> + *src++ = '\0'; > > Same comments as previous patches. I think in this case the value that is assigned to src is complex enough that it's better to keep it separate.

>> >> - *port++ = '\0'; >> - h->name = strdup(host); >> - if (!h->name) >> - goto no_memory; >> + h->transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX; >> + h->socket = strdup(host + strlen("unix:")); >> + } else { >> + port = strchr(host, ':'); >> + if (!port) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("cannot parse nbd filename '%s'"), >> disk->src); >> + goto error; >> + } >> >> - src = strchr(port, ':'); >> - if (src) >> - *src++ = '\0'; >> + *port++ = '\0'; >> + h->name = strdup(host); >> + if (!h->name) >> + goto no_memory; >> >> - h->port = strdup(port); >> - if (!h->port) >> - goto no_memory; >> + src = strchr(port, ':'); >> + if (src) >> + *src++ = '\0'; >> + >> + h->port = strdup(port); >> + if (!h->port) >> + goto no_memory; >> + } >> >> if (src&& STRPREFIX(src, "exportname=")) { >> src = strdup(strchr(src, '=') + 1); >> @@ -2261,6 +2270,14 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, >> virBufferPtr opt) >> virBufferEscape(opt, ',', ",", ":%s", >> disk->hosts->port ? disk->hosts->port : >> "10809"); >> break; >> + case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX: >> + if (!disk->hosts->socket) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("socket attribute required for unix >> transport")); >> + return -1; >> + } > > Not sure if we should do this when parsing, as I think the "socket" is > required as long as the transport protocol is "unix", unless it's > generated somewhere. Yeah, we cannot be sure in general that it will be required for _all_ protocols, so I kept it here and followed what gluster does.