Hi On Sun, Sep 25, 2022 at 4:35 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 opened. > > 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 old one still holds the file handle that does > not allow shared write permission then the new QEMU process will fail. > > There is another test case boot-serial-test that triggers the same > issue. The qtest executable created a serial chardev file to be > passed to the QEMU executable. The serial file was created by > g_file_open_tmp(), which internally opens the file with > FILE_SHARE_WRITE security attribute, and based on [1], there is > only one case that allows the first call to CreateFile() with > GENERIC_READ & FILE_SHARE_WRITE, and second call to CreateFile() > with GENERIC_WRITE & FILE_SHARE_READ. All other combinations > require FILE_SHARE_WRITE in the second call. But there is no way > for the second call (in this case the QEMU executable) to know > what combination was passed to the first call, so we will have to > add FILE_SHARE_WRITE to the second call. > > For both scenarios we should add FILE_SHARE_WRITE in the chardev > file backend driver. This change also makes the behavior to be > consistent with the POSIX platforms. > It seems to me the tests should be fixed instead. I thought you fixed the first case. For the second case, why not close the file before starting qemu? If you have issues, I will take a deeper look. > > [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> > --- > > Changes in v3: > - Add another case "boot-serial-test" to justify the change > > Changes in v2: > - Update commit message to include the use case why we should set > FILE_SHARE_WRITE when opening the file for win32 > > chardev/char-file.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/chardev/char-file.c b/chardev/char-file.c > index 2fd80707e5..66385211eb 100644 > --- a/chardev/char-file.c > +++ b/chardev/char-file.c > @@ -60,8 +60,8 @@ static void qmp_chardev_open_file(Chardev *chr, > flags = CREATE_ALWAYS; > } > > - out = CreateFile(file->out, accessmode, FILE_SHARE_READ, NULL, flags, > - FILE_ATTRIBUTE_NORMAL, NULL); > + out = CreateFile(file->out, accessmode, FILE_SHARE_READ | > FILE_SHARE_WRITE, > + NULL, flags, FILE_ATTRIBUTE_NORMAL, NULL); > if (out == INVALID_HANDLE_VALUE) { > error_setg(errp, "open %s failed", file->out); > return; > -- > 2.34.1 > > > -- Marc-André Lureau
