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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list