From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41488) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0VwM-0008Fb-P0 for qemu-devel@nongnu.org; Fri, 27 Jun 2014 09:11:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X0VwD-0001oz-70 for qemu-devel@nongnu.org; Fri, 27 Jun 2014 09:11:42 -0400 Received: from e06smtp13.uk.ibm.com ([195.75.94.109]:58919) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0VwC-0001op-Ss for qemu-devel@nongnu.org; Fri, 27 Jun 2014 09:11:33 -0400 Received: from /spool/local by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 27 Jun 2014 14:11:31 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 007EE17D804E for ; Fri, 27 Jun 2014 14:12:58 +0100 (BST) Received: from d06av09.portsmouth.uk.ibm.com (d06av09.portsmouth.uk.ibm.com [9.149.37.250]) by b06cxnps3074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s5RDBTKk25296972 for ; Fri, 27 Jun 2014 13:11:29 GMT Received: from d06av09.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av09.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s5RDBSOF030049 for ; Fri, 27 Jun 2014 07:11:29 -0600 Message-ID: <53AD6D80.7000607@de.ibm.com> Date: Fri, 27 Jun 2014 15:11:28 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1403868326-7718-1-git-send-email-cornelia.huck@de.ibm.com> <1403868326-7718-4-git-send-email-cornelia.huck@de.ibm.com> <53AD5947.2040704@suse.de> <53AD5B9D.7030400@de.ibm.com> <53AD6BE4.9000200@linux.vnet.ibm.com> In-Reply-To: <53AD6BE4.9000200@linux.vnet.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 03/10] pc-bios/s390-ccw: handle different sector sizes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Eugene \"jno\" Dvurechenski" , Alexander Graf , Cornelia Huck , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, jfrei@linux.vnet.ibm.com, aliguori@amazon.com On 27/06/14 15:04, Eugene "jno" Dvurechenski wrote: > > > On 06/27/2014 03:55 PM, Christian Borntraeger wrote: >>>> - const int max_entries = (SECTOR_SIZE / sizeof(ScsiBlockPtr)); >>>> + const int max_entries = (MAX_SECTOR_SIZE / sizeof(ScsiBlockPtr)); >>> >>> Is this really safe to increase? Doesn't max_entries depend on the real sector size? >> >> I think this is now covered by this if statement: >> if (bprs[i].blockct == 0 && unused_space(&bprs[i + 1], >> sizeof(ScsiBlockPtr))) { >> >> which was introduced by commit c77cd87cf54f003748f29c14ea1ddaecfc5c653f (pc-bios/s390-ccw: fix for fragmented SCSI bootmap). >> >> So strictly speaking this if statement might not be needed any more: >> if (i == (max_entries - 1)) { >> >> Eugene, can you confirm? If yes we could add this patch later on as a cleanup: > > I'd preserve both checks. > In theory, we may catch a table that consumes all scratch space and > leave no unused entry. > > Plus, this check for zero counter and last entry is for "continuation" > pointer, not for end-of-table by itself. > > I think now, this code may need even few more checks to cover more cases... > Ok. That means, that this patch as is, doesnt make anything worse. Correct? I am expecting more fixes and cleanups for the bios code anyway, so as long as we dont add a regression here this should be good to go as it makes the whole code more flexible. Christian