[libvirt] [PATCH 3/5] net: use virDomainNICModelType{From|To}String functions

[John Ferlan]

On 01/15/2013 03:24 AM, Guannan Ren wrote:
> On 01/14/2013 10:15 PM, John Ferlan wrote:
>> On 01/13/2013 10:34 AM, Guannan Ren wrote:
>>>       if (def->model) {
>>>           virBufferEscapeString(buf, "<model type='%s'/>\n",
>>> -                              def->model);
>>> -        if (STREQ(def->model, "virtio") &&
>>> +                             
>>> virDomainNICModelTypeToString(def->model));
>>> +        if ((def->model == VIR_DOMAIN_NIC_MODEL_VIRTIO) &&
>>>               (def->driver.virtio.name || def->driver.virtio.txmode)) {
>>>               virBufferAddLit(buf, "<driver");
>>>               if (def->driver.virtio.name) {
>> Since model can be "VIR_DOMAIN_NIC_MODEL_DEFAULT" (zero), is this what
>> you really want?
> 
>       if (def->model)
>            virBufferEscapeString(buf, "<model type='%s'/>\n",
> virDomainNICModelTypeToString(def->model));
> 
>       if def->model is  VIR_DOMAIN_NIC_MODEL_DEFAULT(0),
> virDomainNICModelTypeToString
>       will not be executed.
> 
>       For input XML   VIR_DOMAIN_NIC_MODEL_DEFAULT means no particular
> model is specified.
>       in hypervisors code, if often to set it to a default value, for
> qemu , it is "rtl8139".
>       For output XML almost, if def->mode is still
> VIR_DOMAIN_NIC_MODEL_DEFAULT, it will skip
>       printing the model attribute. But there is slightly different
> between each of hypervisors.
> 
> 

OK - Fair enough.  I'm still getting the feel of the code. My frame of
reference was going from a NULL (char*) reference to a integer != 0
reference and thinking hmm... in certain places NULL could mean
something different.


>>
>>
>>   diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>> index c604bd2..0409b0b 100644
>> --- a/src/vmx/vmx.c
>> +++ b/src/vmx/vmx.c
>> @@ -2597,10 +2597,10 @@ virVMXParseEthernet(virConfPtr conf, int
>> controller, virDomainNetDefPtr *def)
>>       /* Setup virDomainNetDef */
>>       if (connectionType == NULL || STRCASEEQ(connectionType,
>> "bridged")) {
>>           (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
>> -        (*def)->model = virtualDev;
>> +        (*def)->model = virDomainNICModelTypeFromString(virtualDev);
>> What if virDomainNICModelTypeFromString() < 0
> 
> 
>     virtualDev is guarantee to be a valid NIC model string in the codes
> above, so
>     there is no possibility for it to return -1.
> 

OK, but seeing as my current frame of reference is resolving Coverity
bugs where the tool will find 3 locations in a module that handle the
return and 2 that don't and then flag the 2 that don't.

>     Guannan
>