From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37905) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z6yoa-0000pH-LR for qemu-devel@nongnu.org; Mon, 22 Jun 2015 06:19:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z6yoZ-0003Yz-7v for qemu-devel@nongnu.org; Mon, 22 Jun 2015 06:18:56 -0400 Date: Mon, 22 Jun 2015 13:18:43 +0300 From: Dimitris Aragiorgis Message-ID: <20150622101843.GA16825@arr> References: <1432115859-11413-1-git-send-email-dimara@arrikto.com> <1432115859-11413-6-git-send-email-dimara@arrikto.com> <20150619123302.GK11189@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6TrnltStXW4iwmi0" Content-Disposition: inline In-Reply-To: <20150619123302.GK11189@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 5/5] raw-posix: Introduce hdev_is_sg() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org --6TrnltStXW4iwmi0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Stefan, Yes, you are right. Using realpath() is a workaround for supporting symlinks, as long as they point to a path starting with "/dev/sg". I will remove this reference in the revised version of this patch. However, it still holds that determining whether a filename is an SG device or not is based on textual (and hardcoded) examination, i.e., whether it starts with "/dev/sg". The point is that a device node is a device node no matter what its filename or its location in the fs tree is. A device node with major 1, minor 3 is the null device whether it's called /dev/null or not. The proposed patch allows the use of any device node which is capable of acting as a scsi-generic device; it checks for the required intrinsic property ("does this device support the SG ioctl?") versus trying to second-guess the nature of the device node by checking its filename. Similarly, the programming example from SG's documentation does not check the node of the device node at all: http://sg.danny.cz/sg/p/sg_v3_ho.html#pexample /* It is prudent to check we have a sg device by trying an ioctl */ if ((ioctl(sg_fd, SG_GET_VERSION_NUM, &k) < 0) || (k < 30000)) { printf("%s is not an sg device, or old sg driver\n", argv[1]); return 1; The above has the advantage that it works with device nodes of any name, and in any directory. So I suggest we go with the submitted patch taking into account Eric's proposal: The code first stat()s the given filename to ensure it is a character device node, and then it issues the SG_GET_VERSION_NUM and SG_GET_SCSI_ID ioctl()s. Thanks, dimara * Stefan Hajnoczi [2015-06-19 13:33:02 +0100]: > On Wed, May 20, 2015 at 12:57:39PM +0300, Dimitris Aragiorgis wrote: > > This is very fragile, e.g. it fails with symlinks or relative paths. >=20 > This is not true since realpath(3) is used to resolve symlinks and > product an absolute path. >=20 > Is this patch really necessary? --6TrnltStXW4iwmi0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAEBAgAGBQJVh+EDAAoJEHFDHex6CBG996AIALhZWnUq1E84s9RgafgKABWA I9JgJxI6WJMobRQTxz+0Ry0yZzGSUEv3K52/v6q6y5xtUaDUPWnGbhNSYIHefAoB mDEIYJQ6EHyrVabZ/mf4WyJbXb4Vki9LGzu4vJrabwMopIBLbIXIcUeItWdMH/rJ WIxysL+IO+ggur4+IsOMDoTn35T/29n6ahVvdo+c4DS0AyqsQUWA/EJZieN5uTja 9i1AbCrcoC4bVa5UUVXbL5yiSUFZp6Q8h+SN3v/g+hu4k4AbCYc8BN17/Ivh38Ew Q30ZNwkccI7MZi2Mj5AhvsZ4c5jfMm14q6+WiWJW7iSr2UEC/FRl7kQUxUSfMgo= =mFZN -----END PGP SIGNATURE----- --6TrnltStXW4iwmi0--