| Message ID | dc59ab41d51c8fa2bf1a2970fc023c9755753cdc.1457455170.git.crobinso@redhat.com |
|---|---|
| State | Accepted |
| Commit | 2dabe2e03e2ebd7ef28f2c61a019330a172b43a7 |
| Headers | show |
ping On 03/08/2016 11:39 AM, Cole Robinson wrote: > Judging by how the whitelist has skewed quite far from the original > error message, I think it's better to just drop these. > > If someone wants to revive this check I suggest implementing it on > a per-HV driver basis with PostParse callbacks. > --- > src/conf/domain_conf.c | 24 ------------------------ > 1 file changed, 24 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index d376a2c..ec14577 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -7969,17 +7969,6 @@ virDomainControllerDefParseXML(xmlNodePtr node, > break; > } > > - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("Controllers must use the 'pci' address type")); > - goto error; > - } > - > cleanup: > ctxt->node = saved; > VIR_FREE(typeStr); > @@ -8670,19 +8659,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, > goto error; > } > > - /* XXX what about ISA/USB based NIC models - once we support > - * them we should make sure address type is correct */ > - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && > - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("Network interfaces must use 'pci' address type")); > - goto error; > - } > - > switch (def->type) { > case VIR_DOMAIN_NET_TYPE_NETWORK: > if (network == NULL) { > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/18/2016 03:05 PM, Laine Stump wrote: > On 03/08/2016 11:39 AM, Cole Robinson wrote: >> Judging by how the whitelist has skewed quite far from the original >> error message, I think it's better to just drop these. >> >> If someone wants to revive this check I suggest implementing it on >> a per-HV driver basis with PostParse callbacks. > > Definitely the error messages for the current checks are incorrect, and > certain types that are allowed here would actually be invalid for some > hypervisors/machinetypes (the example you gave me in IRC was that vmware > doesn't have virtio-mmio, nor do most qemu machinetypes), so you are correct > that the proper place for at least some of the validation is in a post-parse > callback. > > There is some value in the current validation, but it's dubious since any > failure would have led to a misleading error message. Based on that I say ACK > to this, but someone should put "improve address type validation in postparse" > on their todo list :-) Thanks, pushed. I'll add it to http://wiki.libvirt.org/page/BiteSizedTasks - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d376a2c..ec14577 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7969,17 +7969,6 @@ virDomainControllerDefParseXML(xmlNodePtr node, break; } - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Controllers must use the 'pci' address type")); - goto error; - } - cleanup: ctxt->node = saved; VIR_FREE(typeStr); @@ -8670,19 +8659,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - /* XXX what about ISA/USB based NIC models - once we support - * them we should make sure address type is correct */ - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Network interfaces must use 'pci' address type")); - goto error; - } - switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: if (network == NULL) {