[Qemu-devel] [PATCH] migration: fix saving normal page even if it's been compressed

[guangrong . xiao]

From: Xiao Guangrong 

Fix the bug introduced by da3f56cb2e767016 (migration: remove
ram_save_compressed_page()), It should be 'return' rather than
'res'

Sorry for this stupid mistake :(

Signed-off-by: Xiao Guangrong 
---
 migration/ram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 01cc815410..699546cc43 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1490,7 +1490,7 @@ static int ram_save_target_page(RAMState *rs, 
PageSearchStatus *pss,
  * CPU resource.
  */
 if (block == rs->last_sent_block && save_page_use_compression(rs)) {
-res = compress_page_with_multi_thread(rs, block, offset);
+return compress_page_with_multi_thread(rs, block, offset);
 }
 
 return ram_save_page(rs, pss, last_stage);
-- 
2.14.3

[Qemu-devel] Fwd: [RFC PATCH 00/17] reverse debugging

[Ciro Santilli]

Forgetting about debugging, I belive there is a deadlock in the replay
at 63d426dfa4fbfac3d50cda3f553cd975de2b85ea , but it is rare.

I have only reproduced it on ARM so far, and I haven't checked pre-patch.

The setup is 
https://github.com/cirosantilli/qemu-test/tree/6a3497f0d84e7c86ef80f7322e24e8a149b93214
with images-ab21ef58deed8536bc159c2afd680a4fabd68510.zip

Then try to run it several times with:

i=0; while true; do date; echo $i; ../qemu-test/arm/rr; i=$(($i+1)); done

I think the deadlock can happen in a few different places, but the
most common is when the kernel is doing disk related stuff, the last
messages before getting stuck are:

[   11.530325] ALSA device list:
[   11.531451]   No soundcards found.

and what would follow on a normal replay would be:

[   11.551904] EXT4-fs (vda): couldn't mount as ext3 due to feature
incompatibilities
[   11.619238] EXT4-fs (vda): mounted filesystem without journal. Opts: (null)

I then attach GDB with:

gdb -q ./arm-softmmu/qemu-system-arm `pgrep qemu`

and then:

>>> thread apply all bt

Thread 5 (Thread 0x7f59c6efb700 (LWP 22096)):
#0  0x7f59e7aa9072 in futex_wait_cancelable (private=, expected=0, futex_word=0x55a8e99801d8) at
../sysdeps/unix/sysv/linux/futex-internal.h:88
#1  0x7f59e7aa9072 in __pthread_cond_wait_common (abstime=0x0,
mutex=0x55a8e89cbf40 , cond=0x55a8e99801b0) at
pthread_cond_wait.c:502
#2  0x7f59e7aa9072 in __pthread_cond_wait (cond=0x55a8e99801b0,
mutex=0x55a8e89cbf40 ) at pthread_cond_wait.c:655
#3  0x55a8e7f4f178 in qemu_cond_wait_impl (cond=0x55a8e99801b0,
mutex=0x55a8e89cbf40 , file=0x55a8e80b10a8
"/home/ciro/git/qemu/cpus.c", line=1175) at
util/qemu-thread-posix.c:164
#4  0x55a8e765 in qemu_tcg_rr_wait_io_event
(cpu=0x55a8e986b330) at /home/ciro/git/qemu/cpus.c:1175
#5  0x55a8e799a1f5 in qemu_tcg_rr_cpu_thread_fn
(arg=0x55a8e986b330) at /home/ciro/git/qemu/cpus.c:1502
#6  0x7f59e7aa27fc in start_thread (arg=0x7f59c6efb700) at
pthread_create.c:465
#7  0x7f59e77cfb5f in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 4 (Thread 0x7f59c76fc700 (LWP 22095)):
#0  0x7f59e77c3a4b in __GI_ppoll (fds=0x7f59b8000b10, nfds=1,
timeout=, sigmask=0x0) at
../sysdeps/unix/sysv/linux/ppoll.c:39
#1  0x55a8e7f4a02e in qemu_poll_ns (fds=0x7f59b8000b10, nfds=1,
timeout=-1) at util/qemu-timer.c:322
#2  0x55a8e7f4cb5e in aio_poll (ctx=0x55a8e978eab0, blocking=true)
at util/aio-posix.c:629
#3  0x55a8e7b5f084 in iothread_run (opaque=0x55a8e970c710) at
iothread.c:64
#4  0x7f59e7aa27fc in start_thread (arg=0x7f59c76fc700) at
pthread_create.c:465
#5  0x7f59e77cfb5f in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 3 (Thread 0x7f59ced65700 (LWP 22093)):
#0  0x7f59e77c9a49 in syscall () at
../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x7f59e88456ef in g_cond_wait () at
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x55a8e7f43157 in wait_for_trace_records_available () at
trace/simple.c:150
#3  0x55a8e7f431b8 in writeout_thread (opaque=0x0) at
trace/simple.c:169
#4  0x7f59e8827645 in  () at
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x7f59e7aa27fc in start_thread (arg=0x7f59ced65700) at
pthread_create.c:465
#6  0x7f59e77cfb5f in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 2 (Thread 0x7f59cf566700 (LWP 22092)):
#0  0x7f59e77c9a49 in syscall () at
../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x55a8e7f4f5d8 in qemu_futex_wait (f=0x55a8e8e48418
, val=4294967295) at
/home/ciro/git/qemu/include/qemu/futex.h:29
#2  0x55a8e7f4f79f in qemu_event_wait (ev=0x55a8e8e48418
) at util/qemu-thread-posix.c:445
#3  0x55a8e7f67d2d in call_rcu_thread (opaque=0x0) at
util/rcu.c:261
#4  0x7f59e7aa27fc in start_thread (arg=0x7f59cf566700) at
pthread_create.c:465
#5  0x7f59e77cfb5f in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 1 (Thread 0x7f59ecf03280 (LWP 22091)):
#0  0x7f59e77c3a4b in __GI_ppoll (fds=0x55a8e9860aa0, nfds=5,
timeout=, sigmask=0x0) at
../sysdeps/unix/sysv/linux/ppoll.c:39
#1  0x55a8e7f4a0c4 in qemu_poll_ns (fds=0x55a8e9860aa0, nfds=5,
timeout=10) at util/qemu-timer.c:334
#2  0x55a8e7f4b176 in os_host_main_loop_wait (timeout=10)
at util/main-loop.c:258
#3  0x55a8e7f4b241 in main_loop_wait (nonblocking=0) at
util/main-loop.c:522
#4  0x55a8e7b66fed in main_loop () at vl.c:1943
#5  0x55a8e7b6ead4 in main (argc=24, argv=0x7fff6fe0f328,
envp=0x7fff6fe0f3f0) at vl.c:4740

On Wed, Apr 25, 2018 at 1:45 PM, Pavel Dovgalyuk
 wrote:
> GDB remote protocol supports reverse debugging of the targets.
> It includes 'reverse step' and 'reverse continue' operations.
> The first one finds the previous step of the execution,
> and the second one is intended to stop at the last breakpoint that
> would happen when the program is executed normally.
>
> Reverse debugging is possible in the replay mode, when at least
> one snapshot was created at the record

Re: [Qemu-devel] [RFC PATCH 00/17] reverse debugging

[Ciro Santilli]

On Sat, Apr 28, 2018 at 9:12 AM, Pavel Dovgalyuk  wrote:
>> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
>> On Thu, Apr 26, 2018 at 1:34 PM, Pavel Dovgalyuk  wrote:
>> >> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
>> >> On Wed, Apr 25, 2018 at 1:45 PM, Pavel Dovgalyuk
>> >>  wrote:
>> >> > GDB remote protocol supports reverse debugging of the targets.
>> >> > It includes 'reverse step' and 'reverse continue' operations.
>> >> > The first one finds the previous step of the execution,
>> >> > and the second one is intended to stop at the last breakpoint that
>> >> > would happen when the program is executed normally.
>> >> >
>> >> > Reverse debugging is possible in the replay mode, when at least
>> >> > one snapshot was created at the record or replay phase.
>> >> > QEMU can use these snapshots for travelling back in time with GDB.
>> >> >
>> >>
>> >> Hi Pavel,
>> >>
>> >> 1)
>> >>
>> >> Can you provide more details on how to run the reverse debugging? In
>> >> particular how to take the checkpoint?
>> >
>> > There is some information in docs/replay.txt, but I guess, that I can give 
>> > some more.
>> >
>> >>
>> >> My test setup is described in detail at:
>> >> https://github.com/cirosantilli/qemu-test/tree/8127452e5685ed233dc7357a1fe34b7a2d173480
>> >> command "x86_64/reverse-debug".
>> >>
>> >> Here are the actual commands:
>> >>
>> >> #!/usr/bin/env bash
>> >> set -eu
>> >> dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.."
>> >> cmd="\
>> >> time \
>> >> ./x86_64-softmmu/qemu-system-x86_64 \
>> >> -M pc \
>> >> -append 'root=/dev/sda console=ttyS0 nokaslr printk.time=y -
>> >> lkmc_eval=\"/rand_check.out;/sbin/ifup -a;wget -S
>> >> google.com;/poweroff.out;\"' \
>> >> -kernel '${dir}/out/x86_64/buildroot/images/bzImage' \
>> >> -nographic \
>> >> -serial mon:stdio \
>> >> -monitor telnet::45454,server,nowait \
>> >> \
>> >> -drive 
>> >> file='${dir}/out/x86_64/buildroot/images/rootfs.ext2.qcow2,if=none,id=img-
>> >> direct,format=qcow2,snapshot'
>> >
>> > The main thing for reverse debugging is snapshotting.
>> > Therefore you should have an image that does not use temporary overlay 
>> > file (snapshot
>> option).
>> > I'm using the following command line for record:
>> >
>> > rm ./images/xp.ovl
>> > # create overlay to avoid modifying the original image
>> > ./bin/qemu-img create -f qcow2 -b xp.qcow2 ./images/xp.ovl
>> > ./bin/qemu-system-i386 \
>> > # This is workaround for XP. I wonder is it needed for the current version 
>> > or not.
>> >  -global apic-common.vapic=off \
>> > # using newly created overlay instead of the original image
>> > # rrsnapshot creates the snapshot at the start
>> >  -icount shift=7,rr=record,rrfile=xp.replay,rrsnapshot=init -drive
>> file=./images/xp.ovl,if=none,id=img-direct \
>> >  -drive driver=blkreplay,if=none,image=img-direct,id=img-replay -device 
>> > ide-hd,drive=img-
>> replay -net none -m 256M -monitor stdio
>> >
>> > While recording I can create some snapshots with savevm.
>> > Command line for replaying differs only in "rr" option. rrsnapshot there 
>> > loads the initial
>> snapshot.
>> > Any of the previously created snapshots may be specified.
>> > You can also create new snapshots while replaying.
>> >
>>
>> How is the snapshot to be used chosen? Does this patch make it try to
>> smartly use the snapshot that is closest to the target break?
>
> Yes, it selects the closest snapshot.
>
>> Does rrsnapshot select which snapshot will be used, or just creates a
>> snapshot at the start or record?
>
> rrsnapshot creates a snapshot at record and loads it at start.
> It is required, because the disk image is modified by the execution,
> when 'snapshot' option is omitted.
>
>> I have modified my commands to remove snapshot from -drive, and add
>> rrsnapshot=init to -icount and the following works:
>>
>> b start_kernel
>> n
>> n
>> n
>> b
>> n
>> n
>> rc
>
> Great!
>
>> However, if after the "b start_kernel" I make a new snapshot on telnet
>> with "savevm a" to try and make the restore faster, then
>> reverse-continue fails.
>
> That's strange. What did it say?
>

Nothing, it just stayed on the same line.

>> Also, if I do "loadvm a" after "savevm a" while the debugger is
>> attached at start_kernel, the monitor just hangs. Is there a way to
>> restore snapshots while debugging of replay is going on?
>
> Never tried to do this.
> I'll check this out.
>
>
> Pavel Dovgalyuk
>

[Qemu-devel] [RFC v2] tcg: workaround branch instruction overflow in tcg_out_qemu_ld/st

[Laurent Vivier]

ppc64 uses a BC instruction to call the tcg_out_qemu_ld/st
slow path. BC instruction uses a relative address encoded
on 14 bits.

The slow path functions are added at the end of the generated
instructions buffer, in the reverse order of the callers.
So more we have slow path functions more the distance between
the caller (BC) and the function increases.

This patch changes the behavior to generate the functions in
the same order of the callers.

Fixes: 15fa08f845 ("tcg: Dynamically allocate TCGOps")
Signed-off-by: Laurent Vivier 
---

Notes:
v2:
  - add a pointer to the tail of the list to add new element
at the end and keep the original ordering
  - remove the recursive call

 tcg/tcg-ldst.inc.c | 11 ---
 tcg/tcg.c  |  3 ++-
 tcg/tcg.h  |  3 ++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/tcg/tcg-ldst.inc.c b/tcg/tcg-ldst.inc.c
index 0e14cf4357..735ebf8da6 100644
--- a/tcg/tcg-ldst.inc.c
+++ b/tcg/tcg-ldst.inc.c
@@ -46,7 +46,7 @@ static bool tcg_out_ldst_finalize(TCGContext *s)
 TCGLabelQemuLdst *lb;
 
 /* qemu_ld/st slow paths */
-for (lb = s->ldst_labels; lb != NULL; lb = lb->next) {
+for (lb = s->ldst_head; lb != NULL; lb = lb->next) {
 if (lb->is_ld) {
 tcg_out_qemu_ld_slow_path(s, lb);
 } else {
@@ -72,7 +72,12 @@ static inline TCGLabelQemuLdst *new_ldst_label(TCGContext *s)
 {
 TCGLabelQemuLdst *l = tcg_malloc(sizeof(*l));
 
-l->next = s->ldst_labels;
-s->ldst_labels = l;
+l->next = NULL;
+if (s->ldst_tail) {
+s->ldst_tail->next = l;
+} else {
+s->ldst_head = l;
+}
+s->ldst_tail = l;
 return l;
 }
diff --git a/tcg/tcg.c b/tcg/tcg.c
index bb24526c93..3ab195a23f 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -3324,7 +3324,8 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
 s->code_ptr = tb->tc.ptr;
 
 #ifdef TCG_TARGET_NEED_LDST_LABELS
-s->ldst_labels = NULL;
+s->ldst_head = NULL;
+s->ldst_tail = NULL;
 #endif
 #ifdef TCG_TARGET_NEED_POOL_LABELS
 s->pool_labels = NULL;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 30896ca304..22cb7cbffc 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -699,7 +699,8 @@ struct TCGContext {
 
 /* These structures are private to tcg-target.inc.c.  */
 #ifdef TCG_TARGET_NEED_LDST_LABELS
-struct TCGLabelQemuLdst *ldst_labels;
+struct TCGLabelQemuLdst *ldst_head;
+struct TCGLabelQemuLdst *ldst_tail;
 #endif
 #ifdef TCG_TARGET_NEED_POOL_LABELS
 struct TCGLabelPoolData *pool_labels;
-- 
2.14.3

Re: [Qemu-devel] [RFC PATCH 00/17] reverse debugging

[Ciro Santilli]

On Sat, Apr 28, 2018 at 10:27 AM, Pavel Dovgalyuk  wrote:
>
>
>> -Original Message-
>> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
>> Sent: Saturday, April 28, 2018 11:13 AM
>> To: Pavel Dovgalyuk
>> Subject: Re: [RFC PATCH 00/17] reverse debugging
>>
>> Forgetting about debugging, I belive there is a deadlock in the replay
>> at 63d426dfa4fbfac3d50cda3f553cd975de2b85ea , but it is rare.
>>
>> I have only reproduced it on ARM so far, and I haven't checked pre-patch.
>>
>> The setup is https://github.com/cirosantilli/qemu-
>> test/tree/6a3497f0d84e7c86ef80f7322e24e8a149b93214
>> with images-ab21ef58deed8536bc159c2afd680a4fabd68510.zip
>>
>> Then try to run it several times with:
>>
>> i=0; while true; do date; echo $i; ../qemu-test/arm/rr; i=$(($i+1)); done
>>
>> I think the deadlock can happen in a few different places, but the
>> most common is when the kernel is doing disk related stuff, the last
>> messages before getting stuck are:
>
> It usually happens when there is some bugs in the implementation of the 
> virtual devices.
> Our customers mostly emulates x86-based systems, therefore most of
> the ARM hardware is untested.
>

Hi Pete, do you know anything about this? Traces at:
http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg05218.html
command at: 
https://github.com/cirosantilli/qemu-test/blob/6a3497f0d84e7c86ef80f7322e24e8a149b93214/arm/rr

@Pavel: I recommend always replying to both me and to qemu-devel to
preserve a better history of our talk on the tracker.

>> [   11.530325] ALSA device list:
>> [   11.531451]   No soundcards found.
>>
>> and what would follow on a normal replay would be:
>>
>> [   11.551904] EXT4-fs (vda): couldn't mount as ext3 due to feature
>> incompatibilities
>> [   11.619238] EXT4-fs (vda): mounted filesystem without journal. Opts: 
>> (null)
>>
>> I then attach GDB with:
>>
>> gdb -q ./arm-softmmu/qemu-system-arm `pgrep qemu`
>
>
>
>
> Pavel Dovgalyuk
>

Re: [Qemu-devel] [RFC v2 1/2] virtio: add pmem driver

[Pankaj Gupta]

> > > > +int err;
> > > > +
> > > > +sg_init_one(&sg, buf, sizeof(buf));
> > > > +
> > > > +err = virtqueue_add_outbuf(vpmem->req_vq, &sg, 1, buf, 
> > > > GFP_KERNEL);
> > > > +
> > > > +if (err) {
> > > > +dev_err(&vdev->dev, "failed to send command to virtio 
> > > > pmem
> > > > device\n");
> > > > +return;
> > > > +}
> > > > +
> > > > +virtqueue_kick(vpmem->req_vq);
> > > 
> > > Is any locking necessary?  Two CPUs must not invoke virtio_pmem_flush()
> > > at the same time.  Not sure if anything guarantees this, maybe you're
> > > relying on libnvdimm but I haven't checked.
> > 
> > I thought about it to some extent, and wanted to go ahead with simple
> > version first:
> > 
> > - I think file 'inode -> locking' sill is there for request on single file.
> > - For multiple files, our aim is to just flush the backend block image.
> > - Even there is collision for virt queue read/write entry it should just
> > trigger a Qemu fsync.
> >   We just want most recent flush to assure guest writes are synced
> >   properly.
> > 
> > Important point here: We are doing entire block fsync for guest virtual
> > disk.
> 
> I don't understand your answer.  Is locking necessary or not?

It will be required with other changes.

> 
> From the virtqueue_add_outbuf() documentation:
> 
>  * Caller must ensure we don't call this with other virtqueue operations
>  * at the same time (except where noted).

Yes, I also saw it. But thought if can avoid it with current functionality. :)


Thanks,
Pankaj

Re: [Qemu-devel] [PATCH 2/3] block/file-posix: File locking during creation

[Max Reitz]

On 2018-04-27 08:22, Fam Zheng wrote:
> On Sat, 04/21 00:09, Max Reitz wrote:
>> When creating a file, we should take the WRITE and RESIZE permissions.
>> We do not need either for the creation itself, but we do need them for
>> clearing and resizing it.  So we can take the proper permissions by
>> replacing O_TRUNC with an explicit truncation to 0, and by taking the
>> appropriate file locks between those two steps.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  block/file-posix.c | 41 +++--
>>  1 file changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index c98a4a1556..ed7932d6e8 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -1992,6 +1992,7 @@ static int raw_co_create(BlockdevCreateOptions 
>> *options, Error **errp)
>>  {
>>  BlockdevCreateOptionsFile *file_opts;
>>  int fd;
>> +int perm, shared;
>>  int result = 0;
>>  
>>  /* Validate options and set default values */
>> @@ -2006,14 +2007,48 @@ static int raw_co_create(BlockdevCreateOptions 
>> *options, Error **errp)
>>  }
>>  
>>  /* Create file */
>> -fd = qemu_open(file_opts->filename, O_RDWR | O_CREAT | O_TRUNC | 
>> O_BINARY,
>> -   0644);
>> +fd = qemu_open(file_opts->filename, O_RDWR | O_CREAT | O_BINARY, 0644);
>>  if (fd < 0) {
>>  result = -errno;
>>  error_setg_errno(errp, -result, "Could not create file");
>>  goto out;
>>  }
>>  
>> +/* Take permissions: We want to discard everything, so we need
>> + * BLK_PERM_WRITE; and truncation to the desired size requires
>> + * BLK_PERM_RESIZE.
>> + * On the other hand, we cannot share the RESIZE permission
>> + * because we promise that after this function, the file has the
>> + * size given in the options.  If someone else were to resize it
>> + * concurrently, we could not guarantee that. */
> 
> Strictly speaking, we do close(fd) before this function returns so the lock is
> lost and race can happen.

Right, but then creation from the perspective of file-posix is over.  We
are going to reopen the file for formatting, but that is just a normal
bdrv_open() so it will automatically be locked, no?

>> +perm = BLK_PERM_WRITE | BLK_PERM_RESIZE;
>> +shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
>> +
>> +/* Step one: Take locks in shared mode */
>> +result = raw_apply_lock_bytes(fd, perm, shared, false, errp);
>> +if (result < 0) {
>> +goto out_close;
>> +}
>> +
>> +/* Step two: Try to get them exclusively */
>> +result = raw_check_lock_bytes(fd, perm, shared, errp);
>> +if (result < 0) {
>> +goto out_close;
>> +}
>> +
>> +/* Step three: Downgrade them to shared again, and keep
>> + * them that way until we are done */
>> +result = raw_apply_lock_bytes(fd, perm, shared, true, errp);
> 
> You comment it as "downgrade to shared" but what this actually does in 
> addition
> to step one is to "unlock" unneeded bytes which we haven't locked anyway. So 
> I'm
> confused why we need this stop. IIUC no downgrading is necessary after step 
> one
> and step two: only necessary shared locks are acquired in step one, and step 
> two
> is read-only op (F_OFD_GETLK).

Oops.  For some reason I thought raw_check_lock_bytes() would take the
locks exclusively.  Yes, then we don't need this third step.

Thanks for reviewing!

Max

>> +if (result < 0) {
>> +goto out_close;
>> +}
>> +
>> +/* Clear the file by truncating it to 0 */
>> +result = raw_regular_truncate(fd, 0, PREALLOC_MODE_OFF, errp);
>> +if (result < 0) {
>> +goto out_close;
>> +}
>> +
>>  if (file_opts->nocow) {
>>  #ifdef __linux__
>>  /* Set NOCOW flag to solve performance issue on fs like btrfs.
>> @@ -2029,6 +2064,8 @@ static int raw_co_create(BlockdevCreateOptions 
>> *options, Error **errp)
>>  #endif
>>  }
>>  
>> +/* Resize and potentially preallocate the file to the desired
>> + * final size */
>>  result = raw_regular_truncate(fd, file_opts->size, 
>> file_opts->preallocation,
>>errp);
>>  if (result < 0) {
>> -- 
>> 2.14.3
>>
>>
> 
> Fam
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 3/9] block: Add BDRV_REQ_WRITE_UNCHANGED flag

[Max Reitz]

On 2018-04-26 04:12, Eric Blake wrote:
> On 04/25/2018 10:08 AM, Max Reitz wrote:
> 
>>
>>> Also, that does raise the question of whether you have more work to
>>> support write-zero requests with WRITE_UNCHANGED (which indeed sounds
>>> like something plausible to support).
>>
>> I'm afraid I don't quite understand the question.
>> BDRV_REQ_WRITE_UNCHANGED support is usually rather simple because as
>> said above it is only needed by drivers that rely on their parent to
>> request the permissions, i.e. filter drivers.  Since filter drivers just
>> forward the requests, all they have to do is retain the
>> BDRV_REQ_WRITE_UNCHANGED flag, be it a zero write or a normal write.
> 
> I'm trying to figure out if BDRV_REQ_WRITE_UNCHANGED makes sense for
> bdrv_co_pwrite_zeroes as well as bdrv_co_pwrite. I think the answer is
> yes (if we know the guest already reads zeroes, but need to write to the
> protocol layer anyways because of a commit operation, then mixing both
> BDRV_REQ_WRITE_UNCHANGED and BDRV_REQ_ZERO_WRITE to the block layer
> makes sense, and supported_zero_flags should indeed pass
> BDRV_REQ_WRITE_UNCHANGED on to a driver.

Well, why not.

>> It would be more complicated for format drivers, because they would have
>> to verify that the incoming unchanged write actually ends up as an
>> unchanged write in their file.  But we have already recognized that that
>> would be too much to ask and that format drivers may want to generally
>> just write anything to their child if it's writable (even regardless of
>> whether the grandparent issues writes to the format driver node), so
>> they always grab a WRITE permission on their file if possible.
>> Therefore, they do not have to support this request flag.
> 
> So qcow2 doesn't have to support the flag, but file-posix.c might want
> to.  Or are you saying that only filter drivers need to advertise
> support for the flag?

It might make sense for file-posix, but when you think further, it
wouldn't do anything in practice.

First, if a protocol driver receives WRITE_UNCHANGED, there are two
things it can do: If it knows that it only has very plain storage, it
can just ignore the request because it won't do anything.

If a protocol driver supports backing files (gluster does, I think), it
cannot ignore them because just like qcow2 it needs to now make sure
that the data ends up in the top layer.  So if the protocol driver can
draw any benefits from advertising to the protocol that this is an
unchanging write (or that it just wants to bring data from a backing
file up to the top level, which it can infer from the WRITE_UNCHANGED
flag), then it would make sense for it to support the flag.

(Good point to add to the documentation.)

But from qemu's perspective, this is not required.  The protocol driver
will just be less efficient if it submits normal writes (and it can do
that, because qemu will refuse to grant WRITE_UNCHANGED on read-only
nodes, so the protocol driver must have write access).

OTOH, filter drivers (usually) *must* support this flag because they are
caught up in qemu's permission system.  They do not submit the writes to
some protocol, but to the block layer, and the block layer wants you to
have the proper permissions for that -- and that's not a simple RW/RO
decision, but we have read/write-unchanged in between.  So if a filter
driver that only forwards the permissions from its parent may only do
the very same requests the parent does -- and if the parent does a
WRITE_UNCHANGED request, the filter driver must do the same.

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [RFC PATCH v2 01/19] block: implement bdrv_snapshot_goto for blkreplay

[Pavel Dovgalyuk]

From: Pavel Dovgalyuk 

This patch enables making snapshots with blkreplay used in
block devices.
This function is required to make bdrv_snapshot_goto without
calling .bdrv_open which is not implemented.

Signed-off-by: Pavel Dovgalyuk 
---
 block/blkreplay.c |8 
 1 file changed, 8 insertions(+)

diff --git a/block/blkreplay.c b/block/blkreplay.c
index fe5a9b4..ec0aa82 100755
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -127,6 +127,12 @@ static int coroutine_fn 
blkreplay_co_flush(BlockDriverState *bs)
 return ret;
 }
 
+static int blkreplay_snapshot_goto(BlockDriverState *bs,
+   const char *snapshot_id)
+{
+return bdrv_snapshot_goto(bs->file->bs, snapshot_id, NULL);
+}
+
 static BlockDriver bdrv_blkreplay = {
 .format_name= "blkreplay",
 .instance_size  = 0,
@@ -142,6 +148,8 @@ static BlockDriver bdrv_blkreplay = {
 .bdrv_co_pwrite_zeroes  = blkreplay_co_pwrite_zeroes,
 .bdrv_co_pdiscard   = blkreplay_co_pdiscard,
 .bdrv_co_flush  = blkreplay_co_flush,
+
+.bdrv_snapshot_goto = blkreplay_snapshot_goto,
 };
 
 static void bdrv_blkreplay_init(void)

[Qemu-devel] [RFC PATCH v2 00/19] reverse debugging

[Pavel Dovgalyuk]

GDB remote protocol supports reverse debugging of the targets.
It includes 'reverse step' and 'reverse continue' operations.
The first one finds the previous step of the execution,
and the second one is intended to stop at the last breakpoint that
would happen when the program is executed normally.

Reverse debugging is possible in the replay mode, when at least
one snapshot was created at the record or replay phase.
QEMU can use these snapshots for travelling back in time with GDB.

Running the execution in replay mode allows using GDB reverse debugging
commands:
 - reverse-stepi (or rsi): Steps one instruction to the past.
   QEMU loads on of the prior snapshots and proceeds to the desired
   instruction forward. When that step is reaches, execution stops.
 - reverse-continue (or rc): Runs execution "backwards".
   QEMU tries to find breakpoint or watchpoint by loaded prior snapshot
   and replaying the execution. Then QEMU loads snapshots again and
   replays to the latest breakpoint. When there are no breakpoints in
   the examined section of the execution, QEMU finds one more snapshot
   and tries again. After the first snapshot is processed, execution
   stops at this snapshot.

The set of patches include the following modifications:
 - gdbstub update for reverse debugging support
 - functions that automatically perform reverse step and reverse
   continue operations
 - hmp/qmp commands for manipulating the replay process
 - improvement of the snapshotting for saving the execution step
   in the snapshot parameters
 - other record/replay fixes

The patches are available in the repository:
https://github.com/ispras/qemu/tree/rr-180428

v2 changes:
 - documented reverse debugging
 - fixed start vmstate loading in record mode
 - documented qcow2 changes (as suggested by Eric Blake)
 - made icount SnapshotInfo field optional (as suggested by Eric Blake)
 - renamed qmp commands (as suggested by Eric Blake)
 - minor changes

---

Pavel Dovgalyuk (19):
  block: implement bdrv_snapshot_goto for blkreplay
  replay: disable default snapshot for record/replay
  replay: update docs for record/replay with block devices
  replay: don't drain/flush bdrv queue while RR is working
  replay: finish record/replay before closing the disks
  qcow2: introduce icount field for snapshots
  migration: introduce icount field for snapshots
  replay: introduce info hmp/qmp command
  replay: introduce breakpoint at the specified step
  replay: implement replay-seek command to proceed to the desired step
  replay: flush events when exitting
  timer: remove replay clock probe in deadline calculation
  replay: refine replay-time module
  translator: fix breakpoint processing
  replay: flush rr queue before loading the vmstate
  gdbstub: add reverse step support in replay mode
  gdbstub: add reverse continue support in replay mode
  replay: describe reverse debugging in docs/replay.txt
  replay: allow loading any snapshots before recording


 accel/tcg/translator.c|8 +
 block/blkreplay.c |8 +
 block/io.c|   22 +++
 block/qapi.c  |   17 ++-
 block/qcow2-snapshot.c|9 +
 block/qcow2.h |2 
 blockdev.c|   10 ++
 cpus.c|   19 ++-
 docs/interop/qcow2.txt|4 +
 docs/replay.txt   |   45 +++
 exec.c|6 +
 gdbstub.c |   50 +++-
 hmp-commands-info.hx  |   14 ++
 hmp-commands.hx   |   30 +
 hmp.h |3 
 include/block/snapshot.h  |1 
 include/sysemu/replay.h   |   18 +++
 migration/savevm.c|   15 +-
 qapi/block-core.json  |5 +
 qapi/block.json   |3 
 qapi/misc.json|   68 +++
 replay/Makefile.objs  |3 
 replay/replay-debugging.c |  287 +
 replay/replay-events.c|   14 --
 replay/replay-internal.h  |   10 +-
 replay/replay-snapshot.c  |   17 ++-
 replay/replay-time.c  |   27 ++--
 replay/replay.c   |   22 +++
 stubs/replay.c|   10 ++
 util/qemu-timer.c |   11 --
 vl.c  |   18 ++-
 31 files changed, 695 insertions(+), 81 deletions(-)
 create mode 100644 replay/replay-debugging.c

-- 
Pavel Dovgalyuk

[Qemu-devel] [RFC PATCH v2 02/19] replay: disable default snapshot for record/replay

[Pavel Dovgalyuk]

From: Pavel Dovgalyuk 

This patch disables setting '-snapshot' option on by default
in record/replay mode. This is needed for creating vmstates in record
and replay modes.

Signed-off-by: Pavel Dovgalyuk 
---
 vl.c |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 7487535..39fc03a 100644
--- a/vl.c
+++ b/vl.c
@@ -3243,7 +3243,13 @@ int main(int argc, char **argv, char **envp)
 drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
 break;
 case QEMU_OPTION_snapshot:
-snapshot = 1;
+{
+Error *blocker = NULL;
+snapshot = 1;
+error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
+   "-snapshot");
+replay_add_blocker(blocker);
+}
 break;
 case QEMU_OPTION_numa:
 opts = qemu_opts_parse_noisily(qemu_find_opts("numa"),
@@ -4546,7 +4552,7 @@ int main(int argc, char **argv, char **envp)
 qapi_free_BlockdevOptions(bdo->bdo);
 g_free(bdo);
 }
-if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+if (snapshot) {
 qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
   NULL, NULL);
 }

[Qemu-devel] [RFC PATCH v2 10/19] replay: implement replay-seek command to proceed to the desired step

[Pavel Dovgalyuk]

This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
the execution to the specified step.
The commands automatically loads nearest snapshot and replay the execution
to find the desired step.

Signed-off-by: Pavel Dovgalyuk 

--

v2:
 - renamed replay_seek qmp command into replay-seek
   (suggested by Eric Blake)
---
 hmp-commands.hx   |   15 
 hmp.h |1 +
 qapi/misc.json|   16 
 replay/replay-debugging.c |   89 +
 4 files changed, 121 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f162f5e..18b287e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1835,6 +1835,21 @@ Execution stops when the specified step is reached.
 ETEXI
 
 {
+.name   = "replay_seek",
+.args_type  = "step:i",
+.params = "step",
+.help   = "rewinds replay to the specified step",
+.cmd= hmp_replay_seek,
+},
+
+STEXI
+@item replay_seek @var{step}
+@findex replay_seek
+Automatically proceeds to the specified step, when replaying
+the execution.
+ETEXI
+
+{
 .name   = "info",
 .args_type  = "item:s?",
 .params = "[subcommand]",
diff --git a/hmp.h b/hmp.h
index 5ef8f56..31f830c 100644
--- a/hmp.h
+++ b/hmp.h
@@ -146,5 +146,6 @@ void hmp_info_memory_size_summary(Monitor *mon, const QDict 
*qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_seek(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/misc.json b/qapi/misc.json
index ef8fe3c..a57f5bf 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3116,6 +3116,22 @@
 { 'command': 'replay-break', 'data': { 'step': 'int' } }
 
 ##
+# @replay-seek:
+#
+# Automatically proceeds to the specified step, when replaying
+# the execution.
+#
+# @step: destination execution step
+#
+# Since: 2.13
+#
+# Example:
+#
+# -> { "execute": "replay-seek", "data": { "step": 220414 } }
+##
+{ 'command': 'replay-seek', 'data': { 'step': 'int' } }
+
+##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 819017e..8d6c03d 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -18,6 +18,8 @@
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/timer.h"
+#include "block/snapshot.h"
+#include "migration/snapshot.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -94,3 +96,90 @@ void hmp_replay_break(Monitor *mon, const QDict *qdict)
 return;
 }
 }
+
+static char *replay_find_nearest_snapshot(int64_t step, int64_t* snapshot_step)
+{
+BlockDriverState *bs;
+QEMUSnapshotInfo *sn_tab;
+QEMUSnapshotInfo *nearest = NULL;
+char *ret = NULL;
+int nb_sns, i;
+AioContext *aio_context;
+
+*snapshot_step = -1;
+
+bs = bdrv_all_find_vmstate_bs();
+if (!bs) {
+goto fail;
+}
+aio_context = bdrv_get_aio_context(bs);
+
+aio_context_acquire(aio_context);
+nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+aio_context_release(aio_context);
+
+for (i = 0; i < nb_sns; i++) {
+if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
+if (sn_tab[i].icount != -1ULL
+&& sn_tab[i].icount <= step
+&& (!nearest || nearest->icount < sn_tab[i].icount)) {
+nearest = &sn_tab[i];
+}
+}
+}
+if (nearest) {
+ret = g_strdup(nearest->name);
+*snapshot_step = nearest->icount;
+}
+g_free(sn_tab);
+
+fail:
+return ret;
+}
+
+static void replay_seek(int64_t step, Error **errp, QEMUTimerCB callback)
+{
+char *snapshot = NULL;
+if (replay_mode != REPLAY_MODE_PLAY) {
+error_setg(errp, "replay must be enabled to seek");
+return;
+}
+if (!replay_snapshot) {
+error_setg(errp, "snapshotting is disabled");
+return;
+}
+int64_t snapshot_step = -1;
+snapshot = replay_find_nearest_snapshot(step, &snapshot_step);
+if (snapshot) {
+if (step < replay_get_current_step()
+|| replay_get_current_step() < snapshot_step) {
+vm_stop(RUN_STATE_RESTORE_VM);
+load_snapshot(snapshot, errp);
+}
+g_free(snapshot);
+}
+if (replay_get_current_step() <= step) {
+replay_break(step, callback, NULL);
+vm_start();
+} else {
+error_setg(errp, "cannot seek to the specified step");
+}
+}
+
+void qmp_replay_seek(int64_t step, Error **errp)
+{
+replay_seek(step, errp, replay_stop_vm);
+}
+
+void hmp_replay_seek(Monitor *mon, const QDict *qdict)
+{
+int64_t step = qdict_get_try_int(qdict, "step", -1LL);
+Error *err = NULL;
+
+qmp_replay_seek(step, &er

[Qemu-devel] [RFC PATCH v2 09/19] replay: introduce breakpoint at the specified step

[Pavel Dovgalyuk]

This patch introduces replay_break qmp and hmp commands.
These commands allow stopping at the specified instruction.
It may be useful for debugging when there are some known
events that should be investigated.
The commands have one argument - number of instructions
executed since the start of the replay.

Signed-off-by: Pavel Dovgalyuk 

--

v2:
 - renamed replay_break qmp command into replay-break
   (suggested by Eric Blake)
---
 hmp-commands.hx   |   15 
 hmp.h |1 +
 include/sysemu/replay.h   |3 ++
 qapi/misc.json|   17 ++
 replay/replay-debugging.c |   55 +
 replay/replay-internal.h  |4 +++
 replay/replay.c   |   17 ++
 7 files changed, 112 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 35d862a..f162f5e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1820,6 +1820,21 @@ Set QOM property @var{property} of object at location 
@var{path} to value @var{v
 ETEXI
 
 {
+.name   = "replay_break",
+.args_type  = "step:i",
+.params = "step",
+.help   = "sets breakpoint on the specified step of the replay",
+.cmd= hmp_replay_break,
+},
+
+STEXI
+@item replay_break @var{step}
+@findex replay_break
+Set breakpoint on the specified step of the replay.
+Execution stops when the specified step is reached.
+ETEXI
+
+{
 .name   = "info",
 .args_type  = "item:s?",
 .params = "[subcommand]",
diff --git a/hmp.h b/hmp.h
index 084fb62..5ef8f56 100644
--- a/hmp.h
+++ b/hmp.h
@@ -145,5 +145,6 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict 
*qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
+void hmp_replay_break(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 3ced6bc..98d709c 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -71,6 +71,9 @@ void replay_start(void);
 void replay_finish(void);
 /*! Adds replay blocker with the specified error description */
 void replay_add_blocker(Error *reason);
+/*! Sets breakpoint at the specified step.
+If step = -1LL the existing breakpoint is removed. */
+void replay_break(int64_t step, QEMUTimerCB callback, void *opaque);
 
 /* Processing the instructions */
 
diff --git a/qapi/misc.json b/qapi/misc.json
index 0b0e874..ef8fe3c 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3099,6 +3099,23 @@
   'returns': 'ReplayInfo' }
 
 ##
+# @replay-break:
+#
+# Set breakpoint on the specified step of the replay.
+# Execution stops when the specified step is reached.
+#
+# @step: execution step to stop at
+#
+# Since: 2.13
+#
+# Example:
+#
+# -> { "execute": "replay-break", "data": { "step": 220414 } }
+#
+##
+{ 'command': 'replay-break', 'data': { 'step': 'int' } }
+
+##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 03e7db8..819017e 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -16,6 +16,8 @@
 #include "hmp.h"
 #include "monitor/monitor.h"
 #include "qapi/qapi-commands-misc.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu/timer.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -39,3 +41,56 @@ ReplayInfo *qmp_query_replay(Error **errp)
 retval->step = replay_get_current_step();
 return retval;
 }
+
+void replay_break(int64_t step, QEMUTimerCB callback, void *opaque)
+{
+assert(replay_mode == REPLAY_MODE_PLAY);
+assert(replay_mutex_locked());
+
+replay_break_step = step;
+if (replay_break_timer) {
+timer_del(replay_break_timer);
+timer_free(replay_break_timer);
+replay_break_timer = NULL;
+}
+
+if (replay_break_step == -1LL) {
+return;
+}
+assert(replay_break_step >= replay_get_current_step());
+assert(callback);
+
+replay_break_timer = timer_new_ns(QEMU_CLOCK_REALTIME, callback, opaque);
+}
+
+static void replay_stop_vm(void *opaque)
+{
+vm_stop(RUN_STATE_PAUSED);
+replay_break(-1LL, NULL, NULL);
+}
+
+void qmp_replay_break(int64_t step, Error **errp)
+{
+if (replay_mode ==  REPLAY_MODE_PLAY) {
+if (step >= replay_get_current_step()) {
+replay_break(step, replay_stop_vm, NULL);
+} else {
+error_setg(errp, "cannot set break at the step in the past");
+}
+} else {
+error_setg(errp, "setting the break is allowed only in play mode");
+}
+}
+
+void hmp_replay_break(Monitor *mon, const QDict *qdict)
+{
+int64_t step = qdict_get_try_int(qdict, "step", -1LL);
+Error *err = NULL;
+
+qmp_replay_break(step, &err);
+if (err) {
+monitor_printf(mon, "repla

[Qemu-devel] [RFC PATCH v2 04/19] replay: don't drain/flush bdrv queue while RR is working

[Pavel Dovgalyuk]

In record/replay mode bdrv queue is controlled by replay mechanism.
It does not allow saving or loading the snapshots
when bdrv queue is not empty. Stopping the VM is not blocked by nonempty
queue, but flushing the queue is still impossible there,
because it may cause deadlocks in replay mode.
This patch disables bdrv_drain_all and bdrv_flush_all in
record/replay mode.

Signed-off-by: Pavel Dovgalyuk 
---
 block/io.c |   22 ++
 cpus.c |2 --
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/block/io.c b/block/io.c
index bd9a19a..1ceefbc 100644
--- a/block/io.c
+++ b/block/io.c
@@ -32,6 +32,7 @@
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "sysemu/replay.h"
 
 #define NOT_DONE 0x7fff /* used while emulated sync operation in progress 
*/
 
@@ -407,6 +408,13 @@ void bdrv_drain_all_begin(void)
 BdrvNextIterator it;
 GSList *aio_ctxs = NULL, *ctx;
 
+/* bdrv queue is managed by record/replay,
+   waiting for finishing the I/O requests may
+   be infinite */
+if (replay_events_enabled()) {
+return;
+}
+
 /* BDRV_POLL_WHILE() for a node can only be called from its own I/O thread
  * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on
  * nodes in several different AioContexts, so make sure we're in the main
@@ -458,6 +466,13 @@ void bdrv_drain_all_end(void)
 BlockDriverState *bs;
 BdrvNextIterator it;
 
+/* bdrv queue is managed by record/replay,
+   waiting for finishing the I/O requests may
+   be endless */
+if (replay_events_enabled()) {
+return;
+}
+
 for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
 AioContext *aio_context = bdrv_get_aio_context(bs);
 
@@ -1832,6 +1847,13 @@ int bdrv_flush_all(void)
 BlockDriverState *bs = NULL;
 int result = 0;
 
+/* bdrv queue is managed by record/replay,
+   creating new flush request for stopping
+   the VM may break the determinism */
+if (replay_events_enabled()) {
+return result;
+}
+
 for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
 AioContext *aio_context = bdrv_get_aio_context(bs);
 int ret;
diff --git a/cpus.c b/cpus.c
index 38eba8b..140cc4f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1016,7 +1016,6 @@ static int do_vm_stop(RunState state, bool send_stop)
 }
 
 bdrv_drain_all();
-replay_disable_events();
 ret = bdrv_flush_all();
 
 return ret;
@@ -2059,7 +2058,6 @@ int vm_prepare_start(void)
 qapi_event_send_stop(&error_abort);
 res = -1;
 } else {
-replay_enable_events();
 cpu_enable_ticks();
 runstate_set(RUN_STATE_RUNNING);
 vm_state_notify(1, RUN_STATE_RUNNING);

[Qemu-devel] [RFC PATCH v2 03/19] replay: update docs for record/replay with block devices

[Pavel Dovgalyuk]

This patch updates the description of the command lines for using
record/replay with attached block devices.

Signed-off-by: Pavel Dovgalyuk 
---
 docs/replay.txt |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/docs/replay.txt b/docs/replay.txt
index 2e21e9c..f7def53 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -27,7 +27,7 @@ Usage of the record/replay:
  * First, record the execution with the following command line:
 qemu-system-i386 \
  -icount shift=7,rr=record,rrfile=replay.bin \
- -drive file=disk.qcow2,if=none,id=img-direct \
+ -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
  -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
  -device ide-hd,drive=img-blkreplay \
  -netdev user,id=net1 -device rtl8139,netdev=net1 \
@@ -35,7 +35,7 @@ Usage of the record/replay:
  * After recording, you can replay it by using another command line:
 qemu-system-i386 \
  -icount shift=7,rr=replay,rrfile=replay.bin \
- -drive file=disk.qcow2,if=none,id=img-direct \
+ -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
  -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
  -device ide-hd,drive=img-blkreplay \
  -netdev user,id=net1 -device rtl8139,netdev=net1 \
@@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of
 bdrv coroutine functions at the top of block drivers stack.
 To record and replay block operations the drive must be configured
 as following:
- -drive file=disk.qcow2,if=none,id=img-direct
+ -drive file=disk.qcow2,if=none,snapshot,id=img-direct
  -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
  -device ide-hd,drive=img-blkreplay
 
@@ -252,6 +252,12 @@ This snapshot is created at start of recording and 
restored at start
 of replaying. It also can be loaded while replaying to roll back
 the execution.
 
+'snapshot' flag of the disk image must be removed to save the snapshots
+in the overlay (or original image) instead of using the temporary overlay.
+ -drive file=disk.ovl,if=none,id=img-direct
+ -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
+ -device ide-hd,drive=img-blkreplay
+
 Use QEMU monitor to create additional snapshots. 'savevm ' command
 created the snapshot and 'loadvm ' restores it. To prevent corruption
 of the original disk image, use overlay files linked to the original images.

[Qemu-devel] [RFC PATCH v2 13/19] replay: refine replay-time module

[Pavel Dovgalyuk]

This patch removes refactoring artifacts from the replay/replay-time.c

Signed-off-by: Pavel Dovgalyuk 
---
 replay/replay-time.c |   27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/replay/replay-time.c b/replay/replay-time.c
index 6a7565e..40030b8 100644
--- a/replay/replay-time.c
+++ b/replay/replay-time.c
@@ -17,14 +17,12 @@
 
 int64_t replay_save_clock(ReplayClockKind kind, int64_t clock)
 {
+g_assert(replay_file);
+g_assert(replay_mutex_locked());
 
-if (replay_file) {
-g_assert(replay_mutex_locked());
-
-replay_save_instructions();
-replay_put_event(EVENT_CLOCK + kind);
-replay_put_qword(clock);
-}
+replay_save_instructions();
+replay_put_event(EVENT_CLOCK + kind);
+replay_put_qword(clock);
 
 return clock;
 }
@@ -46,20 +44,15 @@ void replay_read_next_clock(ReplayClockKind kind)
 /*! Reads next clock event from the input. */
 int64_t replay_read_clock(ReplayClockKind kind)
 {
+int64_t ret;
 g_assert(replay_file && replay_mutex_locked());
 
 replay_account_executed_instructions();
 
-if (replay_file) {
-int64_t ret;
-if (replay_next_event_is(EVENT_CLOCK + kind)) {
-replay_read_next_clock(kind);
-}
-ret = replay_state.cached_clock[kind];
-
-return ret;
+if (replay_next_event_is(EVENT_CLOCK + kind)) {
+replay_read_next_clock(kind);
 }
+ret = replay_state.cached_clock[kind];
 
-error_report("REPLAY INTERNAL ERROR %d", __LINE__);
-exit(1);
+return ret;
 }

[Qemu-devel] [RFC PATCH v2 05/19] replay: finish record/replay before closing the disks

[Pavel Dovgalyuk]

After recent updates block devices cannot be closed on qemu exit.
This happens due to the block request polling when replay is not finished.
Therefore now we stop execution recording before closing the block devices.

Signed-off-by: Pavel Dovgalyuk 
---
 replay/replay.c |2 ++
 vl.c|1 +
 2 files changed, 3 insertions(+)

diff --git a/replay/replay.c b/replay/replay.c
index 8228261..58a986f 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -366,6 +366,8 @@ void replay_finish(void)
 g_free(replay_snapshot);
 replay_snapshot = NULL;
 
+replay_mode = REPLAY_MODE_NONE;
+
 replay_finish_events();
 }
 
diff --git a/vl.c b/vl.c
index 39fc03a..3bf85a4 100644
--- a/vl.c
+++ b/vl.c
@@ -4761,6 +4761,7 @@ int main(int argc, char **argv, char **envp)
 
 /* No more vcpu or device emulation activity beyond this point */
 vm_shutdown();
+replay_finish();
 
 bdrv_close_all();
 

[Qemu-devel] [RFC PATCH v2 16/19] gdbstub: add reverse step support in replay mode

[Pavel Dovgalyuk]

GDB remote protocol supports two reverse debugging commands:
reverse step and reverse continue.
This patch adds support of the first one to the gdbstub.
Reverse step is intended to step one instruction in the backwards
direction. This is not possible in regular execution.
But replayed execution is deterministic, therefore we can load one of
the prior snapshots and proceed to the desired step. It is equivalent
to stepping one instruction back.
There should be at least one snapshot preceding the debugged part of
the replay log.

Signed-off-by: Pavel Dovgalyuk 
---
 accel/tcg/translator.c|1 +
 cpus.c|   14 +++---
 exec.c|5 +
 gdbstub.c |   42 +++---
 include/sysemu/replay.h   |7 +++
 replay/replay-debugging.c |   33 +
 stubs/replay.c|5 +
 7 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 3c7a035..4adb37c 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -17,6 +17,7 @@
 #include "exec/gen-icount.h"
 #include "exec/log.h"
 #include "exec/translator.h"
+#include "sysemu/replay.h"
 
 /* Pairs with tcg_clear_temp_count.
To be called by #TranslatorOps.{translate_insn,tb_stop} if
diff --git a/cpus.c b/cpus.c
index 140cc4f..3fb9321 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1042,9 +1042,17 @@ static bool cpu_can_run(CPUState *cpu)
 
 static void cpu_handle_guest_debug(CPUState *cpu)
 {
-gdb_set_stop_cpu(cpu);
-qemu_system_debug_request();
-cpu->stopped = true;
+if (!replay_running_debug()) {
+gdb_set_stop_cpu(cpu);
+qemu_system_debug_request();
+cpu->stopped = true;
+} else {
+if (!cpu->singlestep_enabled) {
+cpu_single_step(cpu, SSTEP_ENABLE);
+} else {
+cpu_single_step(cpu, 0);
+}
+}
 }
 
 #ifdef CONFIG_LINUX
diff --git a/exec.c b/exec.c
index c7fcefa..0802aa4 100644
--- a/exec.c
+++ b/exec.c
@@ -2546,6 +2546,11 @@ static void check_watchpoint(int offset, int len, 
MemTxAttrs attrs, int flags)
 QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
 if (cpu_watchpoint_address_matches(wp, vaddr, len)
 && (wp->flags & flags)) {
+if (replay_running_debug()) {
+/* Don't process the watchpoints when we are
+   in a reverse debugging operation. */
+return;
+}
 if (flags == BP_MEM_READ) {
 wp->flags |= BP_WATCHPOINT_HIT_READ;
 } else {
diff --git a/gdbstub.c b/gdbstub.c
index 3c38073..86dec56 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -39,6 +39,7 @@
 #include "sysemu/kvm.h"
 #include "exec/semihost.h"
 #include "exec/exec-all.h"
+#include "sysemu/replay.h"
 
 #ifdef CONFIG_USER_ONLY
 #define GDB_ATTACHED "0"
@@ -334,6 +335,19 @@ typedef struct GDBState {
  */
 static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
 
+/*! Retrieves flags for single step mode. */
+static int get_sstep_flags(void)
+{
+/* In replay mode all events written into the log should be replayed.
+ * That is why NOIRQ flag is removed in this mode.
+ */
+if (replay_mode != REPLAY_MODE_NONE) {
+return SSTEP_ENABLE;
+} else {
+return sstep_flags;
+}
+}
+
 static GDBState *gdbserver_state;
 
 bool gdb_has_xml;
@@ -424,7 +438,7 @@ static int gdb_continue_partial(GDBState *s, char 
*newstates)
 CPU_FOREACH(cpu) {
 if (newstates[cpu->cpu_index] == 's') {
 trace_gdbstub_op_stepping(cpu->cpu_index);
-cpu_single_step(cpu, sstep_flags);
+cpu_single_step(cpu, get_sstep_flags());
 }
 }
 s->running_state = 1;
@@ -443,7 +457,7 @@ static int gdb_continue_partial(GDBState *s, char 
*newstates)
 break; /* nothing to do here */
 case 's':
 trace_gdbstub_op_stepping(cpu->cpu_index);
-cpu_single_step(cpu, sstep_flags);
+cpu_single_step(cpu, get_sstep_flags());
 cpu_resume(cpu);
 flag = 1;
 break;
@@ -1072,9 +1086,28 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 addr = strtoull(p, (char **)&p, 16);
 gdb_set_cpu_pc(s, addr);
 }
-cpu_single_step(s->c_cpu, sstep_flags);
+cpu_single_step(s->c_cpu, get_sstep_flags());
 gdb_continue(s);
 return RS_IDLE;
+case 'b':
+/* Backward debugging commands */
+if (replay_mode == REPLAY_MODE_PLAY) {
+switch (*p) {
+case 's':
+if (replay_reverse_step()) {
+gdb_continue(s);
+return RS_IDLE;
+} else {
+put_packet(s, "E14");
+break;
+}
+default:
+goto unk

[Qemu-devel] [RFC PATCH v2 07/19] migration: introduce icount field for snapshots

[Pavel Dovgalyuk]

Saving icount as a parameters of the snapshot allows navigation between
them in the execution replay scenario.
This information can be used for finding a specific snapshot for rewinding
the recorded execution to the specific moment of the time.
E.g., 'reverse step' action needs to load the nearest snapshot which is
prior to the current moment of time .

Signed-off-by: Pavel Dovgalyuk 

--

v2:
 - made icount in SnapshotInfo optional (suggested by Eric Blake)
---
 block/qapi.c |   17 +
 block/qcow2-snapshot.c   |2 ++
 blockdev.c   |   10 ++
 include/block/snapshot.h |1 +
 migration/savevm.c   |5 +
 qapi/block-core.json |5 -
 qapi/block.json  |3 ++-
 7 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 04c6fc6..b4f9c2f 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -210,6 +210,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 info->date_nsec = sn_tab[i].date_nsec;
 info->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 10;
 info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+info->icount= sn_tab[i].icount;
 
 info_list = g_new0(SnapshotInfoList, 1);
 info_list->value = info;
@@ -648,14 +649,15 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, 
void *f,
 QEMUSnapshotInfo *sn)
 {
 char buf1[128], date_buf[128], clock_buf[128];
+char icount_buf[128] = {0};
 struct tm tm;
 time_t ti;
 int64_t secs;
 
 if (!sn) {
 func_fprintf(f,
- "%-10s%-20s%7s%20s%15s",
- "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+ "%-10s%-18s%7s%20s%13s%11s",
+ "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
 } else {
 ti = sn->date_sec;
 localtime_r(&ti, &tm);
@@ -668,13 +670,18 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, 
void *f,
  (int)((secs / 60) % 60),
  (int)(secs % 60),
  (int)((sn->vm_clock_nsec / 100) % 1000));
+if (sn->icount != -1ULL) {
+snprintf(icount_buf, sizeof(icount_buf),
+"%"PRId64, sn->icount);
+}
 func_fprintf(f,
- "%-10s%-20s%7s%20s%15s",
+ "%-10s%-18s%7s%20s%13s%11s",
  sn->id_str, sn->name,
  get_human_readable_size(buf1, sizeof(buf1),
  sn->vm_state_size),
  date_buf,
- clock_buf);
+ clock_buf,
+ icount_buf);
 }
 }
 
@@ -842,6 +849,8 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, 
void *f,
 .date_nsec = elem->value->date_nsec,
 .vm_clock_nsec = elem->value->vm_clock_sec * 10ULL +
  elem->value->vm_clock_nsec,
+.icount = elem->value->has_icount ?
+  elem->value->icount : -1ULL,
 };
 
 pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index d04553e..0af32c3 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -379,6 +379,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 sn->date_sec = sn_info->date_sec;
 sn->date_nsec = sn_info->date_nsec;
 sn->vm_clock_nsec = sn_info->vm_clock_nsec;
+sn->icount = sn_info->icount;
 
 /* Allocate the L1 table of the snapshot and copy the current one there. */
 l1_table_offset = qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
@@ -698,6 +699,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 sn_info->date_sec = sn->date_sec;
 sn_info->date_nsec = sn->date_nsec;
 sn_info->vm_clock_nsec = sn->vm_clock_nsec;
+sn_info->icount = sn->icount;
 }
 *psn_tab = sn_tab;
 return s->nb_snapshots;
diff --git a/blockdev.c b/blockdev.c
index c31bf3d..965c96a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -56,6 +56,7 @@
 #include "block/trace.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/qtest.h"
+#include "sysemu/replay.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 #include "qemu/throttle-options.h"
@@ -1348,6 +1349,10 @@ SnapshotInfo 
*qmp_blockdev_snapshot_delete_internal_sync(const char *device,
 info->vm_state_size = sn.vm_state_size;
 info->vm_clock_nsec = sn.vm_clock_nsec % 10;
 info->vm_clock_sec = sn.vm_clock_nsec / 10;
+if (sn.icount != -1ULL) {
+info->icount = sn.icount;
+info->has_icount = true;
+}
 
 return info;
 
@@ -1556,6 +1561,11 @@ static void internal_snapshot_prepare(BlkActionState 
*common,
 sn->date_sec = tv.tv_sec;
 sn->date_nsec = t

[Qemu-devel] [RFC PATCH v2 14/19] translator: fix breakpoint processing

[Pavel Dovgalyuk]

QEMU cannot pass through the breakpoints when 'si' command is used
in remote gdb. This patch disables inserting the breakpoints
when we are already single stepping though the gdb remote protocol.
This patch also fixes icount calculation for the blocks that include
breakpoints - instruction with breakpoint is not executed and shouldn't
be used in icount calculation.

Signed-off-by: Pavel Dovgalyuk 
---
 accel/tcg/translator.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 23c6602..3c7a035 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -35,6 +35,7 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
  CPUState *cpu, TranslationBlock *tb)
 {
 int max_insns;
+int bp_insn = 0;
 
 /* Initialize DisasContext */
 db->tb = tb;
@@ -73,11 +74,13 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 tcg_debug_assert(db->is_jmp == DISAS_NEXT);  /* no early exit */
 
 /* Pass breakpoint hits to target for further processing */
-if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
+if (!db->singlestep_enabled
+&& unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
 CPUBreakpoint *bp;
 QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
 if (bp->pc == db->pc_next) {
 if (ops->breakpoint_check(db, cpu, bp)) {
+bp_insn = 1;
 break;
 }
 }
@@ -119,7 +122,7 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 
 /* Emit code to exit the TB, as indicated by db->is_jmp.  */
 ops->tb_stop(db, cpu);
-gen_tb_end(db->tb, db->num_insns);
+gen_tb_end(db->tb, db->num_insns - bp_insn);
 
 /* The disas_log hook may use these values rather than recompute.  */
 db->tb->size = db->pc_next - db->pc_first;

[Qemu-devel] [RFC PATCH v2 08/19] replay: introduce info hmp/qmp command

[Pavel Dovgalyuk]

This patch introduces 'info replay' monitor command and
corresponding qmp request.
These commands request the current record/replay mode, replay log file name,
and the execution step (number or recorded/replayed instructions).

Signed-off-by: Pavel Dovgalyuk 

--

v2:
 - renamed info_replay qmp into query-replay (suggested by Eric Blake)
---
 hmp-commands-info.hx  |   14 ++
 hmp.h |1 +
 qapi/misc.json|   35 +++
 replay/Makefile.objs  |3 ++-
 replay/replay-debugging.c |   41 +
 replay/replay-internal.h  |2 ++
 replay/replay.c   |3 +--
 7 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100644 replay/replay-debugging.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ddfcd5a..f5631be 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -883,6 +883,20 @@ STEXI
 Show SEV information.
 ETEXI
 
+{
+.name   = "replay",
+.args_type  = "",
+.params = "",
+.help   = "show parameters of the record/replay",
+.cmd= hmp_info_replay,
+},
+
+STEXI
+@item info replay
+@findex info replay
+Display the current record/replay mode and the currently executing step.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.h b/hmp.h
index 4e2ec37..084fb62 100644
--- a/hmp.h
+++ b/hmp.h
@@ -144,5 +144,6 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict 
*qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
+void hmp_info_replay(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/misc.json b/qapi/misc.json
index 5636f4a..0b0e874 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3064,6 +3064,41 @@
   'data': [ 'none', 'record', 'play' ] }
 
 ##
+# @ReplayInfo:
+#
+# Status of the record/replay mode.
+#
+# @mode: current mode.
+#
+# @filename: name of the record/replay log file.
+#
+# @step: current step number.
+#
+# Since: 2.13
+#
+##
+{ 'struct': 'ReplayInfo',
+  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'step': 'int' } }
+
+##
+# @query-replay:
+#
+# Retrieves the status of the execution record/replay.
+#
+# Returns: structure with the properties of the record/replay.
+#
+# Since: 2.13
+#
+# Example:
+#
+# -> { "execute": "query-replay" }
+# <- { "return": { "mode": "play", "filename": "log.rr", "step": 220414 } }
+#
+##
+{ 'command': 'query-replay',
+  'returns': 'ReplayInfo' }
+
+##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index cee6539..6694e3e 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -6,4 +6,5 @@ common-obj-y += replay-input.o
 common-obj-y += replay-char.o
 common-obj-y += replay-snapshot.o
 common-obj-y += replay-net.o
-common-obj-y += replay-audio.o
\ No newline at end of file
+common-obj-y += replay-audio.o
+common-obj-y += replay-debugging.o
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
new file mode 100644
index 000..03e7db8
--- /dev/null
+++ b/replay/replay-debugging.c
@@ -0,0 +1,41 @@
+/*
+ * replay-debugging.c
+ *
+ * Copyright (c) 2010-2018 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/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qapi-commands-misc.h"
+
+void hmp_info_replay(Monitor *mon, const QDict *qdict)
+{
+if (replay_mode == REPLAY_MODE_NONE) {
+monitor_printf(mon, "No record/replay\n");
+} else {
+monitor_printf(mon, "%s execution '%s': current step = %"PRId64"\n",
+replay_mode == REPLAY_MODE_RECORD ? "Recording" : "Replaying",
+replay_filename, replay_get_current_step());
+}
+}
+
+ReplayInfo *qmp_query_replay(Error **errp)
+{
+ReplayInfo *retval = g_new0(ReplayInfo, 1);
+retval->mode = replay_mode;
+if (replay_filename) {
+retval->filename = g_strdup(replay_filename);
+retval->has_filename = true;
+}
+retval->step = replay_get_current_step();
+return retval;
+}
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index ac4b27b..ef82b5e 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -91,6 +91,8 @@ extern ReplayState replay_state;
 
 /* File for replay writing */
 extern FILE *replay_file;
+/*! Name of replay file  */
+extern char *replay_filename;
 
 void replay_put_byte(uint8_t byte);
 void replay_put_event(uint8_t event);
diff --git a/replay/replay.c b/replay/replay.c
index 58a986

[Qemu-devel] [RFC PATCH v2 18/19] replay: describe reverse debugging in docs/replay.txt

[Pavel Dovgalyuk]

This patch updates the documentation and describes usage of the reverse
debugging in QEMU+GDB.

Signed-off-by: Pavel Dovgalyuk 
---
 docs/replay.txt |   33 +
 1 file changed, 33 insertions(+)

diff --git a/docs/replay.txt b/docs/replay.txt
index f7def53..086d3f8 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -293,6 +293,39 @@ for recording and replaying must contain identical number 
of ports in record
 and replay modes, but their backends may differ.
 E.g., '-serial stdio' in record mode, and '-serial null' in replay mode.
 
+Reverse debugging
+-
+
+Reverse debugging allows "executing" the program in reverse direction.
+GDB remote protocol supports "reverse step" and "reverse continue"
+commands. The first one steps single instruction backwards in time,
+and the second one finds the last breakpoint in the past.
+
+Recorded executions may be used to enable reverse debugging. QEMU can't
+execute the code in backwards direction, but can load a snapshot and
+replay forward to find the desired position or breakpoint.
+
+The following GDB commands are supported:
+ - reverse-stepi (or rsi) - step one instruction backwards
+ - reverse-continue (or rc) - find last breakpoint in the past
+
+Reverse step loads the nearest snapshot and replays the execution until
+the required instruction is met.
+
+Reverse continue may include several passes of examining the execution
+between the snapshots. Each of the passes include the following steps:
+ 1. loading the snapshot
+ 2. replaying to examine the breakpoints
+ 3. if breakpoint or watchpoint was met
+- loading the snaphot again
+- replaying to the required breakpoint
+ 4. else
+- proceeding to the p.1 with the earlier snapshot
+
+Therefore usage of the reverse debugging requires at least one snapshot
+created in advance. See the "Snapshotting" section to learn about running
+record/replay and creating the snapshot in these modes.
+
 Replay log format
 -
 

[Qemu-devel] [RFC PATCH v2 17/19] gdbstub: add reverse continue support in replay mode

[Pavel Dovgalyuk]

This patch adds support of the reverse continue operation for gdbstub.
Reverse continue finds the last breakpoint that would happen in normal
execution from the beginning to the current moment.
Implementation of the reverse continue replays the execution twice:
to find the breakpoints that were hit and to seek to the last breakpoint.
Reverse continue loads the previous snapshot and tries to find the breakpoint
since that moment. If there are no such breakpoints, it proceeds to
the earlier snapshot, and so on. When no breakpoints or watchpoints were
hit at all, execution stops at the beginning of the replay log.

Signed-off-by: Pavel Dovgalyuk 
---
 cpus.c|3 ++
 exec.c|1 +
 gdbstub.c |   10 ++-
 include/sysemu/replay.h   |6 
 replay/replay-debugging.c |   69 +
 stubs/replay.c|5 +++
 6 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 3fb9321..9e81e9a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1048,6 +1048,9 @@ static void cpu_handle_guest_debug(CPUState *cpu)
 cpu->stopped = true;
 } else {
 if (!cpu->singlestep_enabled) {
+/* Report about the breakpoint and
+   make a single step to skip it */
+replay_breakpoint();
 cpu_single_step(cpu, SSTEP_ENABLE);
 } else {
 cpu_single_step(cpu, 0);
diff --git a/exec.c b/exec.c
index 0802aa4..e5fe4dd 100644
--- a/exec.c
+++ b/exec.c
@@ -2549,6 +2549,7 @@ static void check_watchpoint(int offset, int len, 
MemTxAttrs attrs, int flags)
 if (replay_running_debug()) {
 /* Don't process the watchpoints when we are
in a reverse debugging operation. */
+replay_breakpoint();
 return;
 }
 if (flags == BP_MEM_READ) {
diff --git a/gdbstub.c b/gdbstub.c
index 86dec56..1645b7f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1101,6 +1101,14 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 put_packet(s, "E14");
 break;
 }
+case 'c':
+if (replay_reverse_continue()) {
+gdb_continue(s);
+return RS_IDLE;
+} else {
+put_packet(s, "E14");
+break;
+}
 default:
 goto unknown_command;
 }
@@ -1371,7 +1379,7 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
 }
 if (replay_mode == REPLAY_MODE_PLAY) {
-pstrcat(buf, sizeof(buf), ";ReverseStep+");
+pstrcat(buf, sizeof(buf), ";ReverseStep+;ReverseContinue+");
 }
 put_packet(s, buf);
 break;
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 611eabb..a3113c1 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -78,9 +78,15 @@ void replay_break(int64_t step, QEMUTimerCB callback, void 
*opaque);
 Used by gdbstub for backwards debugging.
 Returns true on success. */
 bool replay_reverse_step(void);
+/*! Start searching the last breakpoint/watchpoint.
+Used by gdbstub for backwards debugging.
+Returns true if the process successfully started. */
+bool replay_reverse_continue(void);
 /*! Returns true if replay module is processing
 reverse_continue or reverse_step request */
 bool replay_running_debug(void);
+/*! Called in reverse debugging mode to collect breakpoint information */
+void replay_breakpoint(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 388cf12..edab98e 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -22,6 +22,8 @@
 #include "migration/snapshot.h"
 
 static bool replay_is_debugging;
+static int64_t replay_last_breakpoint;
+static int64_t replay_last_snapshot;
 
 bool replay_running_debug(void)
 {
@@ -216,3 +218,70 @@ bool replay_reverse_step(void)
 
 return false;
 }
+
+static void replay_continue_end(void)
+{
+replay_is_debugging = false;
+vm_stop(RUN_STATE_DEBUG);
+replay_break(-1LL, NULL, NULL);
+}
+
+static void replay_continue_stop(void *opaque)
+{
+Error *err = NULL;
+if (replay_last_breakpoint != -1LL) {
+replay_seek(replay_last_breakpoint, &err, replay_stop_vm_debug);
+if (err) {
+error_free(err);
+replay_continue_end();
+}
+return;
+}
+/* No breakpoints since the last snapshot.
+   Find previous snapshot and try again. */
+if (replay_last_snapshot != 0) {
+replay_seek(replay_last_snapshot - 1, &err, replay_continue_stop);
+if (err) {
+error_free(err);
+replay_continue_end();
+  

[Qemu-devel] [RFC PATCH v2 06/19] qcow2: introduce icount field for snapshots

[Pavel Dovgalyuk]

This patch introduces the icount field for saving within the snapshot.
It is required for navigation between the snapshots in record/replay mode.

Signed-off-by: Pavel Dovgalyuk 

--

v2:
 - documented format changes in docs/interop/qcow2.txt
   (suggested by Eric Blake)
---
 block/qcow2-snapshot.c |7 +++
 block/qcow2.h  |2 ++
 docs/interop/qcow2.txt |4 
 3 files changed, 13 insertions(+)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 74293be..d04553e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -103,6 +103,12 @@ int qcow2_read_snapshots(BlockDriverState *bs)
 sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
 }
 
+if (extra_data_size >= 24) {
+sn->icount = be64_to_cpu(extra.icount);
+} else {
+sn->icount = -1ULL;
+}
+
 /* Read snapshot ID */
 sn->id_str = g_malloc(id_str_size + 1);
 ret = bdrv_pread(bs->file, offset, sn->id_str, id_str_size);
@@ -209,6 +215,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 memset(&extra, 0, sizeof(extra));
 extra.vm_state_size_large = cpu_to_be64(sn->vm_state_size);
 extra.disk_size = cpu_to_be64(sn->disk_size);
+extra.icount = cpu_to_be64(sn->icount);
 
 id_str_size = strlen(sn->id_str);
 name_size = strlen(sn->name);
diff --git a/block/qcow2.h b/block/qcow2.h
index adf5c39..8880937 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -151,6 +151,7 @@ typedef struct QEMU_PACKED QCowSnapshotHeader {
 typedef struct QEMU_PACKED QCowSnapshotExtraData {
 uint64_t vm_state_size_large;
 uint64_t disk_size;
+uint64_t icount;
 } QCowSnapshotExtraData;
 
 
@@ -164,6 +165,7 @@ typedef struct QCowSnapshot {
 uint32_t date_sec;
 uint32_t date_nsec;
 uint64_t vm_clock_nsec;
+uint64_t icount;
 } QCowSnapshot;
 
 struct Qcow2Cache;
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index feb711f..ff5b4ed 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -506,6 +506,10 @@ Snapshot table entry:
 
 Byte 48 - 55:   Virtual disk size of the snapshot in bytes
 
+Byte 56 - 63:   icount value which corresponds to
+the record/replay step when the snapshot
+was taken
+
 Version 3 images must include extra data at least up to
 byte 55.
 

[Qemu-devel] [RFC PATCH v2 11/19] replay: flush events when exitting

[Pavel Dovgalyuk]

This patch adds events processing when emulation finishes instead
of just cleaning the queue. Now the bdrv coroutines will be in consistent
state when emulator closes. It allows correct polling of the block layer
at exit.

Signed-off-by: Pavel Dovgalyuk 
---
 replay/replay-events.c   |   14 +-
 replay/replay-internal.h |2 --
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/replay/replay-events.c b/replay/replay-events.c
index 707de38..0964a82 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -94,18 +94,6 @@ void replay_disable_events(void)
 }
 }
 
-void replay_clear_events(void)
-{
-g_assert(replay_mutex_locked());
-
-while (!QTAILQ_EMPTY(&events_list)) {
-Event *event = QTAILQ_FIRST(&events_list);
-QTAILQ_REMOVE(&events_list, event, events);
-
-g_free(event);
-}
-}
-
 /*! Adds specified async event to the queue */
 void replay_add_event(ReplayAsyncEventKind event_kind,
   void *opaque,
@@ -308,7 +296,7 @@ void replay_init_events(void)
 void replay_finish_events(void)
 {
 events_enabled = false;
-replay_clear_events();
+replay_flush_events();
 }
 
 bool replay_events_enabled(void)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 34d19eb..a2221e5 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -148,8 +148,6 @@ void replay_init_events(void);
 void replay_finish_events(void);
 /*! Flushes events queue */
 void replay_flush_events(void);
-/*! Clears events list before loading new VM state */
-void replay_clear_events(void);
 /*! Returns true if there are any unsaved events in the queue */
 bool replay_has_events(void);
 /*! Saves events from queue into the file */

[Qemu-devel] [RFC PATCH v2 12/19] timer: remove replay clock probe in deadline calculation

[Pavel Dovgalyuk]

Ciro Santilli reported that commit a5ed352596a8b7eb2f9acce34371b944ac3056c4
breaks the execution replay. It happens due to the probing the clock
for the new instances of iothread.
However, this probing was made in replay mode for the timer lists that
are empty.
This patch removes clock probing in replay mode.
It is an artifact of the old version with another thread model.

Signed-off-by: Pavel Dovgalyuk 
---
 util/qemu-timer.c |   11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 2ed1bf2..86bfe84 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -578,17 +578,10 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup 
*tlg)
 {
 int64_t deadline = -1;
 QEMUClockType type;
-bool play = replay_mode == REPLAY_MODE_PLAY;
 for (type = 0; type < QEMU_CLOCK_MAX; type++) {
 if (qemu_clock_use_for_deadline(type)) {
-if (!play || type == QEMU_CLOCK_REALTIME) {
-deadline = qemu_soonest_timeout(deadline,
-
timerlist_deadline_ns(tlg->tl[type]));
-} else {
-/* Read clock from the replay file and
-   do not calculate the deadline, based on virtual clock. */
-qemu_clock_get_ns(type);
-}
+deadline = qemu_soonest_timeout(deadline,
+
timerlist_deadline_ns(tlg->tl[type]));
 }
 }
 return deadline;

[Qemu-devel] [RFC PATCH v2 15/19] replay: flush rr queue before loading the vmstate

[Pavel Dovgalyuk]

Non-empty record/replay queue prevents saving and loading the VM state,
because it includes pending bottom halves and block coroutines.
But when the new VM state is loaded, we don't have to preserve the consistency
of the current state anymore. Therefore this patch just flushes the queue
allowing the coroutines to finish.

Signed-off-by: Pavel Dovgalyuk 
---
 include/sysemu/replay.h  |2 ++
 migration/savevm.c   |   10 --
 replay/replay-internal.h |2 --
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 98d709c..84a1ec5 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -132,6 +132,8 @@ void replay_disable_events(void);
 void replay_enable_events(void);
 /*! Returns true when saving events is enabled */
 bool replay_events_enabled(void);
+/*! Flushes events queue */
+void replay_flush_events(void);
 /*! Adds bottom half event to the queue */
 void replay_bh_schedule_event(QEMUBH *bh);
 /*! Adds input event to the queue */
diff --git a/migration/savevm.c b/migration/savevm.c
index 08c1d4c..abe7d2f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2423,12 +2423,6 @@ int load_snapshot(const char *name, Error **errp)
 AioContext *aio_context;
 MigrationIncomingState *mis = migration_incoming_get_current();
 
-if (!replay_can_snapshot()) {
-error_report("Record/replay does not allow loading snapshot "
- "right now. Try once more later.");
-return -EINVAL;
-}
-
 if (!bdrv_all_can_snapshot(&bs)) {
 error_setg(errp,
"Device '%s' is writable but does not support snapshots",
@@ -2462,6 +2456,10 @@ int load_snapshot(const char *name, Error **errp)
 return -EINVAL;
 }
 
+/* Flush the record/replay queue. Now the VM state is going
+   to change. Therefore we don't need to preserve its consistency */
+replay_flush_events();
+
 /* Flush all IO requests so they don't interfere with the new state.  */
 bdrv_drain_all_begin();
 
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index a2221e5..08ef2ec 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -146,8 +146,6 @@ void replay_read_next_clock(unsigned int kind);
 void replay_init_events(void);
 /*! Clears internal data structures for events handling */
 void replay_finish_events(void);
-/*! Flushes events queue */
-void replay_flush_events(void);
 /*! Returns true if there are any unsaved events in the queue */
 bool replay_has_events(void);
 /*! Saves events from queue into the file */

[Qemu-devel] [RFC PATCH v2 19/19] replay: allow loading any snapshots before recording

[Pavel Dovgalyuk]

This patch enables using -loadvm in recording mode to allow starting
the execution recording from any of the available snapshots.
It also fixes loading of the record/replay state, therefore snapshots
created in replay mode may also be used for starting the new recording.

Signed-off-by: Pavel Dovgalyuk 
---
 replay/replay-snapshot.c |   17 -
 vl.c |7 ---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 2ab85cf..16bacc9 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -33,11 +33,18 @@ static int replay_pre_save(void *opaque)
 static int replay_post_load(void *opaque, int version_id)
 {
 ReplayState *state = opaque;
-fseek(replay_file, state->file_offset, SEEK_SET);
-qemu_clock_set_last(QEMU_CLOCK_HOST, state->host_clock_last);
-/* If this was a vmstate, saved in recording mode,
-   we need to initialize replay data fields. */
-replay_fetch_data_kind();
+if (replay_mode == REPLAY_MODE_PLAY) {
+fseek(replay_file, state->file_offset, SEEK_SET);
+qemu_clock_set_last(QEMU_CLOCK_HOST, state->host_clock_last);
+/* If this was a vmstate, saved in recording mode,
+   we need to initialize replay data fields. */
+replay_fetch_data_kind();
+} else if (replay_mode == REPLAY_MODE_RECORD) {
+/* This is only useful for loading the initial state.
+   Therefore reset all the counters. */
+state->instructions_count = 0;
+state->block_request_id = 0;
+}
 
 return 0;
 }
diff --git a/vl.c b/vl.c
index 3bf85a4..122ee6b 100644
--- a/vl.c
+++ b/vl.c
@@ -4724,15 +4724,16 @@ int main(int argc, char **argv, char **envp)
 replay_checkpoint(CHECKPOINT_RESET);
 qemu_system_reset(SHUTDOWN_CAUSE_NONE);
 register_global_state();
-if (replay_mode != REPLAY_MODE_NONE) {
-replay_vmstate_init();
-} else if (loadvm) {
+if (loadvm) {
 Error *local_err = NULL;
 if (load_snapshot(loadvm, &local_err) < 0) {
 error_report_err(local_err);
 autostart = 0;
 }
 }
+if (replay_mode != REPLAY_MODE_NONE) {
+replay_vmstate_init();
+}
 
 qdev_prop_check_globals();
 if (vmstate_dump_file) {

[Qemu-devel] [PATCH 0/5] qemu-io: Exit with error when a command failed

[Max Reitz]

Right now, qemu-io's exit code is rather useless as it is usually 0.
Except sometimes, then it's 1 in case of an error (mostly when you
specify a filename as an argument and it cannot open that).

At the same time, most command functions' return values are rather
useless as they are usually 0 (meaning "continue execution").  There is
only a single function that breaks that pattern, which is "quit".  On
one hand, this is pointless because "quit" is in qemu-io.c, so it can
easily signal that fact through a global (yet static) variable.  On the
other, it breaks the usual pattern of I/O functions returning error
codes.

This series resolves the overlap between both issues by making the
command functions' return error values instead of whether to continue
execution or not, and thus makes qemu-io return 1 if any of the commands
executed has failed and 0 only if all of them have succeeded.

Patch 5 showcases how that may be useful for iotests.


See also: https://bugzilla.redhat.com/show_bug.cgi?id=1519617


Max Reitz (5):
  qemu-io: Drop command functions' return values
  qemu-io: Let command functions return error code
  qemu-io: Exit with error when a command failed
  iotests.py: Add qemu_io_silent
  iotests: Let 216 make use of qemu-io's exit code

 include/qemu-io.h |   4 +-
 qemu-io-cmds.c| 276 --
 qemu-io.c |  62 +++---
 tests/qemu-iotests/216|  23 ++--
 tests/qemu-iotests/216.out|  17 +--
 tests/qemu-iotests/iotests.py |   9 ++
 6 files changed, 226 insertions(+), 165 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 4/5] iotests.py: Add qemu_io_silent

[Max Reitz]

With qemu-io now returning a useful exit code, some tests may find it
sufficient to just query that instead of logging (and filtering) the
whole output.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b25d48a91b..9747ca9eba 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -119,6 +119,15 @@ def qemu_io(*args):
 sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
'.join(args)))
 return subp.communicate()[0]
 
+def qemu_io_silent(*args):
+'''Run qemu-io and return the exit code, suppressing stdout'''
+args = qemu_io_args + list(args)
+exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'))
+if exitcode < 0:
+sys.stderr.write('qemu-io received signal %i: %s\n' %
+ (-exitcode, ' '.join(args)))
+return exitcode
+
 
 class QemuIoInteractive:
 def __init__(self, *args):
-- 
2.14.3

[Qemu-devel] [PATCH 3/5] qemu-io: Exit with error when a command failed

[Max Reitz]

Currently, qemu-io basically always returns success when it gets to
interactive mode (so once the whole command line has been parsed; even
before the commands on the command line are interpreted).  That is not
very useful.

This patch makes qemu-io return failure when any of the executed
commands failed.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1519617
Signed-off-by: Max Reitz 
---
 qemu-io.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index e82dd0b999..c95886d22a 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -400,17 +400,21 @@ static void prep_fetchline(void *opaque)
 *fetchable= 1;
 }
 
-static void command_loop(void)
+static int command_loop(void)
 {
 int i, fetchable = 0, prompted = 0;
+int ret, last_error = 0;
 char *input;
 
 for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
-qemuio_command(qemuio_blk, cmdline[i]);
+ret = qemuio_command(qemuio_blk, cmdline[i]);
+if (ret < 0) {
+last_error = ret;
+}
 }
 if (cmdline) {
 g_free(cmdline);
-return;
+return last_error;
 }
 
 while (!quit_qemu_io) {
@@ -431,13 +435,19 @@ static void command_loop(void)
 if (input == NULL) {
 break;
 }
-qemuio_command(qemuio_blk, input);
+ret = qemuio_command(qemuio_blk, input);
 g_free(input);
 
+if (ret < 0) {
+last_error = ret;
+}
+
 prompted = 0;
 fetchable = 0;
 }
 qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
+
+return last_error;
 }
 
 static void add_user_command(char *optarg)
@@ -502,6 +512,7 @@ int main(int argc, char **argv)
 int c;
 int opt_index = 0;
 int flags = BDRV_O_UNMAP;
+int ret;
 bool writethrough = true;
 Error *local_error = NULL;
 QDict *opts = NULL;
@@ -663,7 +674,7 @@ int main(int argc, char **argv)
 }
 }
 }
-command_loop();
+ret = command_loop();
 
 /*
  * Make sure all outstanding requests complete before the program exits.
@@ -672,5 +683,10 @@ int main(int argc, char **argv)
 
 blk_unref(qemuio_blk);
 g_free(readline_state);
-return 0;
+
+if (ret < 0) {
+return 1;
+} else {
+return 0;
+}
 }
-- 
2.14.3

[Qemu-devel] [PATCH 5/5] iotests: Let 216 make use of qemu-io's exit code

[Max Reitz]

As a showcase of how you can use qemu-io's exit code to determine
success or failure (same for qemu-img), this test is changed to use
qemu_io_silent() instead of qemu_io(), and to assert the exit code
instead of logging the filtered result.

One real advantage of this is that in case of an error, you get a
backtrace that helps you locate the issue in the test file quickly.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/216 | 23 ---
 tests/qemu-iotests/216.out | 17 ++---
 2 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/216 b/tests/qemu-iotests/216
index b362cf93a8..072707449d 100755
--- a/tests/qemu-iotests/216
+++ b/tests/qemu-iotests/216
@@ -22,7 +22,7 @@
 # Non-shared storage migration test using NBD server and drive-mirror
 
 import iotests
-from iotests import log, qemu_img_pipe, qemu_io, filter_qemu_io
+from iotests import log, qemu_img, qemu_io_silent
 
 # Need backing file support
 iotests.verify_image_format(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk'])
@@ -52,14 +52,13 @@ with iotests.FilePath('base.img') as base_img_path, \
 log('--- Setting up images ---')
 log('')
 
-qemu_img_pipe('create', '-f', iotests.imgfmt, base_img_path, '64M')
+assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
+assert qemu_io_silent(base_img_path, '-c', 'write -P 1 0M 1M') == 0
+assert qemu_img('create', '-f', iotests.imgfmt, '-b', base_img_path,
+top_img_path) == 0
+assert qemu_io_silent(top_img_path,  '-c', 'write -P 2 1M 1M') == 0
 
-log(filter_qemu_io(qemu_io(base_img_path, '-c', 'write -P 1 0M 1M')))
-
-qemu_img_pipe('create', '-f', iotests.imgfmt, '-b', base_img_path,
-  top_img_path)
-
-log(filter_qemu_io(qemu_io(top_img_path,  '-c', 'write -P 2 1M 1M')))
+log('Done')
 
 log('')
 log('--- Doing COR ---')
@@ -112,6 +111,8 @@ with iotests.FilePath('base.img') as base_img_path, \
 log('--- Checking COR result ---')
 log('')
 
-log(filter_qemu_io(qemu_io(base_img_path, '-c', 'discard 0 64M')))
-log(filter_qemu_io(qemu_io(top_img_path,  '-c', 'read -P 1 0M 1M')))
-log(filter_qemu_io(qemu_io(top_img_path,  '-c', 'read -P 2 1M 1M')))
+assert qemu_io_silent(base_img_path, '-c', 'discard 0 64M') == 0
+assert qemu_io_silent(top_img_path,  '-c', 'read -P 1 0M 1M') == 0
+assert qemu_io_silent(top_img_path,  '-c', 'read -P 2 1M 1M') == 0
+
+log('Done')
diff --git a/tests/qemu-iotests/216.out b/tests/qemu-iotests/216.out
index d3fc590d29..45ea857ee1 100644
--- a/tests/qemu-iotests/216.out
+++ b/tests/qemu-iotests/216.out
@@ -3,12 +3,7 @@
 
 --- Setting up images ---
 
-wrote 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-wrote 1048576/1048576 bytes at offset 1048576
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
+Done
 
 --- Doing COR ---
 
@@ -17,12 +12,4 @@ wrote 1048576/1048576 bytes at offset 1048576
 
 --- Checking COR result ---
 
-discard 67108864/67108864 bytes at offset 0
-64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-read 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-read 1048576/1048576 bytes at offset 1048576
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
+Done
-- 
2.14.3

[Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code

[Max Reitz]

This is basically what everything else in the qemu code base does, so we
can do it here, too.

Signed-off-by: Max Reitz 
---
 include/qemu-io.h |   4 +-
 qemu-io-cmds.c| 346 --
 qemu-io.c |  34 --
 3 files changed, 227 insertions(+), 157 deletions(-)

diff --git a/include/qemu-io.h b/include/qemu-io.h
index 06cdfbf660..380724ad59 100644
--- a/include/qemu-io.h
+++ b/include/qemu-io.h
@@ -22,7 +22,7 @@
 
 #define CMD_FLAG_GLOBAL ((int)0x8000) /* don't iterate "args" */
 
-typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
 typedef void (*helpfunc_t)(void);
 
 typedef struct cmdinfo {
@@ -41,7 +41,7 @@ typedef struct cmdinfo {
 
 extern bool qemuio_misalign;
 
-void qemuio_command(BlockBackend *blk, const char *cmd);
+int qemuio_command(BlockBackend *blk, const char *cmd);
 
 void qemuio_add_command(const cmdinfo_t *ci);
 void qemuio_command_usage(const cmdinfo_t *ci);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index c2fbaae0fd..5bf5f28178 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -65,13 +65,13 @@ static int init_check_command(BlockBackend *blk, const 
cmdinfo_t *ct)
 return 1;
 }
 
-static void command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
-char **argv)
+static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
+   char **argv)
 {
 char *cmd = argv[0];
 
 if (!init_check_command(blk, ct)) {
-return;
+return -EINVAL;
 }
 
 if (argc - 1 < ct->argmin || (ct->argmax != -1 && argc - 1 > ct->argmax)) {
@@ -88,7 +88,7 @@ static void command(BlockBackend *blk, const cmdinfo_t *ct, 
int argc,
 "bad argument count %d to %s, expected between %d and %d 
arguments\n",
 argc-1, cmd, ct->argmin, ct->argmax);
 }
-return;
+return -EINVAL;
 }
 
 /* Request additional permissions if necessary for this command. The caller
@@ -108,13 +108,13 @@ static void command(BlockBackend *blk, const cmdinfo_t 
*ct, int argc,
 ret = blk_set_perm(blk, new_perm, orig_shared_perm, &local_err);
 if (ret < 0) {
 error_report_err(local_err);
-return;
+return ret;
 }
 }
 }
 
 optind = 0;
-ct->cfunc(blk, argc, argv);
+return ct->cfunc(blk, argc, argv);
 }
 
 static const cmdinfo_t *find_command(const char *cmd)
@@ -633,7 +633,7 @@ static void read_help(void)
 "\n");
 }
 
-static void read_f(BlockBackend *blk, int argc, char **argv);
+static int read_f(BlockBackend *blk, int argc, char **argv);
 
 static const cmdinfo_t read_cmd = {
 .name   = "read",
@@ -646,12 +646,12 @@ static const cmdinfo_t read_cmd = {
 .help   = read_help,
 };
 
-static void read_f(BlockBackend *blk, int argc, char **argv)
+static int read_f(BlockBackend *blk, int argc, char **argv)
 {
 struct timeval t1, t2;
 bool Cflag = false, qflag = false, vflag = false;
 bool Pflag = false, sflag = false, lflag = false, bflag = false;
-int c, cnt;
+int c, cnt, ret;
 char *buf;
 int64_t offset;
 int64_t count;
@@ -673,7 +673,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
 pattern_count = cvtnum(optarg);
 if (pattern_count < 0) {
 print_cvtnum_err(pattern_count, optarg);
-return;
+return pattern_count;
 }
 break;
 case 'p':
@@ -683,7 +683,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
 Pflag = true;
 pattern = parse_pattern(optarg);
 if (pattern < 0) {
-return;
+return -EINVAL;
 }
 break;
 case 'q':
@@ -694,7 +694,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
 pattern_offset = cvtnum(optarg);
 if (pattern_offset < 0) {
 print_cvtnum_err(pattern_offset, optarg);
-return;
+return pattern_offset;
 }
 break;
 case 'v':
@@ -702,35 +702,35 @@ static void read_f(BlockBackend *blk, int argc, char 
**argv)
 break;
 default:
 qemuio_command_usage(&read_cmd);
-return;
+return -EINVAL;
 }
 }
 
 if (optind != argc - 2) {
 qemuio_command_usage(&read_cmd);
-return;
+return -EINVAL;
 }
 
 offset = cvtnum(argv[optind]);
 if (offset < 0) {
 print_cvtnum_err(offset, argv[optind]);
-return;
+return offset;
 }
 
 optind++;
 count = cvtnum(argv[optind]);
 if (count < 0) {
 print_cvtnum_err(count, argv[optind]);
-return;
+return count;
 } else if (count > BDRV_REQUEST_MAX_BYTES) {
 printf("

[Qemu-devel] [PATCH 1/5] qemu-io: Drop command functions' return values

[Max Reitz]

For qemu-io, a function returns an integer with two possible values: 0
for "qemu-io may continue execution", or 1 for "qemu-io should exit".
However, there is only a single command that returns 1, and that is
"quit".

So let's turn this case into a global variable instead so we can make
better use of the return value in a later patch.

Signed-off-by: Max Reitz 
---
 include/qemu-io.h |   6 +-
 qemu-io-cmds.c| 294 +-
 qemu-io.c |  36 +++
 3 files changed, 157 insertions(+), 179 deletions(-)

diff --git a/include/qemu-io.h b/include/qemu-io.h
index 196fde0f3a..06cdfbf660 100644
--- a/include/qemu-io.h
+++ b/include/qemu-io.h
@@ -22,7 +22,7 @@
 
 #define CMD_FLAG_GLOBAL ((int)0x8000) /* don't iterate "args" */
 
-typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
 typedef void (*helpfunc_t)(void);
 
 typedef struct cmdinfo {
@@ -41,10 +41,10 @@ typedef struct cmdinfo {
 
 extern bool qemuio_misalign;
 
-bool qemuio_command(BlockBackend *blk, const char *cmd);
+void qemuio_command(BlockBackend *blk, const char *cmd);
 
 void qemuio_add_command(const cmdinfo_t *ci);
-int qemuio_command_usage(const cmdinfo_t *ci);
+void qemuio_command_usage(const cmdinfo_t *ci);
 void qemuio_complete_command(const char *input,
  void (*fn)(const char *cmd, void *opaque),
  void *opaque);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 9b3cd00af6..c2fbaae0fd 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -48,10 +48,9 @@ void qemuio_add_command(const cmdinfo_t *ci)
 qsort(cmdtab, ncmds, sizeof(*cmdtab), compare_cmdname);
 }
 
-int qemuio_command_usage(const cmdinfo_t *ci)
+void qemuio_command_usage(const cmdinfo_t *ci)
 {
 printf("%s %s -- %s\n", ci->name, ci->args, ci->oneline);
-return 0;
 }
 
 static int init_check_command(BlockBackend *blk, const cmdinfo_t *ct)
@@ -66,13 +65,13 @@ static int init_check_command(BlockBackend *blk, const 
cmdinfo_t *ct)
 return 1;
 }
 
-static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
-   char **argv)
+static void command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
+char **argv)
 {
 char *cmd = argv[0];
 
 if (!init_check_command(blk, ct)) {
-return 0;
+return;
 }
 
 if (argc - 1 < ct->argmin || (ct->argmax != -1 && argc - 1 > ct->argmax)) {
@@ -89,7 +88,7 @@ static int command(BlockBackend *blk, const cmdinfo_t *ct, 
int argc,
 "bad argument count %d to %s, expected between %d and %d 
arguments\n",
 argc-1, cmd, ct->argmin, ct->argmax);
 }
-return 0;
+return;
 }
 
 /* Request additional permissions if necessary for this command. The caller
@@ -109,13 +108,13 @@ static int command(BlockBackend *blk, const cmdinfo_t 
*ct, int argc,
 ret = blk_set_perm(blk, new_perm, orig_shared_perm, &local_err);
 if (ret < 0) {
 error_report_err(local_err);
-return 0;
+return;
 }
 }
 }
 
 optind = 0;
-return ct->cfunc(blk, argc, argv);
+ct->cfunc(blk, argc, argv);
 }
 
 static const cmdinfo_t *find_command(const char *cmd)
@@ -634,7 +633,7 @@ static void read_help(void)
 "\n");
 }
 
-static int read_f(BlockBackend *blk, int argc, char **argv);
+static void read_f(BlockBackend *blk, int argc, char **argv);
 
 static const cmdinfo_t read_cmd = {
 .name   = "read",
@@ -647,7 +646,7 @@ static const cmdinfo_t read_cmd = {
 .help   = read_help,
 };
 
-static int read_f(BlockBackend *blk, int argc, char **argv)
+static void read_f(BlockBackend *blk, int argc, char **argv)
 {
 struct timeval t1, t2;
 bool Cflag = false, qflag = false, vflag = false;
@@ -674,7 +673,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 pattern_count = cvtnum(optarg);
 if (pattern_count < 0) {
 print_cvtnum_err(pattern_count, optarg);
-return 0;
+return;
 }
 break;
 case 'p':
@@ -684,7 +683,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 Pflag = true;
 pattern = parse_pattern(optarg);
 if (pattern < 0) {
-return 0;
+return;
 }
 break;
 case 'q':
@@ -695,40 +694,43 @@ static int read_f(BlockBackend *blk, int argc, char 
**argv)
 pattern_offset = cvtnum(optarg);
 if (pattern_offset < 0) {
 print_cvtnum_err(pattern_offset, optarg);
-return 0;
+return;
 }
 break;
 case 'v':
 vflag = true;
 break;
 default:
-return qemuio_command_usage(&read_cmd);

[Qemu-devel] [PATCH 0/2] qcow2: Repair OFLAG_COPIED when fixing leaks

[Max Reitz]

Suppose you have an image with consistent OFLAG_COPIED and refcounts.
Now further suppose that image has leaked clusters (single reference,
but refcount 2).  When checking such an image with qemu-img check, it
will notify you of the leakage, and that's it.

Now when trying to repair that image, you naturally use -r leaks because
that should be sufficient and you want to give qemu-img check just as
many "permissions" as it needs so it doesn't destroy your image by
accident.  But while the qcow2 driver will fix the refcounts, alas! it
will now complain about OFLAG_COPIED because that should be set now.
However, it will not set that without -r all.  So, congratulations, now
you have an image that qemu-img check tells you is really corrupted.

This series makes -r leaks fix OFLAG_COPIED as well, but only if
repairing the refcounts was completely successful.

There is a Red Hat Bugzilla entry here:
https://bugzilla.redhat.com/show_bug.cgi?id=1527085

...but I'm afraid the bug description is set to confidential, although I
don't know why.  Sorry. :-/
For everyone who cannot read it, let's say the test case in this series
may very well be a good indication of what you don't see.


Max Reitz (2):
  qcow2: Repair OFLAG_COPIED when fixing leaks
  iotests: Repairing error during snapshot deletion

 block/qcow2-refcount.c | 25 -
 tests/qemu-iotests/217 | 89 ++
 tests/qemu-iotests/217.out | 42 ++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 149 insertions(+), 8 deletions(-)
 create mode 100755 tests/qemu-iotests/217
 create mode 100644 tests/qemu-iotests/217.out

-- 
2.14.3

[Qemu-devel] [PATCH 2/2] iotests: Repairing error during snapshot deletion

[Max Reitz]

This adds a test for an I/O error during snapshot deletion, and maybe
more importantly, for how to repair the resulting image.  If the
snapshot has been deleted before the error occurs, the only negative
result will be leaked clusters -- and those should be repairable with
qemu-img check -r leaks.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/217 | 89 ++
 tests/qemu-iotests/217.out | 42 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 132 insertions(+)
 create mode 100755 tests/qemu-iotests/217
 create mode 100644 tests/qemu-iotests/217.out

diff --git a/tests/qemu-iotests/217 b/tests/qemu-iotests/217
new file mode 100755
index 00..3b42138760
--- /dev/null
+++ b/tests/qemu-iotests/217
@@ -0,0 +1,89 @@
+#!/bin/bash
+#
+# I/O errors when working with internal qcow2 snapshots, and repairing
+# the result
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$TEST_DIR/blkdebug.conf"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This test is specific to qcow2
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Internal snapshots are (currently) impossible with refcount_bits=1
+_unsupported_imgopts 'refcount_bits=1[^0-9]'
+
+echo
+echo '=== Simulating an I/O error during snapshot deletion ==='
+echo
+
+_make_test_img 64M
+$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+# Create the snapshot
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+
+# Verify the snapshot is there
+echo
+_img_info | grep 'Snapshot list'
+echo '(Snapshot filtered)'
+echo
+
+# Try to delete the snapshot (with an error happening when freeing the
+# then leaked clusters)
+cat > "$TEST_DIR/blkdebug.conf" <

[Qemu-devel] [PATCH 1/2] qcow2: Repair OFLAG_COPIED when fixing leaks

[Max Reitz]

Repairing OFLAG_COPIED is usually safe because it is done after the
refcounts have been repaired.  Therefore, it we did not find anyone else
referencing a data or L2 cluster, it makes no sense to not set
OFLAG_COPIED -- and the other direction (clearing OFLAG_COPIED) is
always safe, anyway, it may just induce leaks.

Furthermore, if OFLAG_COPIED is actually consistent with a wrong (leaky)
refcount, we will decrement the refcount with -r leaks, but OFLAG_COPIED
will then be wrong.  qemu-img check should not produce images that are
more corrupted afterwards then they were before.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1527085
Signed-off-by: Max Reitz 
---
 block/qcow2-refcount.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 2dc23005b7..e1ab4d0bfd 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1799,6 +1799,19 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 int ret;
 uint64_t refcount;
 int i, j;
+bool repair;
+
+if (fix & BDRV_FIX_ERRORS) {
+/* Always repair */
+repair = true;
+} else if (fix & BDRV_FIX_LEAKS) {
+/* Repair only if that seems safe: This function is always
+ * called after the refcounts have been fixed, so the refcount
+ * is accurate if that repair was successful */
+repair = !res->check_errors && !res->corruptions && !res->leaks;
+} else {
+repair = false;
+}
 
 for (i = 0; i < s->l1_size; i++) {
 uint64_t l1_entry = s->l1_table[i];
@@ -1818,10 +1831,8 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) {
 fprintf(stderr, "%s OFLAG_COPIED L2 cluster: l1_index=%d "
 "l1_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
-fix & BDRV_FIX_ERRORS ? "Repairing" :
-"ERROR",
-i, l1_entry, refcount);
-if (fix & BDRV_FIX_ERRORS) {
+repair ? "Repairing" : "ERROR", i, l1_entry, refcount);
+if (repair) {
 s->l1_table[i] = refcount == 1
? l1_entry |  QCOW_OFLAG_COPIED
: l1_entry & ~QCOW_OFLAG_COPIED;
@@ -1862,10 +1873,8 @@ static int check_oflag_copied(BlockDriverState *bs, 
BdrvCheckResult *res,
 if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) {
 fprintf(stderr, "%s OFLAG_COPIED data cluster: "
 "l2_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
-fix & BDRV_FIX_ERRORS ? "Repairing" :
-"ERROR",
-l2_entry, refcount);
-if (fix & BDRV_FIX_ERRORS) {
+repair ? "Repairing" : "ERROR", l2_entry, 
refcount);
+if (repair) {
 l2_table[j] = cpu_to_be64(refcount == 1
 ? l2_entry |  QCOW_OFLAG_COPIED
 : l2_entry & ~QCOW_OFLAG_COPIED);
-- 
2.14.3

[Qemu-devel] [Bug 1766904] Re: Creating high hdd load (with constant fsyncs) on a SATA disk leads to freezes and errors in guest dmesg

[Jake]

I am getting the exact same issue. The freeze occurred when I tried to
install Ubuntu 18.04 with qemu-2.12. However, it seems to be working
just fine with qemu-2.11.1. So it seems that something in between 2.11.1
and 2.12 is the culprit.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1766904

Title:
  Creating high hdd load (with constant fsyncs) on a SATA disk leads to
  freezes and errors in guest dmesg

Status in QEMU:
  New

Bug description:
  After upgrading from qemu 2.10.0+dfsg-2 to 2.12~rc3+dfsg-2 (on debian
  sid host), centos 7 guest started to show freezes and ata errors in
  dmesg during hdd workloads with writing many small files and repeated
  fsyncs.

  Host kernel 4.15.0-3-amd64.
  Guest kernel 3.10.0-693.21.1.el7.x86_64 (slightly older guest kernel was 
tested too with same result).

  Script that reproduces the bug (first run usualy goes smooth, second
  and later runs result in dmesg errors and freezes):

  http://paste.debian.net/hidden/472fb220/

  Sample of error messages in guest dmesg:

  http://paste.debian.net/hidden/8219e234/

  This vm is launchd using virsh start. VM launch command (from logfile
  in /var/log/libvirt/qemu/):

  http://paste.debian.net/plainh/5604126f

  A workaround that I am using right now: I have detached this SATA
  storage and reattached the same .qcow2 file as SCSI - this has fixed
  the issue for me.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1766904/+subscriptions

[Qemu-devel] Unix signal to send ACPI-shutdown to Guest

[Andrew Wood via Qemu-devel]

Ive been looking into the possibility of using a unix signal to send an 
acpi shutdown request to a VM, and came across a posting on this l in 
March 1. See 
https://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg04169.html



To summarise to refresh memories  there was a patch proposed using 
SIGINT followed by discussion as to why SIGINT wasnt ideal because it 
would cause regressions for anyone relying on the existing handling of 
SIGINT, and various other signals were considered including HUP & USR1 & 
USR2 all of which are already used for something else.


Is SIGPWR a suitable candidate, I have made a patch to use SIGPWR but as 
Im new to QEMUs internals Im not sure if there are any circumstances in 
which an alternative handler is registered for SIGPWR which might conflict?


Regards

Andrew

[Qemu-devel] [PATCH v1 2/2] xlnx-zynqmp: Connect the ZynqMP GDMA and ADMA

[Francisco Iglesias]

The ZynqMP contains two instances of a generic DMA, the GDMA, located in the
FPD (full power domain), and the ADMA, located in LPD (low power domain).  This
patch adds these two DMAs to the ZynqMP board.

Signed-off-by: Francisco Iglesias 
---
 hw/arm/xlnx-zynqmp.c | 53 
 include/hw/arm/xlnx-zynqmp.h |  5 +
 2 files changed, 58 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 505253e0d2..d8c3c9e8d9 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -90,6 +90,24 @@ static const int spi_intr[XLNX_ZYNQMP_NUM_SPIS] = {
 19, 20,
 };
 
+static const uint64_t gdma_addr[XLNX_ZYNQMP_NUM_GDMA] = {
+0xFD50, 0xFD51, 0xFD52, 0xFD53,
+0xFD54, 0xFD55, 0xFD56, 0xFD57
+};
+
+static const int gdma_intr[XLNX_ZYNQMP_NUM_GDMA] = {
+124, 125, 126, 127, 128, 129, 130, 131
+};
+
+static const uint64_t adma_addr[XLNX_ZYNQMP_NUM_ADMA] = {
+0xFFA8, 0xFFA9, 0xFFAA, 0xFFAB,
+0xFFAC, 0xFFAD, 0xFFAE, 0xFFAF
+};
+
+static const int adma_intr[XLNX_ZYNQMP_NUM_ADMA] = {
+77, 78, 79, 80, 81, 82, 83, 84
+};
+
 typedef struct XlnxZynqMPGICRegion {
 int region_index;
 uint32_t address;
@@ -197,6 +215,16 @@ static void xlnx_zynqmp_init(Object *obj)
 
 object_initialize(&s->rtc, sizeof(s->rtc), TYPE_XLNX_ZYNQMP_RTC);
 qdev_set_parent_bus(DEVICE(&s->rtc), sysbus_get_default());
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA; i++) {
+object_initialize(&s->gdma[i], sizeof(s->gdma[i]), TYPE_XLNX_ZDMA);
+qdev_set_parent_bus(DEVICE(&s->gdma[i]), sysbus_get_default());
+}
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_ADMA; i++) {
+object_initialize(&s->adma[i], sizeof(s->adma[i]), TYPE_XLNX_ZDMA);
+qdev_set_parent_bus(DEVICE(&s->adma[i]), sysbus_get_default());
+}
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -492,6 +520,31 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, RTC_ADDR);
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0, gic_spi[RTC_IRQ]);
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA; i++) {
+object_property_set_uint(OBJECT(&s->gdma[i]), 128, "bus-width", &err);
+object_property_set_bool(OBJECT(&s->gdma[i]), true, "realized", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gdma[i]), 0, gdma_addr[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->gdma[i]), 0,
+   gic_spi[gdma_intr[i]]);
+}
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_ADMA; i++) {
+object_property_set_bool(OBJECT(&s->adma[i]), true, "realized", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->adma[i]), 0, adma_addr[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->adma[i]), 0,
+   gic_spi[adma_intr[i]]);
+}
 }
 
 static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 3b613e364d..9824a0fa17 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -27,6 +27,7 @@
 #include "hw/sd/sdhci.h"
 #include "hw/ssi/xilinx_spips.h"
 #include "hw/dma/xlnx_dpdma.h"
+#include "hw/dma/xlnx-zdma.h"
 #include "hw/display/xlnx_dp.h"
 #include "hw/intc/xlnx-zynqmp-ipi.h"
 #include "hw/timer/xlnx-zynqmp-rtc.h"
@@ -41,6 +42,8 @@
 #define XLNX_ZYNQMP_NUM_UARTS 2
 #define XLNX_ZYNQMP_NUM_SDHCI 2
 #define XLNX_ZYNQMP_NUM_SPIS 2
+#define XLNX_ZYNQMP_NUM_GDMA 8
+#define XLNX_ZYNQMP_NUM_ADMA 8
 
 #define XLNX_ZYNQMP_NUM_QSPI_BUS 2
 #define XLNX_ZYNQMP_NUM_QSPI_BUS_CS 2
@@ -94,6 +97,8 @@ typedef struct XlnxZynqMPState {
 XlnxDPDMAState dpdma;
 XlnxZynqMPIPI ipi;
 XlnxZynqMPRTC rtc;
+XlnxZDMA gdma[XLNX_ZYNQMP_NUM_GDMA];
+XlnxZDMA adma[XLNX_ZYNQMP_NUM_ADMA];
 
 char *boot_cpu;
 ARMCPU *boot_cpu_ptr;
-- 
2.11.0

[Qemu-devel] [PATCH v1 0/2] xlnx-zynqmp: Add emulation of the ZynqMP GDMA and ADMA

[Francisco Iglesias]

Hi,

The ZynqMP Soc contains two separate instances of a generic DMA, one located in
the FPD (full power domain) called GDMA and a second one located in the LPD
(low power domain) called ADMA. This patch series attempts to add emulation
support for these two DMAs on the ZynqMP board. The first patch in the series
adds a model of the ZynqMP generic DMA (the ZDMA). The second patch in the
series connects the two instances of the ZDMA, the GDMA and the ADMA, to the
ZynqMP board.

Best regards,
Francisco Iglesias

Francisco Iglesias (2):
  xlnx-zdma: Add a model of the Xilinx ZynqMP generic DMA
  xlnx-zynqmp: Connect the ZynqMP GDMA and ADMA

 hw/arm/xlnx-zynqmp.c |  53 +++
 hw/dma/Makefile.objs |   1 +
 hw/dma/xlnx-zdma.c   | 833 +++
 include/hw/arm/xlnx-zynqmp.h |   5 +
 include/hw/dma/xlnx-zdma.h   |  84 +
 5 files changed, 976 insertions(+)
 create mode 100644 hw/dma/xlnx-zdma.c
 create mode 100644 include/hw/dma/xlnx-zdma.h

-- 
2.11.0

[Qemu-devel] [PATCH v1 1/2] xlnx-zdma: Add a model of the Xilinx ZynqMP generic DMA

[Francisco Iglesias]

Add a model of the generic DMA found on Xilinx ZynqMP.

Signed-off-by: Francisco Iglesias 
Signed-off-by: Edgar E. Iglesias 
---
 hw/dma/Makefile.objs   |   1 +
 hw/dma/xlnx-zdma.c | 833 +
 include/hw/dma/xlnx-zdma.h |  84 +
 3 files changed, 918 insertions(+)
 create mode 100644 hw/dma/xlnx-zdma.c
 create mode 100644 include/hw/dma/xlnx-zdma.h

diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index c2afecbf73..79affecc39 100644
--- a/hw/dma/Makefile.objs
+++ b/hw/dma/Makefile.objs
@@ -10,6 +10,7 @@ common-obj-$(CONFIG_ETRAXFS) += etraxfs_dma.o
 common-obj-$(CONFIG_STP2000) += sparc32_dma.o
 obj-$(CONFIG_XLNX_ZYNQMP) += xlnx_dpdma.o
 obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx_dpdma.o
+common-obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zdma.o
 
 obj-$(CONFIG_OMAP) += omap_dma.o soc_dma.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_dma.o
diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
new file mode 100644
index 00..7955a6116d
--- /dev/null
+++ b/hw/dma/xlnx-zdma.c
@@ -0,0 +1,833 @@
+/*
+ * QEMU model of the ZynqMP generic DMA
+ *
+ * Copyright (c) 2014 Xilinx Inc.
+ * Copyright (c) 2018 FEIMTECH AB
+ *
+ * Written by Edgar E. Iglesias ,
+ *Francisco Iglesias 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/dma/xlnx-zdma.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+
+#ifndef XLNX_ZDMA_ERR_DEBUG
+#define XLNX_ZDMA_ERR_DEBUG 0
+#endif
+
+REG32(ZDMA_ERR_CTRL, 0x0)
+FIELD(ZDMA_ERR_CTRL, APB_ERR_RES, 0, 1)
+REG32(ZDMA_CH_ISR, 0x100)
+FIELD(ZDMA_CH_ISR, DMA_PAUSE, 11, 1)
+FIELD(ZDMA_CH_ISR, DMA_DONE, 10, 1)
+FIELD(ZDMA_CH_ISR, AXI_WR_DATA, 9, 1)
+FIELD(ZDMA_CH_ISR, AXI_RD_DATA, 8, 1)
+FIELD(ZDMA_CH_ISR, AXI_RD_DST_DSCR, 7, 1)
+FIELD(ZDMA_CH_ISR, AXI_RD_SRC_DSCR, 6, 1)
+FIELD(ZDMA_CH_ISR, IRQ_DST_ACCT_ERR, 5, 1)
+FIELD(ZDMA_CH_ISR, IRQ_SRC_ACCT_ERR, 4, 1)
+FIELD(ZDMA_CH_ISR, BYTE_CNT_OVRFL, 3, 1)
+FIELD(ZDMA_CH_ISR, DST_DSCR_DONE, 2, 1)
+FIELD(ZDMA_CH_ISR, SRC_DSCR_DONE, 1, 1)
+FIELD(ZDMA_CH_ISR, INV_APB, 0, 1)
+REG32(ZDMA_CH_IMR, 0x104)
+FIELD(ZDMA_CH_IMR, DMA_PAUSE, 11, 1)
+FIELD(ZDMA_CH_IMR, DMA_DONE, 10, 1)
+FIELD(ZDMA_CH_IMR, AXI_WR_DATA, 9, 1)
+FIELD(ZDMA_CH_IMR, AXI_RD_DATA, 8, 1)
+FIELD(ZDMA_CH_IMR, AXI_RD_DST_DSCR, 7, 1)
+FIELD(ZDMA_CH_IMR, AXI_RD_SRC_DSCR, 6, 1)
+FIELD(ZDMA_CH_IMR, IRQ_DST_ACCT_ERR, 5, 1)
+FIELD(ZDMA_CH_IMR, IRQ_SRC_ACCT_ERR, 4, 1)
+FIELD(ZDMA_CH_IMR, BYTE_CNT_OVRFL, 3, 1)
+FIELD(ZDMA_CH_IMR, DST_DSCR_DONE, 2, 1)
+FIELD(ZDMA_CH_IMR, SRC_DSCR_DONE, 1, 1)
+FIELD(ZDMA_CH_IMR, INV_APB, 0, 1)
+REG32(ZDMA_CH_IEN, 0x108)
+FIELD(ZDMA_CH_IEN, DMA_PAUSE, 11, 1)
+FIELD(ZDMA_CH_IEN, DMA_DONE, 10, 1)
+FIELD(ZDMA_CH_IEN, AXI_WR_DATA, 9, 1)
+FIELD(ZDMA_CH_IEN, AXI_RD_DATA, 8, 1)
+FIELD(ZDMA_CH_IEN, AXI_RD_DST_DSCR, 7, 1)
+FIELD(ZDMA_CH_IEN, AXI_RD_SRC_DSCR, 6, 1)
+FIELD(ZDMA_CH_IEN, IRQ_DST_ACCT_ERR, 5, 1)
+FIELD(ZDMA_CH_IEN, IRQ_SRC_ACCT_ERR, 4, 1)
+FIELD(ZDMA_CH_IEN, BYTE_CNT_OVRFL, 3, 1)
+FIELD(ZDMA_CH_IEN, DST_DSCR_DONE, 2, 1)
+FIELD(ZDMA_CH_IEN, SRC_DSCR_DONE, 1, 1)
+FIELD(ZDMA_CH_IEN, INV_APB, 0, 1)
+REG32(ZDMA_CH_IDS, 0x10c)
+FIELD(ZDMA_CH_IDS, DMA_PAUSE, 11, 1)
+FIELD(ZDMA_CH_IDS, DMA_DONE, 10, 1)
+FIELD(ZDMA_CH_IDS, AXI_WR_DATA, 9, 1)
+FIELD(ZDMA_CH_IDS, AXI_RD_DATA, 8, 1)
+FIELD(ZDMA_CH_IDS, AXI_RD_DST_DSCR, 7, 1)
+FIELD(ZDMA_CH_IDS, AXI_RD_SRC_DSCR, 6, 1)
+FIELD(ZDMA_CH_IDS, IRQ_DST_ACCT_ERR, 5, 1)
+FIELD(ZDMA_CH_IDS, IRQ_SRC_ACCT_ERR, 4, 1)
+FIELD(ZDMA_CH_IDS, BYTE_CNT_OVRFL, 3, 1)
+FIELD(ZDMA_CH_IDS, DST_DSCR_DONE, 2, 1)
+FIELD(ZDMA_CH_IDS, SRC_DSCR_DONE, 1, 1)
+FIELD(ZDMA_CH_IDS, INV_APB, 0, 1)
+REG32(ZDMA_CH_CTRL0, 0x110)
+FIELD(ZDMA_CH_CTRL0, OVR_FETCH, 7, 1)
+FIELD(ZDMA_CH_CTRL0, POINT_TYPE, 6, 1)
+FIELD(ZDMA_C