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

[Peter Krempa]

On 10/14/14 10:56, John Ferlan wrote:
> 
> 
> 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

I deliberately chose to leave the error in place so that this patch
doesn't change behavior in these cases in case something would rely on
it. I'm not familiar enough with the s390 addressing scheme to make a
qualified decision about that.

At any rate that can be done as a follow up.

Peter



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141014/55355ab6/attachment-0001.sig>