[libvirt] [PATCH 04/18] conf: Add virDomainNetHasVirtioModel

[Cole Robinson]

On 01/18/2019 08:35 AM, Andrea Bolognani wrote:
> On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
> [...]
>> @@ -11329,6 +11329,22 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>              goto error;
>>      }
>>  
>> +    /* NIC model (see -net nic,model=?).  We only check that it looks
>> +     * reasonable, not that it is a supported NIC type.  FWIW kvm
>> +     * supports these types as of April 2008:
>> +     * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio
>> +     * QEMU PPC64 supports spapr-vlan
>> +     */
>> +    if (model != NULL) {
>> +        if (strspn(model, NET_MODEL_CHARS) < strlen(model)) {
>> +            virReportError(VIR_ERR_INVALID_ARG, "%s",
>> +                           _("Model name contains invalid characters"));
>> +            goto error;
>> +        }
>> +        def->model = model;
>> +        model = NULL;
>> +    }
> 
> Can you please split this code motion...
> 
>> +
>>      switch (def->type) {
>>      case VIR_DOMAIN_NET_TYPE_NETWORK:
>>          if (network == NULL) {
>> @@ -11346,7 +11362,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>          break;
>>  
>>      case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
>> -        if (STRNEQ_NULLABLE(model, "virtio")) {
>> +        if (!virDomainNetHasVirtioModel(def)) {
>>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                             _("Wrong or no <model> 'type' attribute "
>>                               "specified with <interface type='vhostuser'/>. "
> 
> ... along with adjusting this from model to def->model, off to its
> own preparatory patch?
> 
> [...]
>> +bool
>> +virDomainNetHasVirtioModel(const virDomainNetDef *iface)
>> +{
>> +    return STREQ_NULLABLE(iface->model, "virtio");
>> +}
> 
> I'd probably call this virDomainNetIsModelVirtio() and call the
> argument 'net', but your version is fine too if you prefer it.
> 
> With the preparatory work in a separate patch,
> 
>   Reviewed-by: Andrea Bolognani <abologna at redhat.com>
> 

I'll adjust it all. The iface naming was following some similar
functions above it in domain_conf.c but it's certainly less common than
'net' naming

Thanks,
Cole

- Cole