On 10/03/2016 12:55, Pavel Dovgalyuk wrote:
> gdbstub which also acts as a backend is not recorded to allow controlling
> the replaying through gdb.
Perhaps the monitor too?
Overall the patch is nice and can definitely go in 2.6, but there are a
couple changes to do...
> @@ -245,6 +246,9 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
> *buf, int len)
> qemu_chr_fe_write_log(s, buf, ret);
> }
>
> + if (s->replay) {
> + replay_data_int(&ret);
> + }
I think this is wrong. The logic should be
if (replaying) {
read event(&ret);
assert(ret <= len);
len = ret;
}
qemu_mutex_lock(&s->chr_write_lock);
ret = s->chr_write(s, buf, len);
if (ret > 0) {
qemu_chr_fe_write_log(s, buf, ret);
}
qemu_mutex_unlock(&s->chr_write_lock);
if (recording) {
write event(ret);
}
> qemu_mutex_unlock(&s->chr_write_lock);
> return ret;
> }
> @@ -318,9 +322,19 @@ int qemu_chr_fe_read_all(CharDriverState *s, uint8_t
> *buf, int len)
>
> int qemu_chr_fe_ioctl(CharDriverState *s, int cmd, void *arg)
> {
> - if (!s->chr_ioctl)
> - return -ENOTSUP;
> - return s->chr_ioctl(s, cmd, arg);
> + int res;
> + if (!s->chr_ioctl) {
> + res = -ENOTSUP;
> + } else {
> + res = s->chr_ioctl(s, cmd, arg);
> + if (s->replay) {
> + fprintf(stderr,
> + "Replay: ioctl is not supported for serial devices
> yet\n");
> + exit(1);
Is it possible to print this warning just once per device and return
-ENOTSUP instead?
> +void replay_register_char_driver(CharDriverState *chr)
> +{
> + if (replay_mode == REPLAY_MODE_NONE) {
> + return;
> + }
> + char_drivers = g_realloc(char_drivers,
> + sizeof(*char_drivers) * (drivers_count + 1));
> + char_drivers[drivers_count++] = chr;
> +}
You need a way to unregister character drivers when they are
hot-unplugged, or at least you should block chardev-del if in record and
replay mode.
> + /* for int data */
> + EVENT_DATA_INT,
I think you should call the event EVENT_CHAR_WRITE (and perhaps rename
REPLAY_ASYNC_EVENT_CHAR to REPLAY_ASYNC_EVENT_CHAR_READ). And as
mentioned above, I think the load and save cases should be separated in
qemu-char.c, so I'd prefer to have a separate function to read and write
the event as well.
Thanks,
Paolo
