[libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code

[David kiarie]

Thanks for the review, am applying changes to address your comments.



On Tue, Jul 15, 2014 at 1:49 AM, Jim Fehlig <jfehlig at suse.com> wrote:
> David Kiarie wrote:
>> From: Kiarie Kahurani <davidkiarie4 at gmail.com>
>>
>> Introduce the function
>>  xenFormatXMDomainNet(......)
>>
>
> On the parsing side, you called this xenParseXMVif.  To be consistent,
> this should be xenFormatXMVif.
>
>> I think this could be done in a cleaner way
>>
>> signed-of-by: David Kiarie<davidkiarie4 at gmail.com>
>> ---
>>  src/xenxs/xen_xm.c | 49 +++++++++++++++++++++++++++++++------------------
>>  1 file changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
>> index ee5dc19..8dd2823 100644
>> --- a/src/xenxs/xen_xm.c
>> +++ b/src/xenxs/xen_xm.c
>> @@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr def)
>>  cleanup:
>>      return -1;
>>  }
>> +static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn,
>> +                         virDomainDefPtr def, int xendConfigVersion)
>> +{
>> +   virConfValuePtr netVal = NULL;
>> +   size_t i;
>> +
>> +   int hvm = STREQ(def->os.type, "hvm");
>> +
>> +   if (VIR_ALLOC(netVal) < 0)
>> +        goto cleanup;
>> +    netVal->type = VIR_CONF_LIST;
>> +    netVal->list = NULL;
>> +
>> +    for (i = 0; i < def->nnets; i++) {
>> +        if (xenFormatXMNet(conn, netVal, def->nets[i],
>> +                           hvm, xendConfigVersion) < 0)
>>
>
> Ah, I see xenFormatXMNet already exists.  So maybe xenFormatXMVifs for
> the list of VIFs and xenFormatXMVif for each individual VIF is clearer.
>
> Regards,
> Jim
>
>> +           return -1;
>> +    }
>> +    if (netVal->list != NULL) {
>> +        int ret = virConfSetValue(conf, "vif", netVal);
>> +        netVal = NULL;
>> +        if (ret < 0)
>> +            return -1;
>> +    }
>> +    VIR_FREE(netVal);
>> +    return 0;
>> + cleanup:
>> +    VIR_FREE(netVal);
>> +    return -1;
>> +}
>>  virConfPtr xenFormatXM(virConnectPtr conn,
>>                                     virDomainDefPtr def,
>>                                     int xendConfigVersion)
>> @@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
>>              goto cleanup;
>>      }
>>      VIR_FREE(diskVal);
>> -
>> -    if (VIR_ALLOC(netVal) < 0)
>> +    if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) < 0)
>>          goto cleanup;
>> -    netVal->type = VIR_CONF_LIST;
>> -    netVal->list = NULL;
>> -
>> -    for (i = 0; i < def->nnets; i++) {
>> -        if (xenFormatXMNet(conn, netVal, def->nets[i],
>> -                           hvm, xendConfigVersion) < 0)
>> -            goto cleanup;
>> -    }
>> -    if (netVal->list != NULL) {
>> -        int ret = virConfSetValue(conf, "vif", netVal);
>> -        netVal = NULL;
>> -        if (ret < 0)
>> -            goto cleanup;
>> -    }
>> -    VIR_FREE(netVal);
>> -
>>      if (xenFormatXMPCI(conf, def) < 0)
>>          goto cleanup;
>>
>>