Re: [Qemu-devel] [PATCH v2 16/45] scsi-disk: Track tray locked state

[Markus Armbruster]

Hannes Reinecke <address@hidden> writes:

> On 08/04/2011 08:39 AM, Markus Armbruster wrote:
>> Hannes Reinecke<address@hidden>  writes:
>>
>>> On 08/03/2011 03:07 PM, Markus Armbruster wrote:
>>>> We already track it in BlockDriverState.  Just like tray open/close
>>>> state, we should track it in the device models instead, because it's
>>>> device state.
>>>>
>>>> Signed-off-by: Markus Armbruster<address@hidden>
>>>> ---
>>>>    hw/scsi-disk.c |    4 +++-
>>>>    1 files changed, 3 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>>>> index db72b86..8ca69f2 100644
>>>> --- a/hw/scsi-disk.c
>>>> +++ b/hw/scsi-disk.c
>>>> @@ -73,6 +73,7 @@ struct SCSIDiskState
>>>>        char *serial;
>>>>        SCSISense sense;
>>>>        bool tray_open;
>>>> +    bool tray_locked;
>>>>    };
>>>>
>>> Hmm. Shouldn't we use a more generic 'flags' here and have bits for
>>> the individual tray states?
>>> Feels like a waste to have individual values here.
>>
>> On my system, struct SCSIDiskState is 248 bytes before my series (5
>> bytes of padding at the end), and 248 bytes after (3 bytes padding).  We
>> use one per disk.
>>
>> I dare say switching to flags won't make a noticable difference in
>> memory use ;)
>>
>> The bool members are easy to read and hard to screw up.
>>
>> Flags can be nicer when you manipulate several of them together.
>
> Well, but this just proves my point.
> Each 'bool' variable takes up one byte, of which only one bit is
> used. Which qualifies as a waste of space.

A few data bytes per SCSI disk are a drop in the ocean.

Plus they're offset by the flags data word and the extra code bytes you
need to extract the bits.

> But if that's okay with everyone and the general consensus on how to
> code flags in qemu, I'll shut up.

It's a big series, and respinning it is work.  Can we sort such things
in followup patches?