Re: [Xen-devel] [PATCH v11 10/17] libxl: set correct nic type depending on the guest

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v11 10/17] libxl: set correct nic type 
> depending on the guest"):
>> Fix the use of nic type, which results in the following for each type
>> of domain:
>>
>>  * HVM: let the user choose, if none specified use VIF_IOEMU.
>>  * PV: use VIF is none provided, return error if VIF_IOEMU requested.
> ...
>> -int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
>> +int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
>> +                                 uint32_t domid)
>>  {
> ...
>> +    case LIBXL_DOMAIN_TYPE_PV:
>> +        if (nic->nictype == LIBXL_NIC_TYPE_VIF_IOEMU)
>> +            return ERROR_INVAL;
> 
> Should this log ?

Yes, it would be helpful.

> Also:
> 
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 9330012..fc112ce 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -941,7 +941,7 @@ static void domcreate_launch_dm(libxl__egc *egc, 
>> libxl__ao_devices *aodevs,
>>           * called libxl_device_nic_add at this point, but qemu needs
>>           * the nic information to be complete.
>>           */
>> -        libxl__device_nic_setdefault(gc, &d_config->nics[i]);
>> +        libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
> 
> This (and the next one) seem to be lacking error handling, and you're
> making _nic_setdefault capable of returning errors.

Yes, I will change this two.

Thanks for the review.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel