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

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(a)redhat.com&gt;