From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UO7Id-0005Uv-5S for qemu-devel@nongnu.org; Fri, 05 Apr 2013 10:07:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UO7IV-0007sP-3Q for qemu-devel@nongnu.org; Fri, 05 Apr 2013 10:07:27 -0400 Received: from mail-qa0-f47.google.com ([209.85.216.47]:64254) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UO7IU-0007sF-Vx for qemu-devel@nongnu.org; Fri, 05 Apr 2013 10:07:19 -0400 Received: by mail-qa0-f47.google.com with SMTP id hu16so303471qab.6 for ; Fri, 05 Apr 2013 07:07:18 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <87mwtdkudb.fsf@blackfin.pond.sub.org> References: <1365155461-11072-1-git-send-email-stefanha@redhat.com> <87li8xqm9k.fsf@blackfin.pond.sub.org> <20130405121941.GB12305@stefanha-thinkpad.redhat.com> <87mwtdkudb.fsf@blackfin.pond.sub.org> Date: Fri, 5 Apr 2013 16:07:18 +0200 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH] ide: refuse WIN_READ_NATIVE_MAX on empty device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , qemu-devel , Stefan Hajnoczi On Fri, Apr 5, 2013 at 2:57 PM, Markus Armbruster wrote: > Stefan Hajnoczi writes: > >> On Fri, Apr 05, 2013 at 12:56:07PM +0200, Markus Armbruster wrote: >>> Stefan Hajnoczi writes: >>> >>> > What is the highest addressable sector on an empty CD-ROM? Nothing is >>> > addressable so produce an error. >>> > >>> > This patch prevents a divide-by-zero in ide_set_sector() since >>> > s->sectors and s->heads would be 0. Not to mention that a sector=-1 >>> > argument would be nonsense. >>> > >>> > Note that WIN_READ_NATIVE_MAX can be triggered using hdparm -N 1024 >>> > /dev/cdrom. The LBA bit will be set to 1 though, so the only easy way >>> > to go down the ide_set_sector() CHS code path which divides by zero is >>> > to comment out the s->select & 0x40 case for testing. >>> >>> Suggests you did that. >>> >>> Have you tried the reproducer with a physical drive? Does it fail the >>> command when empty, too? >> >> Believe it or not, I don't have access to an ATAPI CD-ROM drive. Would >> you be able to try out hdparm -N 1024 /dev/cdrom? >> >> Note that READ NATIVE MAX is optional, real drives may not implement it >> since it seems geared towards the Host Protected Area feature which >> makes no sense on CD-ROMs. (The idea is a reserved area on the disk >> where system data can be stored and the OS will not touch it.) >> >> Stefan > > # hdparm -N /dev/cdrom > > /dev/cdrom: > READ_NATIVE_MAX_ADDRESS failed: Input/output error > # hdparm -N 1024 /dev/cdrom > > /dev/cdrom: > setting max visible sectors to 1024 (temporary) > READ_NATIVE_MAX_ADDRESS failed: Input/output error > READ_NATIVE_MAX_ADDRESS failed: Input/output error > > Same with and without media. > > If the command makes no sense for CD-ROMs, and generally isn't > implemented by them, we should consider not implementing either, by > clearing its IDE_CD bit in ide_cmd_table. Thank you! You're getting the same result that we get in the guest. This looks good. There's no harm in supporting READ NATIVE MAX when the CD-ROM is inserted. It's basically another way of finding out the block device size. I also like this fix better than blacklisting the command since it now protects us in case HD or CFA devices ever have nb_sectors = 0 too. In other words, it's more defensive than just fixing CD-ROMs. Stefan