Thanks for the review, am applying changes to address your comments.
On Tue, Jul 15, 2014 at 1:49 AM, Jim Fehlig <jfehlig@xxxxxxxx> wrote:
> David Kiarie wrote:
>> From: Kiarie Kahurani <davidkiarie4@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>> ---
>> 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;
>>
>>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list