[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

Paolo


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]