record/replay fixes, maybe for 8.2 or for post release stable?

[PATCH 10/11] chardev: force write all when recording replay logs

Posted by Alex Bennée 11 months, 3 weeks ago

This is mostly a problem within avocado as serial generally isn't busy
enough to overfill pipes. However the consequences of recording a
failed write will haunt us on replay when causing the log to go out of
sync.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2010
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 chardev/char.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/char.c b/chardev/char.c
index 996a024c7a..6e5b4d7345 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -171,7 +171,8 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
         return res;
     }
 
-    res = qemu_chr_write_buffer(s, buf, len, &offset, write_all);
+    res = qemu_chr_write_buffer(s, buf, len, &offset,
+                                replay_mode == REPLAY_MODE_RECORD ? true : write_all);
 
     if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
         replay_char_write_event_save(res, offset);
-- 
2.39.2

Re: [PATCH 10/11] chardev: force write all when recording replay logs

Posted by Pavel Dovgalyuk 11 months, 3 weeks ago

On 05.12.2023 23:41, Alex Bennée wrote:
> This is mostly a problem within avocado as serial generally isn't busy
> enough to overfill pipes. However the consequences of recording a
> failed write will haunt us on replay when causing the log to go out of
> sync.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2010
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>   chardev/char.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/chardev/char.c b/chardev/char.c
> index 996a024c7a..6e5b4d7345 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -171,7 +171,8 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
>           return res;
>       }
>   
> -    res = qemu_chr_write_buffer(s, buf, len, &offset, write_all);
> +    res = qemu_chr_write_buffer(s, buf, len, &offset,
> +                                replay_mode == REPLAY_MODE_RECORD ? true : write_all);
>   
>       if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
>           replay_char_write_event_save(res, offset);

Acked-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>

Re: [PATCH 10/11] chardev: force write all when recording replay logs

Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago

On 5/12/23 21:41, Alex Bennée wrote:
> This is mostly a problem within avocado as serial generally isn't busy
> enough to overfill pipes. However the consequences of recording a
> failed write will haunt us on replay when causing the log to go out of
> sync.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2010
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>   chardev/char.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/chardev/char.c b/chardev/char.c
> index 996a024c7a..6e5b4d7345 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -171,7 +171,8 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
>           return res;
>       }

Please add a comment explaining why in the code. Maybe simpler
using:

        if (replay_mode == REPLAY_MODE_RECORD) {
            /* $explanation */
            write_all = true;
        }

> -    res = qemu_chr_write_buffer(s, buf, len, &offset, write_all);
> +    res = qemu_chr_write_buffer(s, buf, len, &offset,
> +                                replay_mode == REPLAY_MODE_RECORD ? true : write_all);
With a comment:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>