Re: [Qemu-devel] [PULL 0/8] Monitor patches for 2018-10-30

[Peter Xu]

On Thu, Nov 08, 2018 at 10:44:06AM +0800, Peter Xu wrote:
> On Wed, Nov 07, 2018 at 11:21:32AM +0000, Peter Maydell wrote:
> > On 7 November 2018 at 02:56, Peter Xu <address@hidden> wrote:
> > > Strange, "make check -j8" failed on my hosts (I tried two) with either
> > > Markus's pull tree or qemu master:
> > >
> > > hw/core/ptimer.o: In function `timer_new_tl':
> > > /home/xz/git/qemu/include/qemu/timer.h:536: undefined reference to 
> > > `timer_init_tl'
> > > collect2: error: ld returned 1 exit status
> > > make: *** [/home/xz/git/qemu/rules.mak:124: tests/ptimer-test] Error 1
> > >
> > > Is that only happening on my hosts?
> > 
> > Commit 89a603a0c80ae3 changed things so that there is no
> > timer_new_tl() or timer_init_tl() any more, so if you have
> > an object file that's referring to it then it's probably
> > stale. Try a make clean.
> 
> Yeh it worked for me, thanks Peter.
> 
> Though after running a few more rounds of "configure --enable-debug &&
> make check -j8" I still cannot see anything wrong with Markus's tree.
> I'll see whether there's any news from Markus and then I'll consider
> whether I should install a FreeBSD.

I reproduced the error with a FreeBSD guest and this change (which
possibly can be squashed into "tests: qmp-test: add queue full test")
worked for me:

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 6989acbca4..83f353db4f 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -281,8 +281,15 @@ static void test_qmp_oob(void)
      * will only be able to be handled after the queue is shrinked, so
      * it'll be processed only after one existing in-band command
      * finishes.
+     *
+     * NOTE: we need to feed the queue with one extra request to make
+     * sure it'll be stuck since when we have sent the Nth request
+     * it's possible that we have already popped and processing the
+     * 1st request so the Nth request (which could potentially be the
+     * [N-1]th element on the queue) might not trigger the
+     * monitor-full condition deterministically.
      */
-    for (i = 1; i <= QMP_REQ_QUEUE_LEN_MAX; i++) {
+    for (i = 1; i <= QMP_REQ_QUEUE_LEN_MAX + 1; i++) {
         id = g_strdup_printf("queue-blocks-%d", i);
         send_cmd_that_blocks(qts, id);
         g_free(id);
@@ -291,7 +298,7 @@ static void test_qmp_oob(void)
     unblock_blocked_cmd();
     recv_cmd_id(qts, "queue-blocks-1");
     recv_cmd_id(qts, "oob-1");
-    for (i = 2; i <= QMP_REQ_QUEUE_LEN_MAX; i++) {
+    for (i = 2; i <= QMP_REQ_QUEUE_LEN_MAX + 1; i++) {
         unblock_blocked_cmd();
         id = g_strdup_printf("queue-blocks-%d", i);
         recv_cmd_id(qts, id);

So the problem here is that the queue-block-N command might not really
suspend the monitor everytime if we already popped the 1st request,
which will let the N-th request to be (N-1)th, then the parser will
continue to eat the oob command and it could "preempt" the previous
commands.

Maybe FreeBSD is scheduling the threads in some pattern so it happens
only on FreeBSD and very constantly, but anyway it should be a general
fix to the test program.

Regards,

-- 
Peter Xu