Re: [libvirt] [PATCH 1/9] conf: Improve adding of new address types

On 10/14/2014 03:29 AM, Peter Krempa wrote: > Use typecasted switch statement and note the type used to select the > address type in a comment. > --- > src/conf/domain_conf.c | 36 ++++++++++++++++++++++++------------ > src/conf/domain_conf.h | 2 +- > 2 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 35bbd91..55a1cc5 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -3306,7 +3306,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, > virBufferAsprintf(buf, "<address type='%s'", > virDomainDeviceAddressTypeToString(info->type)); > > - switch (info->type) { > + switch ((virDomainDeviceAddressType) info->type) { > case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: > virBufferAsprintf(buf, " domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' function='0x%.1x'", > info->addr.pci.domain, > @@ -3368,10 +3368,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, > virBufferAsprintf(buf, " irq='0x%x'", info->addr.isa.irq); > break; > > - default: > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("unknown address type '%d'"), info->type); > - return -1; > + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: > + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: > + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST: > + break; > } > > virBufferAddLit(buf, "/>\n"); > @@ -3816,7 +3816,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, > type = virXMLPropString(address, "type"); > > if (type) { > - if ((info->type = virDomainDeviceAddressTypeFromString(type)) < 0) { > + if ((info->type = virDomainDeviceAddressTypeFromString(type)) <= 0) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > _("unknown address type '%s'"), type); > goto cleanup; > @@ -3827,7 +3827,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, > goto cleanup; > } > > - switch (info->type) { > + switch ((virDomainDeviceAddressType) info->type) { > case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: > if (virDevicePCIAddressParseXML(address, &info->addr.pci) < 0) > goto cleanup; > @@ -3873,11 +3873,14 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, > goto cleanup; > break; > > - default: > - /* Should not happen */ > - virReportError(VIR_ERR_INTERNAL_ERROR, > - "%s", _("Unknown device address type")); > + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("virtio-s390 bus doesn't have an address")); Do we really care? We could just ignore it (like MMIO) and it'll be lost. Of course since we would have fallen into 'default' before and thus resulted in error - I do see why you've added the error. I'm OK with the error - just figured I'd note it and let you decide.. ACK John