From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TF2Os-00083Y-LE for qemu-devel@nongnu.org; Fri, 21 Sep 2012 08:32:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TF2Om-0004yg-RX for qemu-devel@nongnu.org; Fri, 21 Sep 2012 08:32:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52063) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TF2Om-0004yc-Hg for qemu-devel@nongnu.org; Fri, 21 Sep 2012 08:32:00 -0400 Message-ID: <505C5E39.8060804@redhat.com> Date: Fri, 21 Sep 2012 14:31:53 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <71de241ac2e46041abdddd36683b2a1f82fb1e2c.1348157913.git.jcody@redhat.com> <505C2663.2030901@redhat.com> <505C28C1.2090609@redhat.com> <505C5AC0.5090500@redhat.com> In-Reply-To: <505C5AC0.5090500@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 15/19] block: raw-win32 driver reopen support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jcody@redhat.com Cc: Kevin Wolf , supriyak@linux.vnet.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org Il 21/09/2012 14:17, Jeff Cody ha scritto: > On 09/21/2012 04:43 AM, Paolo Bonzini wrote: >> Il 21/09/2012 10:33, Kevin Wolf ha scritto: >>>>> + /* could not reopen the file handle, so fall back to opening >>>>> + * new file (CreateFile) */ >>>>> + if (raw_s->hfile == INVALID_HANDLE_VALUE) { >>>>> + raw_s->hfile = CreateFile(state->bs->filename, access_flags, >>>>> + FILE_SHARE_READ, NULL, OPEN_EXISTING, >>>>> + overlapped, NULL); >>>>> + if (raw_s->hfile == INVALID_HANDLE_VALUE) { >>>>> + /* this could happen because the access_flags requested are >>>>> + * incompatible with the existing share mode of s->hfile, >>>>> + * so our only option now is to close s->hfile, and try again. >>>>> + * This could end badly */ >>>>> + CloseHandle(s->hfile); >>> How common is this case? >>> >>> We do have another option, namely not reopen at all and return an error. >>> Of course, this only makes sense if it doesn't mean that we almost never >>> succeed. >> >> Probably pretty common since we specify FILE_SHARE_READ for the sharing >> mode, meaning that "subsequent open operations on a file or device are >> only able to request read access". > > Yes, I think this is by far the most common case. Actually ReOpenFile probably only takes into account _other_ sharing modes, not the one for hFile, so it may even be unnecessary. But... >> I would change it to FILE_SHARE_READ|FILE_SHARE_WRITE and remove this code. > > I contemplated doing that, but I wasn't sure if there was any particular > reason it was originally done with FILE_SHARE_READ only in the first > place (security, etc..). I was hesitant to override that behaviour as > the new default under w32. Do we know if this is acceptable / safe? ... let's just make things work the same as in Unix. Paolo