[libvirt] [PATCH 1/2] libxl: support syntax <interface type="hostdev">

[Jim Fehlig]

Chun Yan Liu wrote:
>   
>>>> On 5/10/2014 at 06:18 AM, in message <536D541D.5040107 at suse.com>, Jim Fehlig
>>>>         
> <jfehlig at suse.com> wrote: 
>   
>> Chunyan Liu wrote: 
>>     
>>> Signed-off-by: Chunyan Liu <cyliu at suse.com> 
>>>    
>>>       
>>  
>> A while back when testing Chunyan's "common hostdev library" series, I 
>> mentioned that <interface type='hostdev'> was not working with the libxl 
>> driver.  Chunyan later privately sent a "v1" of this patch for testing 
>> in my setup.  In addition to testing, I provided some private comments.  
>> I see those have been incorporated in this patch and functionally it 
>> looks good, but I do have one additional question about the commit 
>> message... 
>>  
>>     
>>> --- 
>>>  src/libxl/libxl_conf.c | 16 +++++++++++----- 
>>>  1 file changed, 11 insertions(+), 5 deletions(-) 
>>>
>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c 
>>> index 298c8a1..b7fed7f 100644 
>>> --- a/src/libxl/libxl_conf.c 
>>> +++ b/src/libxl/libxl_conf.c 
>>> @@ -921,25 +921,31 @@ static int 
>>>  libxlMakeNicList(virDomainDefPtr def,  libxl_domain_config *d_config) 
>>>  { 
>>>      virDomainNetDefPtr *l_nics = def->nets; 
>>> -    int nnics = def->nnets; 
>>> +    size_t nnics = def->nnets; 
>>>      libxl_device_nic *x_nics; 
>>> -    size_t i; 
>>> +    size_t i, nvnics = 0; 
>>>   
>>>      if (VIR_ALLOC_N(x_nics, nnics) < 0) 
>>>          return -1; 
>>>   
>>>      for (i = 0; i < nnics; i++) { 
>>> -        if (libxlMakeNic(def, l_nics[i], &x_nics[i])) 
>>> +        if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) 
>>> +            continue; 
>>>    
>>>       
>>  
>> After looking at this again, it seems we are really *fixing* <interface 
>> type='hostdev'>.  The driver already supports creating the hostdev 
>> device, but without this patch a libxl_device_nic is created too.  Is 
>> that a fair statement?  If so, the commit message should be changed to 
>> reflect this.  Thanks! 
>>     
>
> A NET_TYPE_HOSTDEV device is really a hostdev device, the driver will create
> a hostdev device for it, so no need to create a libxl_device_nic again. Before 
> this patch, it tried to call libxlMakeNic to create a libxl_device_nic for it but
> failed since NET_TYPE_HOSTDEV is not supported there.
>   

Yep, understood.

> I'll add this to commit message. Is that OK?
>   

I fixed up the commit message and pushed the patch.  Thanks Chunyan!

Regards,
Jim