[RFC,v8,04/21] replay: internal functions for replay log

This patch adds functions to perform read and write operations
with replay log.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 replay/Makefile.objs     |    1 
 replay/replay-internal.c |  141 ++++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h |   50 ++++++++++++++++
 3 files changed, 192 insertions(+), 0 deletions(-)
 create mode 100755 replay/replay-internal.c
 create mode 100755 replay/replay-internal.h

On 22/01/2015 09:51, Pavel Dovgalyuk wrote:
> This patch adds functions to perform read and write operations
> with replay log.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  replay/Makefile.objs     |    1 
>  replay/replay-internal.c |  141 ++++++++++++++++++++++++++++++++++++++++++++++
>  replay/replay-internal.h |   50 ++++++++++++++++
>  3 files changed, 192 insertions(+), 0 deletions(-)
>  create mode 100755 replay/replay-internal.c
>  create mode 100755 replay/replay-internal.h
> 
> diff --git a/replay/Makefile.objs b/replay/Makefile.objs
> index 7ea860f..1148f45 100755
> --- a/replay/Makefile.objs
> +++ b/replay/Makefile.objs
> @@ -1 +1,2 @@
>  obj-y += replay.o
> +obj-y += replay-internal.o
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> new file mode 100755
> index 0000000..3865fa5
> --- /dev/null
> +++ b/replay/replay-internal.c
> @@ -0,0 +1,141 @@
> +/*
> + * replay-internal.c
> + *
> + * Copyright (c) 2010-2015 Institute for System Programming
> + *                         of the Russian Academy of Sciences.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "replay-internal.h"
> +
> +unsigned int replay_data_kind = -1;
> +unsigned int replay_has_unread_data;
> +
> +/* File for replay writing */
> +FILE *replay_file;
> +
> +void replay_put_byte(uint8_t byte)
> +{
> +    if (replay_file) {
> +        fwrite(&byte, sizeof(byte), 1, replay_file);

putc(byte, replay_file);

> +    }
> +}
> +
> +void replay_put_event(uint8_t event)
> +{
> +    replay_put_byte(event);
> +}
> +
> +
> +void replay_put_word(uint16_t word)
> +{
> +    if (replay_file) {
> +        fwrite(&word, sizeof(word), 1, replay_file);

Let's make this use a guaranteed endianness.  QEMU usually uses big
endian.  So either use putc, or use cpu_to_be16 and fwrite.

> +    }
> +}
> +
> +void replay_put_dword(uint32_t dword)
> +{
> +    if (replay_file) {
> +        fwrite(&dword, sizeof(dword), 1, replay_file);

Same here.

> +    }
> +}
> +
> +void replay_put_qword(int64_t qword)
> +{
> +    if (replay_file) {
> +        fwrite(&qword, sizeof(qword), 1, replay_file);

And same here.

> +    }
> +}
> +
> +void replay_put_array(const uint8_t *buf, size_t size)
> +{
> +    if (replay_file) {
> +        fwrite(&size, sizeof(size), 1, replay_file);

replay_put_dword (a limit of 2^32 elements should not be a problem :)).

> +        fwrite(buf, 1, size, replay_file);
> +    }
> +}
> +
> +uint8_t replay_get_byte(void)
> +{
> +    uint8_t byte;
> +    if (replay_file) {
> +        fread(&byte, sizeof(byte), 1, replay_file);

byte = getc(replay_file)

> +    }
> +    return byte;
> +}
> +
> +uint16_t replay_get_word(void)
> +{
> +    uint16_t word;
> +    if (replay_file) {
> +        fread(&word, sizeof(word), 1, replay_file);

Same comment about endianness.

> +    }
> +
> +    return word;
> +}
> +
> +uint32_t replay_get_dword(void)
> +{
> +    uint32_t dword;
> +    if (replay_file) {
> +        fread(&dword, sizeof(dword), 1, replay_file);
> +    }
> +
> +    return dword;
> +}
> +
> +int64_t replay_get_qword(void)
> +{
> +    int64_t qword;
> +    if (replay_file) {
> +        fread(&qword, sizeof(qword), 1, replay_file);
> +    }
> +
> +    return qword;
> +}
> +
> +void replay_get_array(uint8_t *buf, size_t *size)
> +{
> +    if (replay_file) {
> +        fread(size, sizeof(*size), 1, replay_file);
> +        fread(buf, 1, *size, replay_file);
> +    }
> +}
> +
> +void replay_get_array_alloc(uint8_t **buf, size_t *size)
> +{
> +    if (replay_file) {
> +        fread(size, sizeof(*size), 1, replay_file);
> +        *buf = g_malloc(*size);
> +        fread(*buf, 1, *size, replay_file);
> +    }
> +}
> +
> +void replay_check_error(void)

Could this be static? (I haven't checked).

> +{
> +    if (replay_file) {
> +        if (feof(replay_file)) {
> +            fprintf(stderr, "replay file is over\n");
> +            exit(1);

Perhaps qemu_system_vmstop_request_prepare +
qemu_system_vmstop_request(RUN_STATE_PAUSED) instead of exit?  Those two
functions are thread-safe.


> +        } else if (ferror(replay_file)) {
> +            fprintf(stderr, "replay file is over or something goes wrong\n");

Same here (RUN_STATE_INTERNAL_ERROR?).

> +            exit(1);
> +        }
> +    }
> +}
> +
> +void replay_fetch_data_kind(void)
> +{
> +    if (replay_file) {
> +        if (!replay_has_unread_data) {
> +            replay_data_kind = replay_get_byte();
> +            replay_check_error();
> +            replay_has_unread_data = 1;
> +        }
> +    }
> +}
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> new file mode 100755
> index 0000000..dad5bc8
> --- /dev/null
> +++ b/replay/replay-internal.h
> @@ -0,0 +1,50 @@
> +#ifndef REPLAY_INTERNAL_H
> +#define REPLAY_INTERNAL_H
> +
> +/*
> + * replay-internal.h
> + *
> + * Copyright (c) 2010-2015 Institute for System Programming
> + *                         of the Russian Academy of Sciences.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <stdio.h>
> +
> +extern unsigned int replay_data_kind;
> +extern unsigned int replay_has_unread_data;
> +
> +/* File for replay writing */
> +extern FILE *replay_file;
> +
> +void replay_put_byte(uint8_t byte);
> +void replay_put_event(uint8_t event);
> +void replay_put_word(uint16_t word);
> +void replay_put_dword(uint32_t dword);
> +void replay_put_qword(int64_t qword);
> +void replay_put_array(const uint8_t *buf, size_t size);
> +
> +uint8_t replay_get_byte(void);
> +uint16_t replay_get_word(void);
> +uint32_t replay_get_dword(void);
> +int64_t replay_get_qword(void);
> +void replay_get_array(uint8_t *buf, size_t *size);
> +void replay_get_array_alloc(uint8_t **buf, size_t *size);
> +
> +/*! Checks error status of the file. */
> +void replay_check_error(void);
> +
> +/*! Reads data type from the file and stores it in the
> +    replay_data_kind variable. */
> +void replay_fetch_data_kind(void);
> +
> +/*! Saves queued events (like instructions and sound). */
> +void replay_save_instructions(void);
> +/*! Checks that the next data is corresponding to the desired kind.
> +    Terminates the program in case of error. */
> +void validate_data_kind(int kind);

Please move the last two prototypes to the patches that introduce the
functions.

> +
> +#endif
>

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 22/01/2015 09:51, Pavel Dovgalyuk wrote:
> > This patch adds functions to perform read and write operations
> > with replay log.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> > +void replay_check_error(void)
> 
> Could this be static? (I haven't checked).

No, because it is used from several replay files.

> > +{
> > +    if (replay_file) {
> > +        if (feof(replay_file)) {
> > +            fprintf(stderr, "replay file is over\n");
> > +            exit(1);
> 
> Perhaps qemu_system_vmstop_request_prepare +
> qemu_system_vmstop_request(RUN_STATE_PAUSED) instead of exit?  Those two
> functions are thread-safe.

There is no need to stop when replay file is over (because we cannot replay more).
Should we send shutdown request instead?


Pavel Dovgalyuk

On 30/01/2015 13:56, Pavel Dovgaluk wrote:
> > Could this be static? (I haven't checked).
> 
> No, because it is used from several replay files.

I wonder if that's a layering violation.

> > Perhaps qemu_system_vmstop_request_prepare +
> > qemu_system_vmstop_request(RUN_STATE_PAUSED) instead of exit?  Those two
> > functions are thread-safe.
>
> There is no need to stop when replay file is over (because we cannot replay more).
> Should we send shutdown request instead?

I thought about it.  I think no, because shutdown is irreversible (see
runstate_needs_reset).  Just pausing seemed to be the right compromise,
and then the next "cont" can run the VM out of replay mode.

Paolo

I believe thats what we concluded too
Cheers
Mark.

> On 30 Jan 2015, at 14:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> 
> 
> On 30/01/2015 13:56, Pavel Dovgaluk wrote:
>>> Could this be static? (I haven't checked).
>> 
>> No, because it is used from several replay files.
> 
> I wonder if that's a layering violation.
> 
>>> Perhaps qemu_system_vmstop_request_prepare +
>>> qemu_system_vmstop_request(RUN_STATE_PAUSED) instead of exit?  Those two
>>> functions are thread-safe.
>> 
>> There is no need to stop when replay file is over (because we cannot replay more).
>> Should we send shutdown request instead?
> 
> I thought about it.  I think no, because shutdown is irreversible (see
> runstate_needs_reset).  Just pausing seemed to be the right compromise,
> and then the next "cont" can run the VM out of replay mode.
> 
> Paolo


	 +44 (0)20 7100 3485 x 210
 +33 (0)5 33 52 01 77x 210

	+33 (0)603762104
	mark.burton