From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:33979)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1SASQY-0004nL-FG
	for qemu-devel@nongnu.org; Wed, 21 Mar 2012 16:46:39 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1SASQS-0001wE-6U
	for qemu-devel@nongnu.org; Wed, 21 Mar 2012 16:46:38 -0400
Received: from mx1.redhat.com ([209.132.183.28]:45847)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1SASQR-0001w4-Uy
	for qemu-devel@nongnu.org; Wed, 21 Mar 2012 16:46:32 -0400
Date: Wed, 21 Mar 2012 22:46:34 +0200
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20120321204634.GA26460@redhat.com>
References: <1332354545-17044-1-git-send-email-anthony.perard@citrix.com>
	<1332354545-17044-6-git-send-email-anthony.perard@citrix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1332354545-17044-6-git-send-email-anthony.perard@citrix.com>
Subject: Re: [Qemu-devel] [PATCH V9 5/8] Introduce Xen PCI Passthrough,
	qdevice (1/3)
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: Guy Zana <guy@neocleus.com>, Xen Devel <xen-devel@lists.xensource.com>, Allen Kay <allen.m.kay@intel.com>, QEMU-devel <qemu-devel@nongnu.org>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>

On Wed, Mar 21, 2012 at 06:29:02PM +0000, Anthony PERARD wrote:
> diff --git a/hw/xen_pci_passthrough.h b/hw/xen_pci_passthrough.h
> new file mode 100644
> index 0000000..2c5e83d
> --- /dev/null
> +++ b/hw/xen_pci_passthrough.h
> @@ -0,0 +1,250 @@
> +#ifndef QEMU_HW_XEN_PCI_PASSTHROUGH_H
> +#  define QEMU_HW_XEN_PCI_PASSTHROUGH_H

Way overboard. QEMU_HW_ is not needed.

> +
> +#include "qemu-common.h"
> +#include "xen_common.h"
> +#include "pci.h"
> +#include "host-pci-device.h"
> +
> +/* #define PT_LOGGING_ENABLED */
> +/* #define PT_DEBUG_PCI_CONFIG_ACCESS */

Don't keep dead code around esp not in headers.

> +
> +void pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3);
> +
> +#define PT_ERR(d, _f, _a...)  pt_log(d, "%s: Error: " _f, __func__, ##_a)
> +
> +#ifdef PT_LOGGING_ENABLED
> +#  define PT_LOG(d, _f, _a...)  pt_log(d, "%s: " _f, __func__, ##_a)
> +#  define PT_WARN(d, _f, _a...) pt_log(d, "%s: Warning: " _f, __func__, ##_a)
> +#else
> +#  define PT_LOG(d, _f, _a...)
> +#  define PT_WARN(d, _f, _a...)
> +#endif
> +
> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS
> +#  define PT_LOG_CONFIG(d, addr, val, len) \
> +    pt_log(d, "%s: address=0x%04x val=0x%08x len=%d\n", \
> +           __func__, addr, val, len)
> +#else
> +#  define PT_LOG_CONFIG(d, addr, val, len)
> +#endif

There's no reason not to prefix xen things with XEN_PT_
xen_pt_ etc.
And if you make names consistens with file names
naming files xen_pt_xxxx.x, people will know where to look for them.


-- 
MST