Re: make -i check resut for msys2

[Kevin Wolf <kwolf@redhat.com>]

Am 05.09.2020 um 08:21 hat 罗勇刚(Yonggang Luo) geschrieben:
> On Fri, Sep 4, 2020 at 4:51 PM Kevin Wolf <kwolf@redhat.com> wrote:
> 
> > Am 04.09.2020 um 08:03 hat Thomas Huth geschrieben:
> > > On 04/09/2020 00.53, 罗勇刚(Yonggang Luo) wrote:
> > > >
> > > >
> > > > On Thu, Sep 3, 2020 at 10:33 PM Thomas Huth <thuth@redhat.com
> > > > <mailto:thuth@redhat.com>> wrote:
> > > >
> > > >     On 03/09/2020 11.18, 罗勇刚(Yonggang Luo) wrote:
> > > >     [...]
> > > >     >   TEST    check-unit: tests/test-replication.exe
> > > >     > **
> > > >     > ERROR:C:/work/xemu/qemu/tests/test-replication.c:136:make_temp:
> > > >     > assertion failed: (fd >= 0)
> > > >     > ERROR test-replication.exe - Bail out!
> > > >     > ERROR:C:/work/xemu/qemu/tests/test-replication.c:136:make_temp:
> > > >     > assertion failed: (fd >= 0)
> > > >
> > > >     At least this one should be easy to fix: The test uses /tmp as
> > > >     hard-coded directory for temporary files. I think it should use
> > > >     g_get_tmp_dir() from glib to get that directory instead.
> > > >
> > > >      Thomas
> > > >
> > > > After fixes tmp path, how to fixes following error:
> > > > $ tests/test-replication.exe
> >
> > > >
> > > >
> > > >
> > > > # random seed: R02Sdf2e4ffc0e6fbe96624598386b538927
> > > > 1..13
> > > > # Start of replication tests
> > > > # Start of primary tests
> > > > Unexpected error in bdrv_open_inherit() at ../block.c:3456:
> > > > Block protocol 'file' doesn't support the option 'locking'
> > >
> > > Not sure ... as a temporary test, try to remove the "locking=off"
> > > strings from the test. If it then works, it might be worth discussing
> > > with the block layer folks how to handle this test on Windows in the
> > > best way. If it still does not work, it's maybe simply not worth the
> > > effort to try to get this test running on Windows - and thus mark it
> > > with CONFIG_POSIX in the Makefile / meson.build.
> >
> > This is a bug in file-win32. It reads "locking" from the options QDict,
> > but doesn't delete it from it.
> >
> > Does the following help? (Only compile-tested.)
> >
> > If it works for you, I'll send it as a proper patch.
> >
> > Kevin
> >
> > diff --git a/block/file-win32.c b/block/file-win32.c
> > index ab69bd811a..e2900c3a51 100644
> > --- a/block/file-win32.c
> > +++ b/block/file-win32.c
> > @@ -299,6 +299,11 @@ static QemuOptsList raw_runtime_opts = {
> >              .type = QEMU_OPT_STRING,
> >              .help = "host AIO implementation (threads, native)",
> >          },
> > +        {
> > +            .name = "locking",
> > +            .type = QEMU_OPT_STRING,
> > +            .help = "file locking mode (on/off/auto, default: auto)",
> > +        },
> >          { /* end of list */ }
> >      },
> >  };
> > @@ -333,6 +338,7 @@ static int raw_open(BlockDriverState *bs, QDict
> > *options, int flags,
> >      Error *local_err = NULL;
> >      const char *filename;
> >      bool use_aio;
> > +    OnOffAuto locking;
> >      int ret;
> >
> >      s->type = FTYPE_FILE;
> > @@ -343,10 +349,24 @@ static int raw_open(BlockDriverState *bs, QDict
> > *options, int flags,
> >          goto fail;
> >      }
> >
> > -    if (qdict_get_try_bool(options, "locking", false)) {
> > +    locking = qapi_enum_parse(&OnOffAuto_lookup,
> > +                              qemu_opt_get(opts, "locking"),
> > +                              ON_OFF_AUTO_AUTO, &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        ret = -EINVAL;
> > +        goto fail;
> > +    }
> > +    switch (locking) {
> > +    case ON_OFF_AUTO_ON:
> >          error_setg(errp, "locking=on is not supported on Windows");
> >          ret = -EINVAL;
> >          goto fail;
> > +    case ON_OFF_AUTO_OFF:
> > +    case ON_OFF_AUTO_AUTO:
> > +        break;
> > +    default:
> > +        g_assert_not_reached();
> >      }
> >
> >      filename = qemu_opt_get(opts, "filename");
> >
> Partial error fixed, new error are coming:

Good, I'll send the patch then.

> $ ./tests/test-replication.exe
> # random seed: R02S3f4d1c01af2b0a046990e0235c481faf
> 1..13
> # Start of replication tests
> # Start of primary tests
> ok 1 /replication/primary/read
> ok 2 /replication/primary/write
> ok 3 /replication/primary/start
> ok 4 /replication/primary/stop
> ok 5 /replication/primary/do_checkpoint
> ok 6 /replication/primary/get_error_all
> # End of primary tests
> # Start of secondary tests
> ok 7 /replication/secondary/read
> ok 8 /replication/secondary/write
> Unexpected error in bdrv_reopen_prepare() at ../block.c:4191:
> Block format 'file' used by node '#block4287' does not support reopening
> files

Can you try to find out what reopen this is?

I assume it's for switching between read-write and read-only. In this
case an implementation of .bdrv_reopen_prepare/commit/abort that can do
this switch is required.

This is more serious development work, so I can't propose a quick fix.

Alternatively, we could just declare replication unsupported on Windows
and let configure make sure that CONFIG_REPLICATION is never set for it.

Kevin