From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58867) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Poa-0007H3-U7 for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:20:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8PoW-0004HB-Te for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:20:52 -0400 Date: Fri, 26 Jun 2015 10:20:45 +0100 From: Stefan Hajnoczi Message-ID: <20150626092045.GD15457@stefanha-thinkpad.redhat.com> References: <3B3C7431-1003-4AAA-90AF-0E1A154DFBE2@gmail.com> <5589A026.5020800@redhat.com> <455CF92B-58FA-446D-9EB0-42661C23B739@gmail.com> <87pp4kxos3.fsf@blackfin.pond.sub.org> <7B989F99-B95C-4D53-AA2D-29B87634D2BB@gmail.com> <558C22BD.1020404@redhat.com> <558C2852.3050102@redhat.com> <558C295C.2030108@redhat.com> <558C3812.7060202@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="4zI0WCX1RcnW9Hbu" Content-Disposition: inline In-Reply-To: <558C3812.7060202@redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] block.c: fix real cdrom detection List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: Kevin Wolf , Peter Maydell , Qemu-block , qemu-devel qemu-devel , Markus Armbruster , Programmingkid , Paolo Bonzini --4zI0WCX1RcnW9Hbu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 25, 2015 at 07:19:14PM +0200, Laurent Vivier wrote: >=20 >=20 > On 25/06/2015 18:16, Paolo Bonzini wrote: > >=20 > >=20 > > On 25/06/2015 18:12, Laurent Vivier wrote: > >> > >> > >> On 25/06/2015 17:48, Paolo Bonzini wrote: > >>> > >>> On 25/06/2015 17:32, Programmingkid wrote: > >>>>> I think we are going to have to agree to disagree. I have never > >>>>> used the /dev/sr(0 | 1) devices and don't see how they would be > >>>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) > >>>>> devices *should* be handled by this patch? > >>>> > >>>> Thinking about your question some more, I see what you mean. On Linux > >>>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom > >>>> link refers to the /dev/sr0 device file. So if you just use > >>>> /dev/cdrom, you are good. > >>> > >>> Well, that's not how things work. > >>> > >>> If you do things like that, you end up with a bunch of hacks, not wit= h a > >>> decent piece of software. > >>> > >>> There is support for CD-ROM passthrough on Linux and FreeBSD in > >>> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS= X > >>> as well. > >>> > >> > >> In fact, programmingkid, you should fix it in hdev_open() where there = is > >> already a #if __APPLE__ . > >> > >> Paolo, I agree with you but : > >> > >> hdev_open() > >> > >> #if defined(__linux__) > >> { > >> char resolved_path[ MAXPATHLEN ], *temp; > >> > >> temp =3D realpath(filename, resolved_path); > >> if (temp && strstart(temp, "/dev/sg", NULL)) { > >> bs->sg =3D 1; > >> } > >> #endif > >> > >> I'm wondering who had this strange idea... :) > >=20 > > I was very scared to type "git blame" here. :) But the question is also >=20 > http://geek-and-poke.com/2013/11/24/simply-explained >=20 > BTW, it is a legacy from 2006: >=20 > 19cb373 better support of host drives >=20 > coming from MacOS X (again!): >=20 > 3b0d4f6 OS X: support for the built in CD-ROM drive (Mike Kronenberg) >=20 > > where to put the checks. Putting it at a random place in block.c is not > > a good idea. > >=20 > > But yes, this is also bad. It should use stat and check the major/minor > > numbers. >=20 > Yes, we should check if major is SCSI_GENERIC_MAJOR (21) (on linux). That would be too specific since there are other drivers that support SG ioctls, like block/bsg.c. > We can also try to send an SG command like in cdrom_probe_device(). > Something like in scsi_generic_realize(): >=20 > rc =3D blk_ioctl(s->conf.blk, SG_GET_VERSION_NUM, &sg_version); > if (rc < 0) { > error_setg(errp, "cannot get SG_IO version number: %s. " > "Is this a SCSI device?", > strerror(-rc)); > return; > } That was recently done in: commit 3307ed7b3fac5ba99eb3b84904b0b7cdc3592a61 Author: Dimitris Aragiorgis Date: Tue Jun 23 13:45:00 2015 +0300 raw-posix: Introduce hdev_is_sg() --4zI0WCX1RcnW9Hbu Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVjRltAAoJEJykq7OBq3PIihAH/R5wvyQKEVh5Wn23kdQhYF/A Rg52RhpvMAwLfM5DNZHByntR5q11sryDFNw9bIzgRYypg1VUnz0Vl8ApdCdARDNW vQSh92VJRYDAuU042k2gpmgU6Zy1F/MetMwyTX97vNofLNRfNaqYk/NgWK/B4+kQ ui28SRMZWZ0/CSUSA3fUzpogNn0bu5Kk8aXf81W3drCcomreSuzFlmzFKgFPfFV5 aojJ3fQSLHsK/5r7ERRJp4Eq8KUiAF9OAr5bX2vqc9Vp7fbcq4gvd7gXV0J53bzw 4fnBB0QWODRpPcn1bLsZc1lvbEn83szGz+cw//tikxs0TCkL4s18hagLigVF6Nk= =e2Je -----END PGP SIGNATURE----- --4zI0WCX1RcnW9Hbu--