On Fri, Aug 26, 2022 at 9:23 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Fri, Aug 26, 2022 at 5:16 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> On Thu, Aug 25, 2022 at 3:59 PM Marc-André Lureau
>> <marcandre.lureau@redhat.com> wrote:
>> >
>> > Hi
>> >
>> > On Wed, Aug 24, 2022 at 1:43 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>> > >
>> > > From: Xuzhou Cheng <xuzhou.cheng@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.cheng@windriver.com>
>> > > Signed-off-by: Bin Meng <bin.meng@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:
20220824094029.1634519-42-bmeng.cn@gmail.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/qemu-devel/20220824094029.1634519-42-bmeng.cn@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.