Hi On Sun, Aug 28, 2022 at 3:19 AM Bin Meng <bmeng...@gmail.com> wrote:
> On Fri, Aug 26, 2022 at 9:23 PM Marc-André Lureau > <marcandre.lur...@gmail.com> wrote: > > > > Hi > > > > On Fri, Aug 26, 2022 at 5:16 PM Bin Meng <bmeng...@gmail.com> wrote: > >> > >> On Thu, Aug 25, 2022 at 3:59 PM Marc-André Lureau > >> <marcandre.lur...@redhat.com> wrote: > >> > > >> > Hi > >> > > >> > On Wed, Aug 24, 2022 at 1:43 PM Bin Meng <bmeng...@gmail.com> wrote: > >> > > > >> > > From: Xuzhou Cheng <xuzhou.ch...@windriver.com> > >> > > > >> > > The combination of GENERIC_WRITE and FILE_SHARE_READ options does > >> > > not allow the same file to be opened again by CreateFile() from > >> > > another QEMU process with the same options when the previous QEMU > >> > > process still holds the file handle openned. > >> > > >> > opened > >> > > >> > > > >> > > As per [1] we should add FILE_SHARE_WRITE to the share mode to allow > >> > > such use case. This change makes the behavior be consisten with the > >> > > POSIX platforms. > >> > > > >> > > >> > consistent > >> > > >> > > [1] > https://docs.microsoft.com/en-us/windows/win32/fileio/creating-and-opening-files > >> > > > >> > > Signed-off-by: Xuzhou Cheng <xuzhou.ch...@windriver.com> > >> > > Signed-off-by: Bin Meng <bin.m...@windriver.com> > >> > > --- > >> > > >> > > >> > What's the benefit to allow multiple processes write access to the > >> > same file? It seems it could easily lead to corruption or unexpected > >> > results. > >> > >> This was triggered by running the test_multifd_tcp_cancel() case on > >> windows, which cancels the migration, and launches another QEMU > >> process to migrate with the same file opened for write. Chances are > >> that the previous QEMU process does not quit before the new QEMU > >> process runs hence the new one still holds the file handle that does > >> not allow shared write permission then the new QEMU process will fail. > >> > > > > Thanks for the details, that's worth to add in commit message imho. > > Sure, I can add this in the commit message. > > > > > But can't we fix the test instead to use different paths? > > > > Yeah, the test case fix is here: > > https://lore.kernel.org/qemu-devel/20220824094029.1634519-42-bmeng...@gmail.com/ > > I think this patch is still needed as it makes the Windows char-file > behavior be consistent with the posix because there is no lock > mechanism in posix. > In this case, I would rather make posix consistent with the windows behaviour :) I am not sure how to proceed from there, but I would discard your windows patch for now.
