From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QosNf-00040s-11 for qemu-devel@nongnu.org; Thu, 04 Aug 2011 03:30:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QosNd-0003SB-Sx for qemu-devel@nongnu.org; Thu, 04 Aug 2011 03:30:10 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49242 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QosNd-0003Ri-JS for qemu-devel@nongnu.org; Thu, 04 Aug 2011 03:30:09 -0400 Message-ID: <4E3A4A80.5060800@suse.de> Date: Thu, 04 Aug 2011 09:30:08 +0200 From: Hannes Reinecke MIME-Version: 1.0 References: <1312376904-16115-1-git-send-email-armbru@redhat.com> <1312376904-16115-17-git-send-email-armbru@redhat.com> <4E3A38D1.8040802@suse.de> <4E3A405A.5050502@suse.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 16/45] scsi-disk: Track tray locked state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, quintela@redhat.com, stefano.stabellini@eu.citrix.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, amit.shah@redhat.com, hch@lst.de On 08/04/2011 09:25 AM, Markus Armbruster wrote: > Hannes Reinecke writes: > >> On 08/04/2011 08:39 AM, Markus Armbruster wrote: >>> Hannes Reinecke 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 >>>>> --- >>>>> 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? As said, it's a minor point. And as it's only a byte you are correct about a drop in the ocean. And Paolo promised to keep it up in a further patch series. So I'll=20 retract my objection. Cheers, Hannes --=20 Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)