Re: [PATCH v2 08/21] tests: test-replication disable /replication/secondary/* on msys2/mingw.

2020-09-09 Thread Daniel P . Berrangé
On Wed, Sep 09, 2020 at 05:46:04PM +0800, Yonggang Luo wrote:
> They cause failure on msys2/mingw, that's because file-win32.c not implement
> .bdrv_reopen_prepare/commit/abort yet.
> 
> > $ ./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.
> 
>  luoyonggang: That might be missing functionality in 
> block/file-win32.c.
> * davidgiluk yawns and looks up
>  luoyonggang: The block/file-posix.c block driver supports 
> .bdrv_reopen_*()
> while block/file-win32.c does not. It's probably because no one has tried to 
> implement it.
>  OK, I got the direction,
>  Just need implement .bdrv_reopen_*() functions in file-win32.c


We don't need to add IRC transscripts into the commit message. It
is sufficient to note that .bdrv_reopen* are not implemented on
in block/file-win32.c, which you already did at the start.

> 
> Signed-off-by: Yonggang Luo 
> ---
>  tests/test-replication.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/test-replication.c b/tests/test-replication.c
> index e7cbd6b144..b067240add 100644
> --- a/tests/test-replication.c
> +++ b/tests/test-replication.c
> @@ -392,6 +392,7 @@ static void test_secondary_write(void)
>  teardown_secondary();
>  }
>  
> +#ifndef _WIN32
>  static void test_secondary_start(void)
>  {
>  BlockBackend *top_blk, *local_blk;
> @@ -546,6 +547,7 @@ static void test_secondary_get_error_all(void)
>  
>  teardown_secondary();
>  }
> +#endif
>  
>  static void sigabrt_handler(int signo)
>  {
> @@ -597,6 +599,7 @@ int main(int argc, char **argv)
>  /* Secondary */
>  g_test_add_func("/replication/secondary/read",  test_secondary_read);
>  g_test_add_func("/replication/secondary/write", test_secondary_write);
> +#ifndef _WIN32
>  g_test_add_func("/replication/secondary/start", test_secondary_start);
>  g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
>  g_test_add_func("/replication/secondary/continuous_replication",
> @@ -605,6 +608,7 @@ int main(int argc, char **argv)
>  test_secondary_do_checkpoint);
>  g_test_add_func("/replication/secondary/get_error_all",
>  test_secondary_get_error_all);
> +#endif
>  
>  ret = g_test_run();

With the commit msg trimmed

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




[PATCH v2 08/21] tests: test-replication disable /replication/secondary/* on msys2/mingw.

2020-09-09 Thread Yonggang Luo
They cause failure on msys2/mingw, that's because file-win32.c not implement
.bdrv_reopen_prepare/commit/abort yet.

> $ ./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.

 luoyonggang: That might be missing functionality in 
block/file-win32.c.
* davidgiluk yawns and looks up
 luoyonggang: The block/file-posix.c block driver supports 
.bdrv_reopen_*()
while block/file-win32.c does not. It's probably because no one has tried to 
implement it.
 OK, I got the direction,
 Just need implement .bdrv_reopen_*() functions in file-win32.c

Signed-off-by: Yonggang Luo 
---
 tests/test-replication.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/test-replication.c b/tests/test-replication.c
index e7cbd6b144..b067240add 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -392,6 +392,7 @@ static void test_secondary_write(void)
 teardown_secondary();
 }
 
+#ifndef _WIN32
 static void test_secondary_start(void)
 {
 BlockBackend *top_blk, *local_blk;
@@ -546,6 +547,7 @@ static void test_secondary_get_error_all(void)
 
 teardown_secondary();
 }
+#endif
 
 static void sigabrt_handler(int signo)
 {
@@ -597,6 +599,7 @@ int main(int argc, char **argv)
 /* Secondary */
 g_test_add_func("/replication/secondary/read",  test_secondary_read);
 g_test_add_func("/replication/secondary/write", test_secondary_write);
+#ifndef _WIN32
 g_test_add_func("/replication/secondary/start", test_secondary_start);
 g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
 g_test_add_func("/replication/secondary/continuous_replication",
@@ -605,6 +608,7 @@ int main(int argc, char **argv)
 test_secondary_do_checkpoint);
 g_test_add_func("/replication/secondary/get_error_all",
 test_secondary_get_error_all);
+#endif
 
 ret = g_test_run();
 
-- 
2.28.0.windows.1