[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/5] replay: character devices
From: |
Pavel Dovgalyuk |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/5] replay: character devices |
Date: |
Fri, 11 Mar 2016 09:19:42 +0300 |
> From: Paolo Bonzini [mailto:address@hidden
> 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?
Right. I'll check that it works.
> 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;
In this case return value in record and replay modes may differ
and the behavior of caller won't be deterministic.
E.g.,
static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
void *opaque)
{
...
ret = qemu_chr_fe_write(s->chr, s->tx_fifo, s->tx_count);
s->tx_count -= ret;
memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_count);
...
}
Pavel Dovgalyuk
[Qemu-devel] [PATCH v4 4/5] block: add flush callback, Pavel Dovgalyuk, 2016/03/10
[Qemu-devel] [PATCH v4 5/5] replay: introduce block devices record/replay, Pavel Dovgalyuk, 2016/03/10