[Qemu-devel] Re: [PATCHv4 1/2] qdev: add bit property type

[Qemu-devel] Re: [PATCHv4 1/2] qdev: add bit property type

[Gerd Hoffmann]

On 12/24/09 13:43, Michael S. Tsirkin wrote:
> This adds "bit" property type, which is a boolean stored in a 32 bit
> integer field, with legal values on and off.  Will be used by virtio for
> feature bits.

> +static void *qdev_bit_prop_ptr(DeviceState *dev, Property *prop)
> +{
> +    void *ptr = dev;
> +    assert(prop->info->type == PROP_TYPE_BIT);
> +    ptr += prop->offset / 32;
> +    return ptr;
> +}

I don't like that idea.  IMHO prop->offset should be specified in bytes 
no matter what the property is.  Better add a new prop->bitnr field to 
specify which bit of the 32bit word should be toggled.  Then you don't 
need this function in the first place.

cheers,
   Gerd

[Qemu-devel] Re: [PATCHv4 2/2] virtio: add features as qdev properties

[Gerd Hoffmann]

On 12/24/09 13:44, Michael S. Tsirkin wrote:
> Add feature bits as properties to virtio. This makes it possible to e.g. define
> machine without indirect buffer support, which is required for 0.10
> compatibility, or without hardware checksum support, which is required for 0.11
> compatibility.  Since default values for optional features are now set by qdev,
> get_features callback has been modified: it sets non-optional bits, and clears
> bits not supported by host.

I guess you'll need a bunch of entries for pc-0.10 and pc-0.11 to switch 
the virtio devices into the correct compatibility modes?  Should 
probably go as patch #3 of this series.

cheers,
   Gerd

[Qemu-devel] Re: [PATCHv4 2/2] virtio: add features as qdev properties

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 03:24:18PM +0100, Gerd Hoffmann wrote:
> On 12/24/09 13:44, Michael S. Tsirkin wrote:
>> Add feature bits as properties to virtio. This makes it possible to e.g. define
>> machine without indirect buffer support, which is required for 0.10
>> compatibility, or without hardware checksum support, which is required for 0.11
>> compatibility.  Since default values for optional features are now set by qdev,
>> get_features callback has been modified: it sets non-optional bits, and clears
>> bits not supported by host.
>
> I guess you'll need a bunch of entries for pc-0.10 and pc-0.11 to switch  
> the virtio devices into the correct compatibility modes?  Should  
> probably go as patch #3 of this series.
>
> cheers,
>   Gerd

Yes, but this will be done separately, after these are applied.

-- 
MST

[Qemu-devel] Re: [PATCHv4 1/2] qdev: add bit property type

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 03:22:36PM +0100, Gerd Hoffmann wrote:
> On 12/24/09 13:43, Michael S. Tsirkin wrote:
>> This adds "bit" property type, which is a boolean stored in a 32 bit
>> integer field, with legal values on and off.  Will be used by virtio for
>> feature bits.
>
>> +static void *qdev_bit_prop_ptr(DeviceState *dev, Property *prop)
>> +{
>> +    void *ptr = dev;
>> +    assert(prop->info->type == PROP_TYPE_BIT);
>> +    ptr += prop->offset / 32;
>> +    return ptr;
>> +}
>
> I don't like that idea.  IMHO prop->offset should be specified in bytes  
> no matter what the property is.  Better add a new prop->bitnr field to  
> specify which bit of the 32bit word should be toggled.  Then you don't  
> need this function in the first place.
>
> cheers,
>   Gerd

OK, I reworked the patches in this way.