Re: [PATCH v9 08/13] Add IOREQ_TYPE_VMWARE_PORT

[Don Slutz <dslutz@verizon.com>]

On 02/17/15 05:08, Paul Durrant wrote:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>> bounces@lists.xen.org] On Behalf Of Don Slutz
>> Sent: 16 February 2015 23:05


>> -static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, bool_t
>> buf)
>> +static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, int buf)
>>  {
>> -    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
>> +    struct hvm_ioreq_page *iorp = NULL;
>> +
>> +    switch ( buf )
>> +    {
>> +    case 0:
>> +        iorp = &s->ioreq;
>> +        break;
>> +    case 1:
>> +        iorp = &s->bufioreq;
>> +        break;
>> +    case 2:
>> +        iorp = &s->vmport_ioreq;
>> +        break;
> 
> Now that buf is no longer a bool, could we have #defined values for the types rather than magic numbers?
> 

Sure.

>> +    }
>> +    ASSERT(iorp);
>>
>>      destroy_ring_for_helper(&iorp->va, iorp->page);
>>  }

...

>> +        case HVMOP_IO_RANGE_VMWARE_PORT:
>> +            type_name = "VMware port";
>> +            limit = 1;
>> +            break;
>> +        case HVMOP_IO_RANGE_TIMEOFFSET:
>> +            type_name = "timeoffset";
>> +            limit = 1;
>> +            break;
>> +        case HVMOP_IO_RANGE_INVALIDATE:
>> +            type_name = "invalidate";
>> +            limit = 1;
>> +            break;
>> +        default:
>> +            break;
> 
> Thanks for adding the extra types for timeoffset and
> invalidate, although I don't think the invalidate type is
> needed. This ioreq is actually broadcast, so there is no
> selection required.
> 

Ok, will drop the invalidate one.

   -Don Slutz

>   Paul