[Qemu-devel] [PULL 0/2] Net patches

[Qemu-devel] [PULL 0/2] Net patches

[Jason Wang]

The following changes since commit 7b1db0908d88f0c9cfac24e214ff72a860692e23:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180323' into staging (2018-03-25 13:51:33 +0100)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 7587855cd23755a7a6bd01b026611465f5584ecd:

  net/vde: print error on vde_open() failure (2018-03-26 14:52:43 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Greg Kurz (1):
      virtio_net: flush uncompleted TX on reset

Julia Suvorova via Qemu-devel (1):
      net/vde: print error on vde_open() failure

 hw/net/virtio-net.c | 11 +++++++++++
 include/net/net.h   |  1 +
 net/net.c           |  1 -
 net/vde.c           |  7 ++++---
 4 files changed, 16 insertions(+), 4 deletions(-)

[Qemu-devel] [PULL 1/2] virtio_net: flush uncompleted TX on reset

[Jason Wang]

From: Greg Kurz <groug@kaod.org>

If the backend could not transmit a packet right away for some reason,
the packet is queued for asynchronous sending. The corresponding vq
element is tracked in the async_tx.elem field of the VirtIONetQueue,
for later freeing when the transmission is complete.

If a reset happens before completion, virtio_net_tx_complete() will push
async_tx.elem back to the guest anyway, and we end up with the inuse flag
of the vq being equal to -1. The next call to virtqueue_pop() is then
likely to fail with "Virtqueue size exceeded".

This can be reproduced easily by starting a guest with an hubport backend
that is not connected to a functional network, eg,

 -device virtio-net-pci,netdev=hub0 -netdev hubport,id=hub0,hubid=0

and no other -netdev hubport,hubid=0 on the command line.

The appropriate fix is to ensure that such an asynchronous transmission
cannot survive a device reset. So for all queues, we first try to send
the packet again, and eventually we purge it if the backend still could
not deliver it.

CC: qemu-stable@nongnu.org
Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com>
Buglink: https://github.com/open-power-host-os/qemu/issues/37
Signed-off-by: Greg Kurz <groug@kaod.org>
Tested-by: R. Nageswara Sastry <nasastry@in.ibm.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 11 +++++++++++
 include/net/net.h   |  1 +
 net/net.c           |  1 -
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 67ad38c..90502fc 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -427,6 +427,7 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
 static void virtio_net_reset(VirtIODevice *vdev)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
+    int i;
 
     /* Reset back to compatibility mode */
     n->promisc = 1;
@@ -450,6 +451,16 @@ static void virtio_net_reset(VirtIODevice *vdev)
     memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
     qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
     memset(n->vlans, 0, MAX_VLAN >> 3);
+
+    /* Flush any async TX */
+    for (i = 0;  i < n->max_queues; i++) {
+        NetClientState *nc = qemu_get_subqueue(n->nic, i);
+
+        if (nc->peer) {
+            qemu_flush_or_purge_queued_packets(nc->peer, true);
+            assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
+        }
+    }
 }
 
 static void peer_test_vnet_hdr(VirtIONet *n)
diff --git a/include/net/net.h b/include/net/net.h
index a943e96..1f7341e 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -153,6 +153,7 @@ ssize_t qemu_send_packet_async(NetClientState *nc, const uint8_t *buf,
                                int size, NetPacketSent *sent_cb);
 void qemu_purge_queued_packets(NetClientState *nc);
 void qemu_flush_queued_packets(NetClientState *nc);
+void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge);
 void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
 bool qemu_has_ufo(NetClientState *nc);
 bool qemu_has_vnet_hdr(NetClientState *nc);
diff --git a/net/net.c b/net/net.c
index 5222e45..29f8398 100644
--- a/net/net.c
+++ b/net/net.c
@@ -595,7 +595,6 @@ void qemu_purge_queued_packets(NetClientState *nc)
     qemu_net_queue_purge(nc->peer->incoming_queue, nc);
 }
 
-static
 void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge)
 {
     nc->receive_disabled = 0;
-- 
2.7.4

[Qemu-devel] [PULL 2/2] net/vde: print error on vde_open() failure

[Jason Wang]

From: Julia Suvorova via Qemu-devel <qemu-devel@nongnu.org>

Despite the fact that now when the initialization of vde fails, qemu
does not end silently, no informative error is printed. The patch
generates an error and pushes it through the calling function.

Related bug: https://bugs.launchpad.net/qemu/+bug/676029

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/vde.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/vde.c b/net/vde.c
index e50e5d6..99189cc 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -30,6 +30,7 @@
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/main-loop.h"
+#include "qapi/error.h"
 
 typedef struct VDEState {
     NetClientState nc;
@@ -76,7 +77,7 @@ static NetClientInfo net_vde_info = {
 
 static int net_vde_init(NetClientState *peer, const char *model,
                         const char *name, const char *sock,
-                        int port, const char *group, int mode)
+                        int port, const char *group, int mode, Error **errp)
 {
     NetClientState *nc;
     VDEState *s;
@@ -92,6 +93,7 @@ static int net_vde_init(NetClientState *peer, const char *model,
 
     vde = vde_open(init_sock, (char *)"QEMU", &args);
     if (!vde){
+        error_setg_errno(errp, errno, "Could not open vde");
         return -1;
     }
 
@@ -112,7 +114,6 @@ static int net_vde_init(NetClientState *peer, const char *model,
 int net_init_vde(const Netdev *netdev, const char *name,
                  NetClientState *peer, Error **errp)
 {
-    /* FIXME error_setg(errp, ...) on failure */
     const NetdevVdeOptions *vde;
 
     assert(netdev->type == NET_CLIENT_DRIVER_VDE);
@@ -120,7 +121,7 @@ int net_init_vde(const Netdev *netdev, const char *name,
 
     /* missing optional values have been initialized to "all bits zero" */
     if (net_vde_init(peer, "vde", name, vde->sock, vde->port, vde->group,
-                     vde->has_mode ? vde->mode : 0700) == -1) {
+                     vde->has_mode ? vde->mode : 0700, errp) == -1) {
         return -1;
     }
 
-- 
2.7.4

Re: [Qemu-devel] [PULL 2/2] net/vde: print error on vde_open() failure

[Philippe Mathieu-Daudé]

Hi Jason,

On 03/26/2018 04:00 AM, Jason Wang wrote:
> From: Julia Suvorova via Qemu-devel <qemu-devel@nongnu.org>

This doesn't look right, shouldn't it be Julia Suvorova <jusual@mail.ru>?

> 
> Despite the fact that now when the initialization of vde fails, qemu
> does not end silently, no informative error is printed. The patch
> generates an error and pushes it through the calling function.
> 
> Related bug: https://bugs.launchpad.net/qemu/+bug/676029
> 
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  net/vde.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/net/vde.c b/net/vde.c
> index e50e5d6..99189cc 100644
> --- a/net/vde.c
> +++ b/net/vde.c
> @@ -30,6 +30,7 @@
>  #include "qemu-common.h"
>  #include "qemu/option.h"
>  #include "qemu/main-loop.h"
> +#include "qapi/error.h"
>  
>  typedef struct VDEState {
>      NetClientState nc;
> @@ -76,7 +77,7 @@ static NetClientInfo net_vde_info = {
>  
>  static int net_vde_init(NetClientState *peer, const char *model,
>                          const char *name, const char *sock,
> -                        int port, const char *group, int mode)
> +                        int port, const char *group, int mode, Error **errp)
>  {
>      NetClientState *nc;
>      VDEState *s;
> @@ -92,6 +93,7 @@ static int net_vde_init(NetClientState *peer, const char *model,
>  
>      vde = vde_open(init_sock, (char *)"QEMU", &args);
>      if (!vde){
> +        error_setg_errno(errp, errno, "Could not open vde");
>          return -1;
>      }
>  
> @@ -112,7 +114,6 @@ static int net_vde_init(NetClientState *peer, const char *model,
>  int net_init_vde(const Netdev *netdev, const char *name,
>                   NetClientState *peer, Error **errp)
>  {
> -    /* FIXME error_setg(errp, ...) on failure */
>      const NetdevVdeOptions *vde;
>  
>      assert(netdev->type == NET_CLIENT_DRIVER_VDE);
> @@ -120,7 +121,7 @@ int net_init_vde(const Netdev *netdev, const char *name,
>  
>      /* missing optional values have been initialized to "all bits zero" */
>      if (net_vde_init(peer, "vde", name, vde->sock, vde->port, vde->group,
> -                     vde->has_mode ? vde->mode : 0700) == -1) {
> +                     vde->has_mode ? vde->mode : 0700, errp) == -1) {
>          return -1;
>      }
>  
> 

Re: [Qemu-devel] [PULL 2/2] net/vde: print error on vde_open() failure

[Eric Blake]

On 03/26/2018 07:13 AM, Philippe Mathieu-Daudé wrote:
> Hi Jason,
> 
> On 03/26/2018 04:00 AM, Jason Wang wrote:
>> From: Julia Suvorova via Qemu-devel <qemu-devel@nongnu.org>
> 
> This doesn't look right, shouldn't it be Julia Suvorova <jusual@mail.ru>?

Can patchew be taught to flag mails like this?  (The rewrite occurs when 
a sender's mail service is so strict that mailman has to rewrite the 
From: address to get it to the list without the mail being dropped to 
certain recipients; the sender can work around the problem by manually 
including a From: line in the body of the message that overrides the 
overwritten From: line from the headers).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 0/2] Net patches

[no-reply]

Hi,

This series failed docker-quick@centos6 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1522047629-27658-1-git-send-email-jasowang@redhat.com
Subject: [Qemu-devel] [PULL 0/2] Net patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5550e5c60f net/vde: print error on vde_open() failure
bcddfa430e virtio_net: flush uncompleted TX on reset

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-_5w_20ll/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-_5w_20ll/src'
  GEN     /var/tmp/patchew-tester-tmp-_5w_20ll/src/docker-src.2018-03-26-08.40.27.22090/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-_5w_20ll/src/docker-src.2018-03-26-08.40.27.22090/qemu.tar.vroot'...
done.
Checking out files:  50% (3057/6057)   
Checking out files:  51% (3090/6057)   
Checking out files:  52% (3150/6057)   
Checking out files:  53% (3211/6057)   
Checking out files:  54% (3271/6057)   
Checking out files:  55% (3332/6057)   
Checking out files:  56% (3392/6057)   
Checking out files:  57% (3453/6057)   
Checking out files:  58% (3514/6057)   
Checking out files:  59% (3574/6057)   
Checking out files:  60% (3635/6057)   
Checking out files:  61% (3695/6057)   
Checking out files:  62% (3756/6057)   
Checking out files:  63% (3816/6057)   
Checking out files:  64% (3877/6057)   
Checking out files:  65% (3938/6057)   
Checking out files:  66% (3998/6057)   
Checking out files:  67% (4059/6057)   
Checking out files:  68% (4119/6057)   
Checking out files:  69% (4180/6057)   
Checking out files:  70% (4240/6057)   
Checking out files:  71% (4301/6057)   
Checking out files:  72% (4362/6057)   
Checking out files:  73% (4422/6057)   
Checking out files:  74% (4483/6057)   
Checking out files:  75% (4543/6057)   
Checking out files:  76% (4604/6057)   
Checking out files:  77% (4664/6057)   
Checking out files:  78% (4725/6057)   
Checking out files:  79% (4786/6057)   
Checking out files:  80% (4846/6057)   
Checking out files:  81% (4907/6057)   
Checking out files:  82% (4967/6057)   
Checking out files:  83% (5028/6057)   
Checking out files:  84% (5088/6057)   
Checking out files:  85% (5149/6057)   
Checking out files:  86% (5210/6057)   
Checking out files:  87% (5270/6057)   
Checking out files:  88% (5331/6057)   
Checking out files:  89% (5391/6057)   
Checking out files:  90% (5452/6057)   
Checking out files:  91% (5512/6057)   
Checking out files:  92% (5573/6057)   
Checking out files:  93% (5634/6057)   
Checking out files:  94% (5694/6057)   
Checking out files:  95% (5755/6057)   
Checking out files:  96% (5815/6057)   
Checking out files:  97% (5876/6057)   
Checking out files:  98% (5936/6057)   
Checking out files:  99% (5997/6057)   
Checking out files: 100% (6057/6057)   
Checking out files: 100% (6057/6057), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-_5w_20ll/src/docker-src.2018-03-26-08.40.27.22090/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-_5w_20ll/src/docker-src.2018-03-26-08.40.27.22090/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPY    RUNNER
    RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.6-2.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison     bzip2-devel     ccache     csnappy-devel     flex     g++     gcc     gettext     git     glib2-devel     libepoxy-devel     libfdt-devel     librdmacm-devel     lzo-devel     make     mesa-libEGL-devel     mesa-libgbm-devel     pixman-devel     SDL-devel     spice-glib-devel     spice-server-devel     tar     vte-devel     xen-devel     zlib-devel
HOSTNAME=dcffb93657b5
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=e2f9684e30f211e896f652540069c830', '-u', '0', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-_5w_20ll/src/docker-src.2018-03-26-08.40.27.22090:/var/tmp/qemu:z,ro', 'qemu:centos6', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 137
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-_5w_20ll/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-quick@centos6] Error 2

real	0m25.010s
user	0m3.863s
sys	0m3.531s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 0/2] Net patches

[Peter Maydell]

On 26 March 2018 at 08:00, Jason Wang <jasowang@redhat.com> wrote:
> The following changes since commit 7b1db0908d88f0c9cfac24e214ff72a860692e23:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180323' into staging (2018-03-25 13:51:33 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 7587855cd23755a7a6bd01b026611465f5584ecd:
>
>   net/vde: print error on vde_open() failure (2018-03-26 14:52:43 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
> Greg Kurz (1):
>       virtio_net: flush uncompleted TX on reset
>
> Julia Suvorova via Qemu-devel (1):
>       net/vde: print error on vde_open() failure

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 0/2] Net patches

[Eric Blake]

On 03/26/2018 08:14 AM, Peter Maydell wrote:
> On 26 March 2018 at 08:00, Jason Wang <jasowang@redhat.com> wrote:
>> The following changes since commit 7b1db0908d88f0c9cfac24e214ff72a860692e23:
>>
>>    Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180323' into staging (2018-03-25 13:51:33 +0100)
>>
>> are available in the git repository at:
>>
>>    https://github.com/jasowang/qemu.git tags/net-pull-request
>>
>> for you to fetch changes up to 7587855cd23755a7a6bd01b026611465f5584ecd:
>>
>>    net/vde: print error on vde_open() failure (2018-03-26 14:52:43 +0800)
>>
>> ----------------------------------------------------------------
>>
>> ----------------------------------------------------------------
>> Greg Kurz (1):
>>        virtio_net: flush uncompleted TX on reset
>>
>> Julia Suvorova via Qemu-devel (1):
>>        net/vde: print error on vde_open() failure
> 
> Applied, thanks.

We'll want a followup patch to .mailmap to make the git log attribution 
look nicer.  We currently have the following patches all attributed to 
the same email address; I have no idea if .mailmap can correctly sort 
between them:

$ git shortlog origin --author=qemu-devel | grep '^[^ ]'
Ed Swierk via Qemu-devel (2):
Ian McKellar via Qemu-devel (1):
Julia Suvorova via Qemu-devel (1):
Justin Terry (VM) via Qemu-devel (8):
Paul Donohue (2):

but it would be nice if we can improve our tooling to prevent future 
instances of the recurring problem.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 0/2] Net patches

[Peter Maydell]

On 26 March 2018 at 15:23, Eric Blake <eblake@redhat.com> wrote:
> We'll want a followup patch to .mailmap to make the git log attribution look
> nicer.  We currently have the following patches all attributed to the same
> email address; I have no idea if .mailmap can correctly sort between them:
>
> $ git shortlog origin --author=qemu-devel | grep '^[^ ]'
> Ed Swierk via Qemu-devel (2):
> Ian McKellar via Qemu-devel (1):
> Julia Suvorova via Qemu-devel (1):
> Justin Terry (VM) via Qemu-devel (8):
> Paul Donohue (2):
>
> but it would be nice if we can improve our tooling to prevent future
> instances of the recurring problem.

Yuck. (Maybe we should try whatever the other workaround for
this SPF vs mailing lists problem is?)

If you can suggest a patch to my apply-pullreq script I'm
happy to change it to reject these at pull application time.

https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/apply-pullreq

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/2] Net patches

[Eric Blake]

On 03/26/2018 11:12 AM, Peter Maydell wrote:

>> but it would be nice if we can improve our tooling to prevent future
>> instances of the recurring problem.
> 
> Yuck. (Maybe we should try whatever the other workaround for
> this SPF vs mailing lists problem is?)
> 
> If you can suggest a patch to my apply-pullreq script I'm
> happy to change it to reject these at pull application time.
> 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/apply-pullreq

My first quick attempt:

diff --git i/apply-pullreq w/apply-pullreq
index a5528e4..9ae4b8f 100755
--- i/apply-pullreq
+++ w/apply-pullreq
@@ -104,6 +104,12 @@ if git diff master..staging | grep -q 'Subproject 
commit'; then
      echo "WARNING: pull appears to include submodule update, please 
check it!"
  fi

+# Check whether any authors needs to be corrected after SPF rewrites
+if git shortlog --author=qemu-devel@nongnu.org master..staging | grep 
.; then
+    echo "ERROR: pull request includes commits attributed to list"
+    exit 1
+fi
+
  # This should exit with an error status if any of the sub-builds fails.
  parallel-buildtest


Hmm, on re-reading that, I wonder if shortlog will do the right thing 
when a .mailmap entry exists.  I'm trying to make sure we don't have to 
go lower-level with use of 'git log --format=%ae' (vs. --format=%aE 
and/or log --use-mailmap).

/me goes and experiments with:

diff --git i/.mailmap w/.mailmap
index cf689b9ec99..a90d7deebe6 100644
--- i/.mailmap
+++ w/.mailmap
@@ -10,6 +10,7 @@ Edgar E. Iglesias <edgar.iglesias@gmail.com> edgar_igl 
<edgar_igl@c046a42c-6fe2-
  Fabrice Bellard <fabrice@bellard.org> bellard 
<bellard@c046a42c-6fe2-441c-8c8c-71466251a162>
  James Hogan <jhogan@kernel.org> <james.hogan@imgtec.com>
  Jocelyn Mayer <l_indien@magic.fr> j_mayer 
<j_mayer@c046a42c-6fe2-441c-8c8c-71466251a162>
+Julia Suvorova <<jusual@mail.ru> Julia Suvorova via Qemu-devel 
<qemu-devel@nongnu.org>
  Paul Brook <paul@codesourcery.com> pbrook 
<pbrook@c046a42c-6fe2-441c-8c8c-71466251a162>
  Paul Burton <paul.burton@mips.com> <paul.burton@imgtec.com>
  Paul Burton <paul.burton@mips.com> <paul@archlinuxmips.org>

Yay - shortlog still lists Julia's commit even with the mailmap in place 
(but with a better spelling of her name), so I don't need to try 
anything fancier.  I'll post a separate patch for mailmap, then leave it 
up to you whether to incorporate my shortlog snippet above into your 
build script.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 0/2] Net patches

[Peter Maydell]

On 26 March 2018 at 17:54, Eric Blake <eblake@redhat.com> wrote:
> Hmm, on re-reading that, I wonder if shortlog will do the right thing when a
> .mailmap entry exists.  I'm trying to make sure we don't have to go
> lower-level with use of 'git log --format=%ae' (vs. --format=%aE and/or log
> --use-mailmap).
>
> /me goes and experiments with:
>
> diff --git i/.mailmap w/.mailmap
> index cf689b9ec99..a90d7deebe6 100644
> --- i/.mailmap
> +++ w/.mailmap
> @@ -10,6 +10,7 @@ Edgar E. Iglesias <edgar.iglesias@gmail.com> edgar_igl
> <edgar_igl@c046a42c-6fe2-
>  Fabrice Bellard <fabrice@bellard.org> bellard
> <bellard@c046a42c-6fe2-441c-8c8c-71466251a162>
>  James Hogan <jhogan@kernel.org> <james.hogan@imgtec.com>
>  Jocelyn Mayer <l_indien@magic.fr> j_mayer
> <j_mayer@c046a42c-6fe2-441c-8c8c-71466251a162>
> +Julia Suvorova <<jusual@mail.ru> Julia Suvorova via Qemu-devel
> <qemu-devel@nongnu.org>
>  Paul Brook <paul@codesourcery.com> pbrook
> <pbrook@c046a42c-6fe2-441c-8c8c-71466251a162>
>  Paul Burton <paul.burton@mips.com> <paul.burton@imgtec.com>
>  Paul Burton <paul.burton@mips.com> <paul@archlinuxmips.org>
>
> Yay - shortlog still lists Julia's commit even with the mailmap in place
> (but with a better spelling of her name), so I don't need to try anything
> fancier.  I'll post a separate patch for mailmap, then leave it up to you
> whether to incorporate my shortlog snippet above into your build script.

Your mailmap change above seems to have a stray extra '<' in it --
was that added by the mailing list server, or is it in the original?

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/2] Net patches

[Eric Blake]

On 03/26/2018 11:59 AM, Peter Maydell wrote:
> On 26 March 2018 at 17:54, Eric Blake <eblake@redhat.com> wrote:
>> Hmm, on re-reading that, I wonder if shortlog will do the right thing when a
>> .mailmap entry exists.  I'm trying to make sure we don't have to go
>> lower-level with use of 'git log --format=%ae' (vs. --format=%aE and/or log
>> --use-mailmap).
>>
>> /me goes and experiments with:
>>

>> +Julia Suvorova <<jusual@mail.ru> Julia Suvorova via Qemu-devel
>> <qemu-devel@nongnu.org>

> 
> Your mailmap change above seems to have a stray extra '<' in it --
> was that added by the mailing list server, or is it in the original?

D'oh, extra < in my playground.  Thankfully, removing it, and trying 
again, finds the same results: 'git shortlog 
--author=qemu-devel@nongnu.org' finds all Author: entries that were 
originally spelled with the list address, even if a .mailmap entry would 
rewrite it during 'git log --use-mailmap'.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 0/2] Net patches

[Eric Blake]

On 03/26/2018 09:23 AM, Eric Blake wrote:

>>> Julia Suvorova via Qemu-devel (1):
>>>        net/vde: print error on vde_open() failure
>>
>> Applied, thanks.
> 
> We'll want a followup patch to .mailmap to make the git log attribution 
> look nicer.  We currently have the following patches all attributed to 
> the same email address; I have no idea if .mailmap can correctly sort 
> between them:

Looks like it can, if we use the four-argument form.  Quoting 
'git-shortlog --help':

            Proper Name <proper@email.xx> Commit Name <commit@email.xx>

        which allows mailmap to replace both the name and the email of a 
commit
        matching both the specified commit name and email address.

> 
> $ git shortlog origin --author=qemu-devel | grep '^[^ ]'

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org