[libvirt] [PATCH] vz: assign static IPs and default gateways for network adapter

[Mikhail Feoktistov]

send v2

06.07.2015 21:28, Dmitry Guryanov пишет:
> On 07/01/2015 01:15 PM, Mikhail Feoktistov wrote:
>> We support only one IPv4 and one IPv6 default gateway.
>> If static IPs are not present in instance config,
>> then we switch on DHCP for this adapter.
>> PrlVmDevNet_SetAutoApply to makes necessary settings within guest OS
>> In linux case it creates network startup scripts
>> /etc/sysconfig/network-scripts/ifcfg-ethN and fills it with necessary
>> parameters.
>> ---
>>   src/vz/vz_sdk.c |  120 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 120 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
>> index f27098c..7d318f8 100644
>> --- a/src/vz/vz_sdk.c
>> +++ b/src/vz/vz_sdk.c
>> @@ -2788,8 +2788,13 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
>>       PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
>>       PRL_HANDLE vnet = PRL_INVALID_HANDLE;
>>       PRL_HANDLE job = PRL_INVALID_HANDLE;
>> +    PRL_HANDLE addrlist = PRL_INVALID_HANDLE;
>> +    size_t i;
>>       int ret = -1;
>>       char macstr[PRL_MAC_STRING_BUFNAME];
>> +    char *addrstr = NULL;
>> +    bool ipv6present = false;
>> +    bool ipv4present = false;
>>         if (prlsdkCheckNetUnsupportedParams(net) < 0)
>>           return -1;
>> @@ -2814,6 +2819,118 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
>>       pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
>>       prlsdkCheckRetGoto(pret, cleanup);
>>   +    pret = PrlApi_CreateStringsList(&addrlist);
>> +    prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +    for (i = 0; i < net->nips; i++) {
>> +        virBuffer buf = VIR_BUFFER_INITIALIZER;
>> +        char *tmpstr;
>> +
>> +        if (AF_INET == VIR_SOCKET_ADDR_FAMILY(&net->ips[i]->address))
>> +            ipv4present = true;
>> +        else if (AF_INET6 == 
>> VIR_SOCKET_ADDR_FAMILY(&net->ips[i]->address))
>> +            ipv6present = true;
>> +        else
>> +            continue;
>> +
>> +        if (!(tmpstr = virSocketAddrFormat(&net->ips[i]->address)))
>> +            goto cleanup;
>> +
>> +        virBufferAsprintf(&buf, "%s/%d", tmpstr, net->ips[i]->prefix);
>> +        VIR_FREE(tmpstr);
>> +        if (!(addrstr = virBufferContentAndReset(&buf)))
>> +            goto cleanup;
>
> It's better to use virAsprintf here, since you do string formatting 
> only once.
>
>> +
>> +        pret = PrlStrList_AddItem(addrlist, addrstr);
>> +        prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +        VIR_FREE(addrstr);
>> +        addrstr = NULL;
> VIR_FREE sets the pointer to NULL, you don't need to do it by 
> yourself. Also it's being caught by "make syntax-check" Could you, 
> please, run it before sending patches?
>
>
>> +    }
>> +
>> +    if (ipv4present || ipv6present) {
>> +        pret = PrlVmDevNet_SetNetAddresses(sdknet, addrlist);
>> +        prlsdkCheckRetGoto(pret, cleanup);
>> +    }
>> +
>> +    pret = PrlVmDevNet_SetConfigureWithDhcp(sdknet, !ipv4present);
>> +    prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +    pret = PrlVmDevNet_SetConfigureWithDhcpIPv6(sdknet, !ipv6present);
>> +    prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +    pret = PrlVmDevNet_SetAutoApply(sdknet, true);
>> +    prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +    if (net->nroutes) {
>
> As I can remember, it's not possible to set gateway together with 
> DHCP. So I think you should set up gateways before dhcp and only if 
> ipv4present/ipv6present is true.
>
>> +        bool alreadySetIPv4Gateway = false;
>> +        bool alreadySetIPv6Gateway = false;
>> +
>> +        for (i = 0; i < net->nroutes; i++) {
>> +            virSocketAddrPtr addrdst, gateway;
>> +            virSocketAddr zero;
>> +
>> +            addrdst = virNetworkRouteDefGetAddress(net->routes[i]);
>> +            gateway = virNetworkRouteDefGetGateway(net->routes[i]);
>> +
>> +            ignore_value(virSocketAddrParse(&zero,
>> + (VIR_SOCKET_ADDR_IS_FAMILY(addrdst, AF_INET)
>> +                                     ? VIR_SOCKET_ADDR_IPV4_ALL
>> +                                     : VIR_SOCKET_ADDR_IPV6_ALL),
>> + VIR_SOCKET_ADDR_FAMILY(addrdst)));
>> +
>> +            if (!virSocketAddrEqual(addrdst, &zero)) {
>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                               _("Support only default gateway"));
>> +                goto cleanup;
>> +            }
>> +
>> +            switch (VIR_SOCKET_ADDR_FAMILY(gateway)) {
>> +            case AF_INET:
>> +                if (alreadySetIPv4Gateway) {
>> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                                   _("Support only one IPv4 default 
>> gateway"));
>> +                    goto cleanup;
>> +                }
>> +
>> +                if (!(addrstr = virSocketAddrFormat(gateway)))
>> +                    goto cleanup;
>> +
>> +                pret = PrlVmDevNet_SetDefaultGateway(sdknet, addrstr);
>> +                prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +                alreadySetIPv4Gateway = true;
>> +                break;
>> +
>> +            case AF_INET6:
>> +                if (alreadySetIPv6Gateway) {
>> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                                   _("Support only one IPv6 default 
>> gateway"));
>> +                    goto cleanup;
>> +                }
>> +
>> +                if (!(addrstr = virSocketAddrFormat(gateway)))
>> +                    goto cleanup;
>> +
>> +                pret = PrlVmDevNet_SetDefaultGatewayIPv6(sdknet, 
>> addrstr);
>> +                prlsdkCheckRetGoto(pret, cleanup);
>> +
>> +                alreadySetIPv6Gateway = true;
>> +                break;
>> +
>> +            default:
>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                               _("Support only IPv4 and IPv6 default 
>> gateways"));
>> +                goto cleanup;
> You report error here, if address family is not ipv4 or ipv6 but don't 
> report for IP addresses. Could you make the same behavior for both 
> parameters?
>
>
>> +            }
>> +
>> +            if (addrstr) {
>> +                VIR_FREE(addrstr);
>> +                addrstr = NULL;
> VIR_FREE  set pointer to NULL, you don't need to do it.
>> +            }
>> +        }
>> +    }
>> +
>>       if (isCt) {
>>           if (net->model)
>>               VIR_WARN("Setting network adapter for containers is not "
>> @@ -2879,6 +2996,9 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
>>         ret = 0;
>>    cleanup:
>> +    if (addrstr)
>> +         VIR_FREE(addrstr);
> VIR_FREE handles NULL argument, so you don't need to check.
>
>> +    PrlHandle_Free(addrlist);
>>       PrlHandle_Free(vnet);
>>       PrlHandle_Free(sdknet);
>>       return ret;
>