[Qemu-devel] Re: [PATCH] megasas: LSI Megaraid SAS emulation

[Stefan Hajnoczi]

On Mon, Nov 22, 2010 at 10:15 AM, Hannes Reinecke <address@hidden> wrote:
Not a full review but two small things I noticed:

> +#define megasas_frame_set_cmd_status(f,v)              \
> +    stb_phys((f) + offsetof(struct mfi_frame_header, cmd_status), v);
> +
> +#define megasas_frame_set_scsi_status(f,v)             \
> +    stb_phys((f) + offsetof(struct mfi_frame_header, scsi_status), v);
> +
> +#define megasas_frame_get_cmd(f)                       \
> +    ldub_phys((f) + offsetof(struct mfi_frame_header, frame_cmd))
> +
> +#define megasas_frame_get_context(f)                   \
> +    ldl_phys(frame_addr + offsetof(struct mfi_frame_header, context));

These macros include a semicolon.  They should be wrapped in:
do { ... } while (0)

> +static int megasas_scsi_uninit(PCIDevice *d)
> +{
> +    MPTState *s = DO_UPCAST(MPTState, dev, d);
> +
> +    cpu_unregister_io_memory(s->mmio_io_addr);

Missing cpu_unregister_io_memory(s->io_addr) and
cpu_unregister_io_memory(s->queue_addr)?

Stefan