[Qemu-devel] [PATCH v3 00/15] mirror: follow AioContext change gracefully

[Qemu-devel] [PATCH v3 00/15] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

v3:
 * Push infrastructure down into blockjob.c so other jobs can reuse it [Stefan]
 * Tested with drive_mirror + migration [Stefan]

v2:
 * Fam introduced the concept of a synchronous aio_poll() loop to quiesce the
   block job during detach

When dataplane is enabled or disabled the drive switches to a new
AioContext.  The mirror block job must also move to the new AioContext
so that drive accesses are always made within its AioContext.

This series extends the block job pause functionality so that detaching from an
AioContext pauses the job and attaching to the new AioContext resumes the job.

Pause points are added to the mirror job so that long I/O loops can yield for
an AioContext switch.  Other block jobs need pause points too but this can be
done as a follow-up series.

Daniel P. Berrange (1):
  ui: fix regression in printing VNC host/port on startup

Gerd Hoffmann (2):
  vnc: drop unused depth arg for set_pixel_format
  console: ignore ui_info updates which don't actually update something

Olaf Hering (1):
  gtk: fix vte version check

Peter Maydell (6):
  translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()
  user-exec: Push resume-from-signal code out to handle_cpu_signal()
  cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()
  user-exec: Don't reextract sigmask from usercontext pointer
  target-i386: Add comment about do_interrupt_user() next_eip argument
  target-i386: Move user-mode exception actions out of user-exec.c

Stefan Hajnoczi (4):
  blockjob: move iostatus reset out of block_job_enter()
  blockjob: add pause points
  blockjob: add AioContext attach/detach callbacks
  mirror: follow AioContext change gracefully

Thomas Huth (1):
  ui/console-gl: Add support for big endian display surfaces

[Qemu-devel] [PATCH v3 01/15] translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()

[Stefan Hajnoczi]

From: Peter Maydell <peter.maydell@linaro.org>

The user-mode-only function tb_invalidate_phys_page() is only
called from two places:
 * page_unprotect(), which passes in a non-zero pc, a puc pointer
   and the value 'true' for the locked argument
 * page_set_flags(), which passes in a zero pc, a NULL puc pointer
   and a 'false' locked argument

If the pc is non-zero then we may call cpu_resume_from_signal(),
which does a longjmp out of the calling code (and out of the
signal handler); this is to cover the case of a target CPU with
"precise self-modifying code" (currently only x86) executing
a store instruction which modifies code in the same TB as the
store itself. Rather than doing the longjump directly here,
return a flag to the caller which indicates whether the current
TB was modified, and move the longjump to page_unprotect.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Riku Voipio <riku.voipio@linaro.org>
Message-id: 1463494687-25947-2-git-send-email-peter.maydell@linaro.org
---
 translate-all.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index c599dc4..2285961 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1439,10 +1439,13 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
     }
 }
 #else
-/* Called with mmap_lock held.  */
-static void tb_invalidate_phys_page(tb_page_addr_t addr,
-                                    uintptr_t pc, void *puc,
-                                    bool locked)
+/* Called with mmap_lock held. If pc is not 0 then it indicates the
+ * host PC of the faulting store instruction that caused this invalidate.
+ * Returns true if the caller needs to abort execution of the current
+ * TB (because it was modified by this store and the guest CPU has
+ * precise-SMC semantics).
+ */
+static bool tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc)
 {
     TranslationBlock *tb;
     PageDesc *p;
@@ -1460,7 +1463,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
     addr &= TARGET_PAGE_MASK;
     p = page_find(addr >> TARGET_PAGE_BITS);
     if (!p) {
-        return;
+        return false;
     }
     tb = p->first_tb;
 #ifdef TARGET_HAS_PRECISE_SMC
@@ -1499,12 +1502,10 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
            modifying the memory. It will ensure that it cannot modify
            itself */
         tb_gen_code(cpu, current_pc, current_cs_base, current_flags, 1);
-        if (locked) {
-            mmap_unlock();
-        }
-        cpu_resume_from_signal(cpu, puc);
+        return true;
     }
 #endif
+    return false;
 }
 #endif
 
@@ -1902,7 +1903,7 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
         if (!(p->flags & PAGE_WRITE) &&
             (flags & PAGE_WRITE) &&
             p->first_tb) {
-            tb_invalidate_phys_page(addr, 0, NULL, false);
+            tb_invalidate_phys_page(addr, 0);
         }
         p->flags = flags;
     }
@@ -1996,7 +1997,10 @@ int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
 
             /* and since the content will be modified, we must invalidate
                the corresponding translated code. */
-            tb_invalidate_phys_page(addr, pc, puc, true);
+            if (tb_invalidate_phys_page(addr, pc)) {
+                mmap_unlock();
+                cpu_resume_from_signal(current_cpu, puc);
+            }
 #ifdef DEBUG_TB_CHECK
             tb_invalidate_check(addr);
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 02/15] user-exec: Push resume-from-signal code out to handle_cpu_signal()

[Stefan Hajnoczi]

From: Peter Maydell <peter.maydell@linaro.org>

Since the only caller of page_unprotect() which might cause it to
need to call cpu_resume_from_signal() is handle_cpu_signal() in
the user-mode code, push the longjump handling out to that function.

Since this is the only caller of cpu_resume_from_signal() which
passes a non-NULL puc argument, split the non-NULL handling into
a new cpu_exit_tb_from_sighandler() function. This allows us
to merge the softmmu and usermode implementations of the
cpu_resume_from_signal() function, which are now identical.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Riku Voipio <riku.voipio@linaro.org>
Message-id: 1463494687-25947-3-git-send-email-peter.maydell@linaro.org
---
 cpu-exec-common.c |  2 +-
 translate-all.c   | 12 ++++++++----
 translate-all.h   |  2 +-
 user-exec.c       | 41 +++++++++++++++++++++++++++++------------
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/cpu-exec-common.c b/cpu-exec-common.c
index 132cd03..0cb5b63 100644
--- a/cpu-exec-common.c
+++ b/cpu-exec-common.c
@@ -29,7 +29,6 @@ CPUState *tcg_current_cpu;
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-#if defined(CONFIG_SOFTMMU)
 void cpu_resume_from_signal(CPUState *cpu, void *puc)
 {
     /* XXX: restore cpu registers saved in host registers */
@@ -38,6 +37,7 @@ void cpu_resume_from_signal(CPUState *cpu, void *puc)
     siglongjmp(cpu->jmp_env, 1);
 }
 
+#if defined(CONFIG_SOFTMMU)
 void cpu_reloading_memory_map(void)
 {
     if (qemu_in_vcpu_thread()) {
diff --git a/translate-all.c b/translate-all.c
index 2285961..ff588f3 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1955,7 +1955,7 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
             /* unprotect the page if it was put read-only because it
                contains translated code */
             if (!(p->flags & PAGE_WRITE)) {
-                if (!page_unprotect(addr, 0, NULL)) {
+                if (!page_unprotect(addr, 0)) {
                     return -1;
                 }
             }
@@ -1965,8 +1965,12 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
 }
 
 /* called from signal handler: invalidate the code and unprotect the
-   page. Return TRUE if the fault was successfully handled. */
-int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
+ * page. Return 0 if the fault was not handled, 1 if it was handled,
+ * and 2 if it was handled but the caller must cause the TB to be
+ * immediately exited. (We can only return 2 if the 'pc' argument is
+ * non-zero.)
+ */
+int page_unprotect(target_ulong address, uintptr_t pc)
 {
     unsigned int prot;
     PageDesc *p;
@@ -1999,7 +2003,7 @@ int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
                the corresponding translated code. */
             if (tb_invalidate_phys_page(addr, pc)) {
                 mmap_unlock();
-                cpu_resume_from_signal(current_cpu, puc);
+                return 2;
             }
 #ifdef DEBUG_TB_CHECK
             tb_invalidate_check(addr);
diff --git a/translate-all.h b/translate-all.h
index 0384640..ce6071b 100644
--- a/translate-all.h
+++ b/translate-all.h
@@ -27,7 +27,7 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end);
 void tb_check_watchpoint(CPUState *cpu);
 
 #ifdef CONFIG_USER_ONLY
-int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
+int page_unprotect(target_ulong address, uintptr_t pc);
 #endif
 
 #endif /* TRANSLATE_ALL_H */
diff --git a/user-exec.c b/user-exec.c
index c809daa..efd34a6 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -55,7 +55,7 @@ static void exception_action(CPUState *cpu)
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-void cpu_resume_from_signal(CPUState *cpu, void *puc)
+static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
 {
 #ifdef __linux__
     struct ucontext *uc = puc;
@@ -63,20 +63,18 @@ void cpu_resume_from_signal(CPUState *cpu, void *puc)
     struct sigcontext *uc = puc;
 #endif
 
-    if (puc) {
-        /* XXX: use siglongjmp ? */
+    /* XXX: use siglongjmp ? */
 #ifdef __linux__
 #ifdef __ia64
-        sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
+    sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
 #else
-        sigprocmask(SIG_SETMASK, &uc->uc_sigmask, NULL);
+    sigprocmask(SIG_SETMASK, &uc->uc_sigmask, NULL);
 #endif
 #elif defined(__OpenBSD__)
-        sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
+    sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
-    }
-    cpu->exception_index = -1;
-    siglongjmp(cpu->jmp_env, 1);
+
+    cpu_resume_from_signal(cpu, NULL);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is
@@ -96,9 +94,28 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
            pc, address, is_write, *(unsigned long *)old_set);
 #endif
     /* XXX: locking issue */
-    if (is_write && h2g_valid(address)
-        && page_unprotect(h2g(address), pc, puc)) {
-        return 1;
+    if (is_write && h2g_valid(address)) {
+        switch (page_unprotect(h2g(address), pc)) {
+        case 0:
+            /* Fault not caused by a page marked unwritable to protect
+             * cached translations, must be the guest binary's problem
+             */
+            break;
+        case 1:
+            /* Fault caused by protection of cached translation; TBs
+             * invalidated, so resume execution
+             */
+            return 1;
+        case 2:
+            /* Fault caused by protection of cached translation, and the
+             * currently executing TB was modified and must be exited
+             * immediately.
+             */
+            cpu_exit_tb_from_sighandler(current_cpu, puc);
+            g_assert_not_reached();
+        default:
+            g_assert_not_reached();
+        }
     }
 
     /* Convert forcefully to guest address space, invalid addresses
-- 
2.5.5

[Qemu-devel] [PATCH v3 03/15] cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()

[Stefan Hajnoczi]

From: Peter Maydell <peter.maydell@linaro.org>

The function cpu_resume_from_signal() is now always called with a
NULL puc argument, and is rather misnamed since it is never called
from a signal handler. It is essentially forcing an exit to the
top level cpu loop but without raising any exception, so rename
it to cpu_loop_exit_noexc() and drop the useless unused argument.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Riku Voipio <riku.voipio@linaro.org>
Message-id: 1463494687-25947-4-git-send-email-peter.maydell@linaro.org
---
 cpu-exec-common.c        | 6 ++----
 exec.c                   | 2 +-
 hw/i386/kvmvapic.c       | 2 +-
 include/exec/exec-all.h  | 2 +-
 target-i386/bpt_helper.c | 2 +-
 target-lm32/helper.c     | 2 +-
 target-s390x/helper.c    | 2 +-
 target-xtensa/helper.c   | 2 +-
 translate-all.c          | 4 ++--
 user-exec.c              | 2 +-
 10 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/cpu-exec-common.c b/cpu-exec-common.c
index 0cb5b63..0cb4ae6 100644
--- a/cpu-exec-common.c
+++ b/cpu-exec-common.c
@@ -26,10 +26,8 @@
 bool exit_request;
 CPUState *tcg_current_cpu;
 
-/* exit the current TB from a signal handler. The host registers are
-   restored in a state compatible with the CPU emulator
- */
-void cpu_resume_from_signal(CPUState *cpu, void *puc)
+/* exit the current TB, but without causing any exception to be raised */
+void cpu_loop_exit_noexc(CPUState *cpu)
 {
     /* XXX: restore cpu registers saved in host registers */
 
diff --git a/exec.c b/exec.c
index f2c9e37..a9d465b 100644
--- a/exec.c
+++ b/exec.c
@@ -2091,7 +2091,7 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
                 } else {
                     cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags);
                     tb_gen_code(cpu, pc, cs_base, cpu_flags, 1);
-                    cpu_resume_from_signal(cpu, NULL);
+                    cpu_loop_exit_noexc(cpu);
                 }
             }
         } else {
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 5b71b1b..3bf1ddd 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -450,7 +450,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
 
     if (!kvm_enabled()) {
         tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
-        cpu_resume_from_signal(cs, NULL);
+        cpu_loop_exit_noexc(cs);
     }
 }
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index b6a4a12..e076397 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -50,7 +50,7 @@ void restore_state_to_opc(CPUArchState *env, struct TranslationBlock *tb,
 void cpu_gen_init(void);
 bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc);
 
-void QEMU_NORETURN cpu_resume_from_signal(CPUState *cpu, void *puc);
+void QEMU_NORETURN cpu_loop_exit_noexc(CPUState *cpu);
 void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base,
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 499a277..6fd7fe0 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -218,7 +218,7 @@ void breakpoint_handler(CPUState *cs)
             if (check_hw_breakpoints(env, false)) {
                 raise_exception(env, EXCP01_DB);
             } else {
-                cpu_resume_from_signal(cs, NULL);
+                cpu_loop_exit_noexc(cs);
             }
         }
     } else {
diff --git a/target-lm32/helper.c b/target-lm32/helper.c
index b8f4ed9..891da18 100644
--- a/target-lm32/helper.c
+++ b/target-lm32/helper.c
@@ -141,7 +141,7 @@ void lm32_debug_excp_handler(CPUState *cs)
             if (check_watchpoints(env)) {
                 raise_exception(env, EXCP_WATCHPOINT);
             } else {
-                cpu_resume_from_signal(cs, NULL);
+                cpu_loop_exit_noexc(cs);
             }
         }
     } else {
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index ad8f797..9a744df 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -688,7 +688,7 @@ void s390x_cpu_debug_excp_handler(CPUState *cs)
            will be triggered, it will call load_psw which will recompute
            the watchpoints.  */
         cpu_watchpoint_remove_all(cs, BP_CPU);
-        cpu_resume_from_signal(cs, NULL);
+        cpu_loop_exit_noexc(cs);
     }
 }
 #endif /* CONFIG_USER_ONLY */
diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index 839f4a7..768b32c 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -108,7 +108,7 @@ void xtensa_breakpoint_handler(CPUState *cs)
             if (cause) {
                 debug_exception_env(env, cause);
             }
-            cpu_resume_from_signal(cs, NULL);
+            cpu_loop_exit_noexc(cs);
         }
     }
 }
diff --git a/translate-all.c b/translate-all.c
index ff588f3..118e7d3 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1395,7 +1395,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
            modifying the memory. It will ensure that it cannot modify
            itself */
         tb_gen_code(cpu, current_pc, current_cs_base, current_flags, 1);
-        cpu_resume_from_signal(cpu, NULL);
+        cpu_loop_exit_noexc(cpu);
     }
 #endif
 }
@@ -1654,7 +1654,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
        repeating the fault, which is horribly inefficient.
        Better would be to execute just this insn uncached, or generate a
        second new TB.  */
-    cpu_resume_from_signal(cpu, NULL);
+    cpu_loop_exit_noexc(cpu);
 }
 
 void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr)
diff --git a/user-exec.c b/user-exec.c
index efd34a6..c5179a4 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -74,7 +74,7 @@ static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
     sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
 
-    cpu_resume_from_signal(cpu, NULL);
+    cpu_loop_exit_noexc(cpu);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is
-- 
2.5.5

[Qemu-devel] [PATCH v3 04/15] user-exec: Don't reextract sigmask from usercontext pointer

[Stefan Hajnoczi]

From: Peter Maydell <peter.maydell@linaro.org>

Extracting the old signal mask from the usercontext pointer passed to
a signal handler is a pain because it is OS and CPU dependent.
Since we've already done it once and passed it to handle_cpu_signal(),
there's no need to do it again in cpu_exit_tb_from_sighandler().
This then means we don't need to pass a usercontext pointer in to
handle_cpu_signal() at all.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Riku Voipio <riku.voipio@linaro.org>
Message-id: 1463494687-25947-5-git-send-email-peter.maydell@linaro.org
---
 user-exec.c | 48 ++++++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/user-exec.c b/user-exec.c
index c5179a4..b9e7bec 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -55,25 +55,10 @@ static void exception_action(CPUState *cpu)
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
+static void cpu_exit_tb_from_sighandler(CPUState *cpu, sigset_t *old_set)
 {
-#ifdef __linux__
-    struct ucontext *uc = puc;
-#elif defined(__OpenBSD__)
-    struct sigcontext *uc = puc;
-#endif
-
     /* XXX: use siglongjmp ? */
-#ifdef __linux__
-#ifdef __ia64
-    sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
-#else
-    sigprocmask(SIG_SETMASK, &uc->uc_sigmask, NULL);
-#endif
-#elif defined(__OpenBSD__)
-    sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
-#endif
-
+    sigprocmask(SIG_SETMASK, old_set, NULL);
     cpu_loop_exit_noexc(cpu);
 }
 
@@ -82,8 +67,7 @@ static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
    write caused the exception and otherwise 0'. 'old_set' is the
    signal set which should be restored */
 static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
-                                    int is_write, sigset_t *old_set,
-                                    void *puc)
+                                    int is_write, sigset_t *old_set)
 {
     CPUState *cpu;
     CPUClass *cc;
@@ -111,7 +95,7 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
              * currently executing TB was modified and must be exited
              * immediately.
              */
-            cpu_exit_tb_from_sighandler(current_cpu, puc);
+            cpu_exit_tb_from_sighandler(current_cpu, old_set);
             g_assert_not_reached();
         default:
             g_assert_not_reached();
@@ -205,7 +189,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              trapno == 0xe ?
                              (ERROR_sig(uc) >> 1) & 1 : 0,
-                             &MASK_sig(uc), puc);
+                             &MASK_sig(uc));
 }
 
 #elif defined(__x86_64__)
@@ -251,7 +235,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              TRAP_sig(uc) == 0xe ?
                              (ERROR_sig(uc) >> 1) & 1 : 0,
-                             &MASK_sig(uc), puc);
+                             &MASK_sig(uc));
 }
 
 #elif defined(_ARCH_PPC)
@@ -367,7 +351,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     }
 #endif
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__alpha__)
@@ -398,7 +382,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     }
 
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 #elif defined(__sparc__)
 
@@ -458,7 +442,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
         }
     }
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, sigmask, NULL);
+                             is_write, sigmask);
 }
 
 #elif defined(__arm__)
@@ -493,7 +477,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              is_write,
-                             &uc->uc_sigmask, puc);
+                             &uc->uc_sigmask);
 }
 
 #elif defined(__aarch64__)
@@ -521,7 +505,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
                 || (insn & 0x3a400000) == 0x28000000); /* C3.3.7,14-16 */
 
     return handle_cpu_signal(pc, (uintptr_t)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__mc68000)
@@ -539,7 +523,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     is_write = 0;
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              is_write,
-                             &uc->uc_sigmask, puc);
+                             &uc->uc_sigmask);
 }
 
 #elif defined(__ia64)
@@ -574,7 +558,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
     }
     return handle_cpu_signal(ip, (unsigned long)info->si_addr,
                              is_write,
-                             (sigset_t *)&uc->uc_sigmask, puc);
+                             (sigset_t *)&uc->uc_sigmask);
 }
 
 #elif defined(__s390__)
@@ -627,7 +611,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
         break;
     }
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__mips__)
@@ -643,7 +627,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     /* XXX: compute is_write */
     is_write = 0;
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__hppa__)
@@ -685,7 +669,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     }
 
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #else
-- 
2.5.5

[Qemu-devel] [PATCH v3 05/15] target-i386: Add comment about do_interrupt_user() next_eip argument

[Stefan Hajnoczi]

From: Peter Maydell <peter.maydell@linaro.org>

Add a comment to do_interrupt_user() along the same lines as the
existing one for do_interrupt_all() noting that the next_eip
argument is not used unless is_int is true or intno is EXCP_SYSCALL.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Riku Voipio <riku.voipio@linaro.org>
Message-id: 1463494687-25947-6-git-send-email-peter.maydell@linaro.org
---
 target-i386/seg_helper.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 97aee09..6cbdf17 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -1129,7 +1129,11 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
 }
 
 #if defined(CONFIG_USER_ONLY)
-/* fake user mode interrupt */
+/* fake user mode interrupt. is_int is TRUE if coming from the int
+ * instruction. next_eip is the env->eip value AFTER the interrupt
+ * instruction. It is only relevant if is_int is TRUE or if intno
+ * is EXCP_SYSCALL.
+ */
 static void do_interrupt_user(CPUX86State *env, int intno, int is_int,
                               int error_code, target_ulong next_eip)
 {
-- 
2.5.5

[Qemu-devel] [PATCH v3 06/15] target-i386: Move user-mode exception actions out of user-exec.c

[Stefan Hajnoczi]

From: Peter Maydell <peter.maydell@linaro.org>

The exception_action() function in user-exec.c is just a call to
cpu_loop_exit() for every target CPU except i386.  Since this
function is only called if the target's handle_mmu_fault() hook has
indicated an MMU fault, and that hook is only called from the
handle_cpu_signal() code path, we can simply move the x86-specific
setup into that hook, which allows us to remove the TARGET_I386
ifdef from user-exec.c.

Of the actions that were done by the call to raise_interrupt_err():
 * cpu_svm_check_intercept_param() is a no-op in user mode
 * check_exception() is a no-op since double faults are impossible
   for user-mode
 * assignments to cs->exception_index and env->error_code are no-ops
 * assigning to env->exception_next_eip is unnecessary because it
   is not used unless env->exception_is_int is true
 * cpu_loop_exit_restore() is equivalent to cpu_loop_exit() since
   pc is 0
which leaves just setting env_>exception_is_int as the action that
needs to be added to x86_cpu_handle_mmu_fault().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Riku Voipio <riku.voipio@linaro.org>
Message-id: 1463494687-25947-7-git-send-email-peter.maydell@linaro.org
---
 target-i386/helper.c |  2 ++
 user-exec.c          | 16 +---------------
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 889fdab..1c250b8 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -701,6 +701,8 @@ int x86_cpu_handle_mmu_fault(CPUState *cs, vaddr addr,
     env->error_code = (is_write << PG_ERROR_W_BIT);
     env->error_code |= PG_ERROR_U_MASK;
     cs->exception_index = EXCP0E_PAGE;
+    env->exception_is_int = 0;
+    env->exception_next_eip = -1;
     return 1;
 }
 
diff --git a/user-exec.c b/user-exec.c
index b9e7bec..50e95a6 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -40,18 +40,6 @@
 
 //#define DEBUG_SIGNAL
 
-static void exception_action(CPUState *cpu)
-{
-#if defined(TARGET_I386)
-    X86CPU *x86_cpu = X86_CPU(cpu);
-    CPUX86State *env1 = &x86_cpu->env;
-
-    raise_exception_err(env1, cpu->exception_index, env1->error_code);
-#else
-    cpu_loop_exit(cpu);
-#endif
-}
-
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
@@ -120,10 +108,8 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
     /* now we have a real cpu fault */
     cpu_restore_state(cpu, pc);
 
-    /* we restore the process signal mask as the sigreturn should
-       do it (XXX: use sigsetjmp) */
     sigprocmask(SIG_SETMASK, old_set, NULL);
-    exception_action(cpu);
+    cpu_loop_exit(cpu);
 
     /* never comes here */
     return 1;
-- 
2.5.5

[Qemu-devel] [PATCH v3 07/15] vnc: drop unused depth arg for set_pixel_format

[Stefan Hajnoczi]

From: Gerd Hoffmann <kraxel@redhat.com>

Spotted by Coverity.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1465204725-31562-1-git-send-email-kraxel@redhat.com
---
 ui/vnc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index c862fdc..942cfb9 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2115,8 +2115,7 @@ static void send_color_map(VncState *vs)
     }
 }
 
-static void set_pixel_format(VncState *vs,
-                             int bits_per_pixel, int depth,
+static void set_pixel_format(VncState *vs, int bits_per_pixel,
                              int big_endian_flag, int true_color_flag,
                              int red_max, int green_max, int blue_max,
                              int red_shift, int green_shift, int blue_shift)
@@ -2124,7 +2123,6 @@ static void set_pixel_format(VncState *vs,
     if (!true_color_flag) {
         /* Expose a reasonable default 256 color map */
         bits_per_pixel = 8;
-        depth = 8;
         red_max = 7;
         green_max = 7;
         blue_max = 3;
@@ -2231,7 +2229,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
         if (len == 1)
             return 20;
 
-        set_pixel_format(vs, read_u8(data, 4), read_u8(data, 5),
+        set_pixel_format(vs, read_u8(data, 4),
                          read_u8(data, 6), read_u8(data, 7),
                          read_u16(data, 8), read_u16(data, 10),
                          read_u16(data, 12), read_u8(data, 14),
-- 
2.5.5

[Qemu-devel] [PATCH v3 08/15] ui: fix regression in printing VNC host/port on startup

[Stefan Hajnoczi]

From: "Daniel P. Berrange" <berrange@redhat.com>

If VNC is chosen as the compile time default display backend,
QEMU will print the host/port it listens on at startup.
Previously this would look like

  VNC server running on '::1:5900'

but in 04d2529da27db512dcbd5e99d0e26d333f16efcc the ':' was
accidentally replaced with a ';'. This the ':' back.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1465382576-25552-1-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 942cfb9..95e4db7 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3223,7 +3223,7 @@ char *vnc_display_local_addr(const char *id)
         qapi_free_SocketAddress(addr);
         return NULL;
     }
-    ret = g_strdup_printf("%s;%s", addr->u.inet.data->host,
+    ret = g_strdup_printf("%s:%s", addr->u.inet.data->host,
                           addr->u.inet.data->port);
     qapi_free_SocketAddress(addr);
 
-- 
2.5.5

[Qemu-devel] [PATCH v3 09/15] gtk: fix vte version check

[Stefan Hajnoczi]

From: Olaf Hering <olaf@aepfle.de>

vte_terminal_set_encoding takes 3 args since 0.38.0.
This fixes commit fba958c6 ("gtk: implement set_echo")

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Message-id: 20160608214352.32669-1-olaf@aepfle.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 01b8216..58d20ee 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1748,7 +1748,7 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc,
     /* The documentation says that the default is UTF-8, but actually it is
      * 7-bit ASCII at least in VTE 0.38.
      */
-#if VTE_CHECK_VERSION(0, 40, 0)
+#if VTE_CHECK_VERSION(0, 38, 0)
     vte_terminal_set_encoding(VTE_TERMINAL(vc->vte.terminal), "UTF-8", NULL);
 #else
     vte_terminal_set_encoding(VTE_TERMINAL(vc->vte.terminal), "UTF-8");
-- 
2.5.5

[Qemu-devel] [PATCH v3 10/15] ui/console-gl: Add support for big endian display surfaces

[Stefan Hajnoczi]

From: Thomas Huth <thuth@redhat.com>

This is required for running QEMU on big endian hosts (like
PowerPC machines) that use RGB instead of BGR byte ordering.

Ticket: https://bugs.launchpad.net/qemu/+bug/1581796
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1465243261-26731-1-git-send-email-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/console-gl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ui/console-gl.c b/ui/console-gl.c
index 74b1bed..5165e21 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -88,6 +88,11 @@ void surface_gl_create_texture(ConsoleGLState *gls,
         surface->glformat = GL_BGRA_EXT;
         surface->gltype = GL_UNSIGNED_BYTE;
         break;
+    case PIXMAN_BE_x8r8g8b8:
+    case PIXMAN_BE_a8r8g8b8:
+        surface->glformat = GL_RGBA;
+        surface->gltype = GL_UNSIGNED_BYTE;
+        break;
     case PIXMAN_r5g6b5:
         surface->glformat = GL_RGB;
         surface->gltype = GL_UNSIGNED_SHORT_5_6_5;
-- 
2.5.5

[Qemu-devel] [PATCH v3 11/15] console: ignore ui_info updates which don't actually update something

[Stefan Hajnoczi]

From: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1464597673-26464-1-git-send-email-kraxel@redhat.com
---
 ui/console.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ui/console.c b/ui/console.c
index bf38579..ce1e105 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1453,16 +1453,21 @@ bool dpy_ui_info_supported(QemuConsole *con)
 int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info)
 {
     assert(con != NULL);
-    con->ui_info = *info;
+
     if (!dpy_ui_info_supported(con)) {
         return -1;
     }
+    if (memcmp(&con->ui_info, info, sizeof(con->ui_info)) == 0) {
+        /* nothing changed -- ignore */
+        return 0;
+    }
 
     /*
      * Typically we get a flood of these as the user resizes the window.
      * Wait until the dust has settled (one second without updates), then
      * go notify the guest.
      */
+    con->ui_info = *info;
     timer_mod(con->ui_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1000);
     return 0;
 }
-- 
2.5.5

[Qemu-devel] [PATCH v3 12/15] blockjob: move iostatus reset out of block_job_enter()

[Stefan Hajnoczi]

The QMP block-job-resume command and cancellation may want to reset the
job's iostatus.  The next patches add a user who does not want to reset
iostatus so move it up to block_job_enter() callers.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 1 +
 blockjob.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 7fd515a..19b963c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3799,6 +3799,7 @@ void qmp_block_job_resume(const char *device, Error **errp)
 
     job->user_paused = false;
     trace_qmp_block_job_resume(job);
+    block_job_iostatus_reset(job);
     block_job_resume(job);
     aio_context_release(aio_context);
 }
diff --git a/blockjob.c b/blockjob.c
index c095cc5..463bccf 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -269,7 +269,6 @@ void block_job_resume(BlockJob *job)
 
 void block_job_enter(BlockJob *job)
 {
-    block_job_iostatus_reset(job);
     if (job->co && !job->busy) {
         qemu_coroutine_enter(job->co, NULL);
     }
@@ -278,6 +277,7 @@ void block_job_enter(BlockJob *job)
 void block_job_cancel(BlockJob *job)
 {
     job->cancelled = true;
+    block_job_iostatus_reset(job);
     block_job_enter(job);
 }
 
-- 
2.5.5

[Qemu-devel] [PATCH v3 13/15] blockjob: add pause points

[Stefan Hajnoczi]

Block jobs are coroutines that usually perform I/O but sometimes also
sleep or yield.  Currently only sleeping or yielded block jobs can be
paused.  This means jobs that do not sleep or yield (using
block_job_yield()) are unaffected by block_job_pause().

Add block_job_pause_point() so that block jobs can mark quiescent points
that are suitable for pausing.  This solves the problem that is can take
a block job a long time to pause if it is performing a long series of
I/O operations.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockjob.c               |  7 +++++++
 include/block/blockjob.h | 12 ++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 463bccf..b810d73 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -247,6 +247,13 @@ void block_job_complete(BlockJob *job, Error **errp)
     job->driver->complete(job, errp);
 }
 
+void block_job_pause_point(BlockJob *job)
+{
+    if (block_job_is_paused(job)) {
+        block_job_yield(job);
+    }
+}
+
 void block_job_pause(BlockJob *job)
 {
     job->pause_count++;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 00ac418..f83a4f0 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -299,6 +299,18 @@ bool block_job_is_cancelled(BlockJob *job);
 BlockJobInfo *block_job_query(BlockJob *job);
 
 /**
+ * block_job_pause_point:
+ * @job: The job that is ready to pause.
+ *
+ * Pause now if block_job_pause() has been called.  Block jobs that perform
+ * lots of I/O must call this between requests so that the job can be paused.
+ *
+ * There must be no I/O in flight when this function is called.  There must be
+ * no I/O or event loop activity until after this function returns.
+ */
+void coroutine_fn block_job_pause_point(BlockJob *job);
+
+/**
  * block_job_pause:
  * @job: The job to be paused.
  *
-- 
2.5.5

[Qemu-devel] [PATCH v3 14/15] blockjob: add AioContext attach/detach callbacks

[Stefan Hajnoczi]

Block jobs need callbacks to get their affairs in order when the
AioContext is switched.  Simple block jobs can get away without
implementing these callbacks.

The callbacks are needed if the block job accesses other
BlockDriverStates.  Other BDSes need to be moved to the new AioContext
in the attach callback.

The detach callback must be used to quiesce asynchronous I/O.  Although
bdrv_set_aio_context() internally calls bdrv_drain(), this isn't enough
when multiple BDSes are accessed by the job:

When completing requests on one BDS submits new requests on another BDS,
especially if this is cyclical, then a custom detach callback is needed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockjob.c               | 33 +++++++++++++++++++++++++++++++++
 include/block/blockjob.h | 14 ++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index b810d73..dd384fe 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -60,6 +60,33 @@ BlockJob *block_job_next(BlockJob *job)
     return QLIST_NEXT(job, job_list);
 }
 
+static void block_job_attached_aio_context(AioContext *new_context,
+                                           void *opaque)
+{
+    BlockJob *job = opaque;
+
+    if (job->driver->attached_aio_context) {
+        job->driver->attached_aio_context(job, new_context);
+    }
+
+    block_job_resume(job);
+}
+
+static void block_job_detach_aio_context(void *opaque)
+{
+    BlockJob *job = opaque;
+
+    block_job_pause(job);
+
+    if (job->driver->detach_aio_context) {
+        job->driver->detach_aio_context(job);
+    }
+
+    while (job->busy) {
+        aio_poll(blk_get_aio_context(job->blk), true);
+    }
+}
+
 void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
                        int64_t speed, BlockCompletionFunc *cb,
                        void *opaque, Error **errp)
@@ -92,6 +119,9 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
 
     QLIST_INSERT_HEAD(&block_jobs, job, job_list);
 
+    blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
+                                 block_job_detach_aio_context, job);
+
     /* Only set speed when necessary to avoid NotSupported error */
     if (speed != 0) {
         Error *local_err = NULL;
@@ -117,6 +147,9 @@ void block_job_unref(BlockJob *job)
         BlockDriverState *bs = blk_bs(job->blk);
         bs->job = NULL;
         bdrv_op_unblock_all(bs, job->blocker);
+        blk_remove_aio_context_notifier(job->blk,
+                                        block_job_attached_aio_context,
+                                        block_job_detach_aio_context, job);
         blk_unref(job->blk);
         error_free(job->blocker);
         g_free(job->id);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index f83a4f0..604aff8 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -70,6 +70,20 @@ typedef struct BlockJobDriver {
      * never both.
      */
     void (*abort)(BlockJob *job);
+
+    /**
+     * If the callback is not NULL, it will be invoked before the job is
+     * resumed in a new AioContext.  This is the place to move any resources
+     * besides job->blk to the new AioContext.
+     */
+    void (*attached_aio_context)(BlockJob *job, AioContext *new_context);
+
+    /**
+     * If the callback is not NULL, it will be invoked after the job is paused
+     * but before job->blk is detached from the old AioContext.  This is the
+     * place to complete all asynchronous I/O that is in flight.
+     */
+    void (*detach_aio_context)(BlockJob *job);
 } BlockJobDriver;
 
 /**
-- 
2.5.5

[Qemu-devel] [PATCH v3 15/15] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

Add block_job_pause_point() calls to mark quiescent points and make sure
to complete in-flight requests when switching AioContexts.

This patch solves undefined behavior in the mirror block job when the
BDS AioContext is changed by dataplane.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/mirror.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 80fd3c7..046e95c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -331,6 +331,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         mirror_wait_for_io(s);
     }
 
+    block_job_pause_point(&s->common);
+
     /* Find the number of consective dirty chunks following the first dirty
      * one, and wait for in flight requests in them. */
     while (nb_chunks * sectors_per_chunk < (s->buf_size >> BDRV_SECTOR_BITS)) {
@@ -581,6 +583,8 @@ static void coroutine_fn mirror_run(void *opaque)
             if (now - last_pause_ns > SLICE_TIME) {
                 last_pause_ns = now;
                 block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
+            } else {
+                block_job_pause_point(&s->common);
             }
 
             if (block_job_is_cancelled(&s->common)) {
@@ -612,6 +616,8 @@ static void coroutine_fn mirror_run(void *opaque)
             goto immediate_exit;
         }
 
+        block_job_pause_point(&s->common);
+
         cnt = bdrv_get_dirty_count(s->dirty_bitmap);
         /* s->common.offset contains the number of bytes already processed so
          * far, cnt is the number of dirty sectors remaining and
@@ -781,18 +787,38 @@ static void mirror_complete(BlockJob *job, Error **errp)
     block_job_enter(&s->common);
 }
 
+static void mirror_attached_aio_context(BlockJob *job, AioContext *new_context)
+{
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+    blk_set_aio_context(s->target, new_context);
+}
+
+static void mirror_detach_aio_context(BlockJob *job)
+{
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+    while (s->in_flight > 0) {
+        aio_poll(blk_get_aio_context(job->blk), true);
+    }
+}
+
 static const BlockJobDriver mirror_job_driver = {
-    .instance_size = sizeof(MirrorBlockJob),
-    .job_type      = BLOCK_JOB_TYPE_MIRROR,
-    .set_speed     = mirror_set_speed,
-    .complete      = mirror_complete,
+    .instance_size          = sizeof(MirrorBlockJob),
+    .job_type               = BLOCK_JOB_TYPE_MIRROR,
+    .set_speed              = mirror_set_speed,
+    .complete               = mirror_complete,
+    .attached_aio_context   = mirror_attached_aio_context,
+    .detach_aio_context     = mirror_detach_aio_context,
 };
 
 static const BlockJobDriver commit_active_job_driver = {
-    .instance_size = sizeof(MirrorBlockJob),
-    .job_type      = BLOCK_JOB_TYPE_COMMIT,
-    .set_speed     = mirror_set_speed,
-    .complete      = mirror_complete,
+    .instance_size          = sizeof(MirrorBlockJob),
+    .job_type               = BLOCK_JOB_TYPE_COMMIT,
+    .set_speed              = mirror_set_speed,
+    .complete               = mirror_complete,
+    .attached_aio_context   = mirror_attached_aio_context,
+    .detach_aio_context     = mirror_detach_aio_context,
 };
 
 static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
-- 
2.5.5

Re: [Qemu-devel] [PATCH v3 15/15] mirror: follow AioContext change gracefully

[Paolo Bonzini]

On 13/06/2016 19:05, Stefan Hajnoczi wrote:
> index 80fd3c7..046e95c 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -331,6 +331,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>          mirror_wait_for_io(s);
>      }
>  
> +    block_job_pause_point(&s->common);

I/O can be in-flight here, so your description of the function is not
100% accurate.  I suppose it's okay because s->waiting_for_io is false
at the pause points you specified?

I guess a comment that explains it will do; and apart from this detail,
the patches are really nice.  Or maybe it's just that you scared with
that 00/15 in the cover letter. :->

Paolo

>          cnt = bdrv_get_dirty_count(s->dirty_bitmap);
>          /* s->common.offset contains the number of bytes already processed so
>           * far, cnt is the number of dirty sectors remaining and
> @@ -781,18 +787,38 @@ static void mirror_complete(BlockJob *job, Error **errp)
>      block_job_enter(&s->common);
>  }

Re: [Qemu-devel] [PATCH v3 15/15] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

On Tue, Jun 14, 2016 at 1:09 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 13/06/2016 19:05, Stefan Hajnoczi wrote:
>> index 80fd3c7..046e95c 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -331,6 +331,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>>          mirror_wait_for_io(s);
>>      }
>>
>> +    block_job_pause_point(&s->common);
>
> I/O can be in-flight here, so your description of the function is not
> 100% accurate.  I suppose it's okay because s->waiting_for_io is false
> at the pause points you specified?

You are right, this brings up a subtle issue that already exists
without my patches:

The co-routine may sleep with aio requests pending.  This makes the
job "paused" (!job->busy) while there is still activity!

The solution for AioContext switching that this patch relies on is:

+static void mirror_detach_aio_context(BlockJob *job)
+{
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+    while (s->in_flight > 0) {
+        aio_poll(blk_get_aio_context(job->blk), true);
+    }
+}

blockjob.c:block_job_detach_aio_context() first calls
block_job_pause(), then mirror_detach_aio_context() above, and finally
waits for !job->busy.

I chose this approach because it safely handles existing sleep or
block_job_yield() callers while there is still activity.

Now that I think more about it, we should solve the problem not just
for AioContext switching but also for pausing sleeping or
block_job_yield() coroutines.

I'll try to come up with a solution in v4.

Stefan

Re: [Qemu-devel] [PATCH v3 00/15] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

On Mon, Jun 13, 2016 at 6:05 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Daniel P. Berrange (1):
>   ui: fix regression in printing VNC host/port on startup
>
> Gerd Hoffmann (2):
>   vnc: drop unused depth arg for set_pixel_format
>   console: ignore ui_info updates which don't actually update something
>
> Olaf Hering (1):
>   gtk: fix vte version check
>
> Peter Maydell (6):
>   translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()
>   user-exec: Push resume-from-signal code out to handle_cpu_signal()
>   cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()
>   user-exec: Don't reextract sigmask from usercontext pointer
>   target-i386: Add comment about do_interrupt_user() next_eip argument
>   target-i386: Move user-mode exception actions out of user-exec.c
>
> Stefan Hajnoczi (4):
>   blockjob: move iostatus reset out of block_job_enter()
>   blockjob: add pause points
>   blockjob: add AioContext attach/detach callbacks
>   mirror: follow AioContext change gracefully
>
> Thomas Huth (1):
>   ui/console-gl: Add support for big endian display surfaces

Daniel, Olaf, Peter, and Thomas: Sorry for the noise.  I made a
mistake when running my patch sending script and it included a bunch
of patches that are already in qemu.git/master.

Re: [Qemu-devel] [PATCH v3 00/15] mirror: follow AioContext change gracefully

[Fam Zheng]

On Mon, 06/13 18:05, Stefan Hajnoczi wrote:
> v3:
>  * Push infrastructure down into blockjob.c so other jobs can reuse it [Stefan]
>  * Tested with drive_mirror + migration [Stefan]

Looks good to me apart from the question raised by Paolo. I'm looking forward
to v4. Thanks!

Fam