Re: [Qemu-devel] [PATCH V9 5/8] Introduce Xen PCI Passthrough, qdevice (1/3)

["Michael S. Tsirkin" <mst@redhat.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