From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42547) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZguiE-0004SS-32 for qemu-devel@nongnu.org; Tue, 29 Sep 2015 09:12:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zgui8-0002BA-C9 for qemu-devel@nongnu.org; Tue, 29 Sep 2015 09:12:54 -0400 Received: from mx6-phx2.redhat.com ([209.132.183.39]:53242) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zgui8-0002B4-45 for qemu-devel@nongnu.org; Tue, 29 Sep 2015 09:12:48 -0400 Date: Tue, 29 Sep 2015 09:12:41 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <379496541.19670855.1443532361334.JavaMail.zimbra@redhat.com> In-Reply-To: <560A8B89.1070808@huawei.com> References: <1443094669-4144-1-git-send-email-marcandre.lureau@redhat.com> <1443094669-4144-18-git-send-email-marcandre.lureau@redhat.com> <560A8B89.1070808@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 17/47] ivshmem: improve debug messages List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Claudio Fontana Cc: drjones@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, marcandre lureau , cam@cs.ualberta.ca ----- Original Message ----- > On 24.09.2015 13:37, marcandre.lureau@redhat.com wrote: > > From: Marc-Andr=C3=A9 Lureau > >=20 > > Some misc improvements to ivshmem debug. > >=20 > > Signed-off-by: Marc-Andr=C3=A9 Lureau > > --- > > hw/misc/ivshmem.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > >=20 > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > > index c4c130d..71a71fc 100644 > > --- a/hw/misc/ivshmem.c > > +++ b/hw/misc/ivshmem.c > > @@ -208,10 +208,13 @@ static void ivshmem_io_write(void *opaque, hwaddr > > addr, > > if (vector < s->peers[dest].nb_eventfds) { > > IVSHMEM_DPRINTF("Notifying VM %d on vector %d\n", dest= , > > vector); > > event_notifier_set(&s->peers[dest].eventfds[vector]); > > + } else { > > + IVSHMEM_DPRINTF("Invalid destination vector %d on VM > > %d\n", > > + vector, dest); > > } > > break; > > default: > > - IVSHMEM_DPRINTF("Invalid VM Doorbell VM %d\n", dest); > > + IVSHMEM_DPRINTF("Unhandled write " TARGET_FMT_plx "\n", ad= dr); > > } > > } > > =20 > > @@ -263,9 +266,9 @@ static void ivshmem_receive(void *opaque, const uin= t8_t > > *buf, int size) > > { > > IVShmemState *s =3D opaque; > > =20 > > - ivshmem_IntrStatus_write(s, *buf); > > + IVSHMEM_DPRINTF("ivshmem_receive 0x%02x size: %d\n", *buf, size); > > =20 > > - IVSHMEM_DPRINTF("ivshmem_receive 0x%02x\n", *buf); > > + ivshmem_IntrStatus_write(s, *buf); > > } >=20 > I may understand how this went here, so these debug messages are clearly > specific about this particular function. > here you have "ivshmem_receive". Do you want to put () to help someone > getting these messages after enabling debug to understand that he should > look up this function? sure, notice it was there before without () > In the other messages though you (and the previous code) have been less > function-specific in the message. imho this kind of debug messages should just give enough context to underst= and and locate them. It's not important whether or not they have the functi= on name. > > =20 > > static int ivshmem_can_receive(void * opaque) > > @@ -592,6 +595,7 @@ static void ivshmem_use_msix(IVShmemState * s) > > PCIDevice *d =3D PCI_DEVICE(s); > > int i; > > =20 > > + IVSHMEM_DPRINTF("use msix, present: %d\n", msix_present(d)); > > if (!msix_present(d)) { > > return; > > } >=20 > hmm do you want to say "ivshmem_use_msix(): present: true/false?" > Or do you want to say something more generic about MSI-X in this case? >=20 > As is the comment you have: >=20 > 1) if it is specific to this function behavior, it does not help to look = up > the correct function It's actually easy to locate with a grep "use msix" in ivshmem code (the ma= cro prepend IVHSMEM:) > 2) if it is a more generic comment, it does not help to understand what i= s > going on >=20 > Can you explain more what the purpose of that debug statement is? It helps to trace what's going on, at least it helped me. But if it's super= flous, we can remove it (though it's really only there to help developers) >=20 > Ciao >=20 > Claudio >=20