From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33653) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKnSl-0007pO-LD for qemu-devel@nongnu.org; Sat, 14 Sep 2013 06:52:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKnSa-0001jo-C3 for qemu-devel@nongnu.org; Sat, 14 Sep 2013 06:52:27 -0400 Received: from mail-wi0-f174.google.com ([209.85.212.174]:39673) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKnSa-0001fE-6p for qemu-devel@nongnu.org; Sat, 14 Sep 2013 06:52:16 -0400 Received: by mail-wi0-f174.google.com with SMTP id hj3so1745693wib.1 for ; Sat, 14 Sep 2013 03:52:14 -0700 (PDT) Message-ID: <52343FC3.4010208@6wind.com> Date: Sat, 14 Sep 2013 12:51:47 +0200 From: Damien Millescamps MIME-Version: 1.0 References: <1379010228-15324-1-git-send-email-damien.millescamps@6wind.com> <52342E0E.3010700@msgid.tls.msk.ru> In-Reply-To: <52342E0E.3010700@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] ivshmem: allow the sharing of hugepages List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org On 09/14/2013 11:36 AM, Michael Tokarev wrote: > That to say, this is not a _definition_ of a shared memory object, it > is just > a suggested name syntax, suggested purely for portability. In other > words, > there may be other acceptable syntaxes for it. You are right, the definition can be found in shm_open(P), and reads: If name does not begin with the slash character, the effect is implementation-defined. The interpretation of slash characters other than the leading slash character in name is implementation-defined. The "implementation-defined" found in glibc is as follow: The leading '/', if present, is removed. and '/dev/shm/' is prepended to the resulting name before calling open(). (found in sysdeps/posix/shm_open.c around line 57 depending on the version). Note that a workaround in my case is to give a name in the form: /../../path/to/file" ... > So as the result, I'm not sure this approach is valid. Maybe we should > always try shared first and create-new second? I dunno. This is probably a better approach, yes. cf next paragraph. > Note that whole thing - using shared memory object like this - may lead > to surprizes at least, -- users who previously expected one behavour now > see different behavour. Most likely the old behavour wasn't correct. By first trying to open with shm_open, and only when it fails with open, the behavior should stay the same. Because according to glibc implementation, if "folder" exists in /dev/shm, a name like "/folder/shared_mem" should work, but will trigger a false positive with the checks I added. Note that I am not sure anyone uses this syntax, but still, this is a false positive. I'll change that. > At least this should be documented somewhere in user-visible part of > ivshmem, so users will have an ides when objects will be shared and > when truncated. I will add a paragraph in docs/specs/ivshmem_device_spec.txt for that. Thanks for mentioning it. > It is a somewhat minor nitpick, but it'd be not nice to spread such tests > (for NULLness) where the object can't be NULL and to confuse readers. agreed. > Thanks, Thanks for your review, that was helpful. I'll send a reworked patch, probably on Monday. -- Damien Millescamps