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