From mboxrd@z Thu Jan  1 00:00:00 1970
From: Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v11 10/17] libxl: set correct nic type
 depending on the guest
Date: Tue, 24 Jul 2012 17:02:47 +0100
Message-ID: <500EC727.1070807@citrix.com>
References: <1343064465-17864-1-git-send-email-roger.pau@citrix.com>
	<1343064465-17864-11-git-send-email-roger.pau@citrix.com>
	<20494.48940.372805.134165@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20494.48940.372805.134165@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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.