Code review comment for ~rafaeldtinoco/ubuntu/+source/qemu:lp1805256-bionic-refix

On Mon, Jul 27, 2020 at 7:46 PM Rafael David Tinoco
<email address hidden> wrote:
>
> > @rafaeldtinoco: Testing all looks good on arm64/armhf. Can you change the name of the patch in debian/changelog to match the new name of the patch? It is missing the -arm-only suffix. With that, I'm +1. But, if you haven't already, I'd recommend testing w/ unifdef just to prove that the !aarch64 code is exactly what it was before.
>
> I'll ask to @paelzer for a full set of regression tests from the package in the PPA so that will cover what you asked.

I think it's still a good idea to the unifdef test in addition to
regression tests. I went ahead and did this:

$ for file in aio-posix.c aio-win32.c async.c; do \
  unifdef -U__aarch64__ qemu-2.11+dfsg-1ubuntu7.30/util/${file} > \
    qemu-2.11+dfsg-1ubuntu7.30/util/${file}.undef__aarch64__;
  diff -u qemu-2.11+dfsg-1ubuntu7.29/util/${file} \
    qemu-2.11+dfsg-1ubuntu7.30/util/${file}.undef__aarch64__;
done
--- qemu-2.11+dfsg-1ubuntu7.29/util/async.c 2017-12-13 10:27:20.000000000 -0700
+++ qemu-2.11+dfsg-1ubuntu7.30/util/async.c.undef__aarch64__
2020-07-28 18:24:12.870325172 -0600
@@ -338,6 +338,7 @@
      * with atomic_or in aio_ctx_prepare or atomic_add in aio_poll.
      */
     smp_mb();
+
     if (ctx->notify_me) {
         event_notifier_set(&ctx->notifier);
         atomic_mb_set(&ctx->notified, true);

Looks good!

 -dann