Re: [Qemu-devel] [PATCH v3 3/4] hw/misc/pvpanic: Add the MMIO interface

[Peter Maydell]

On 18 October 2018 at 14:04, Philippe Mathieu-Daudé <address@hidden> wrote:
> Signed-off-by: Peng Hao <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> [PMD: Use TYPE_PVPANIC definition, split in 2 patches]
> ---
> Peng: I hope this is now more obvious how you could reuse the pvpanic device.
>
>  hw/misc/pvpanic.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 4f552e1533..b81c5fa633 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -2,10 +2,12 @@
>   * QEMU simulated pvpanic device.
>   *
>   * Copyright Fujitsu, Corp. 2013
> + * Copyright (c) 2018 ZTE Ltd.
>   *
>   * Authors:
>   *     Wen Congyang <address@hidden>
>   *     Hu Tao <address@hidden>
> + *     Peng Hao <address@hidden>
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.
> @@ -47,7 +49,10 @@ static void handle_event(int event)
>
>  typedef struct PVPanicState {
>      /*< private >*/
> -    ISADevice isadev;
> +    union {
> +        ISADevice isadev;
> +        SysBusDevice busdev;
> +    };

This field is the parent-type for the QOM object, so I don't
think it makes sense for it to be a union. Any one QOM object
should have a single distinct parent type. (There are other
examples of "some more or less similar device has ISA and
SysBus or PCI and SysBus variations" which should provide
some models to copy from.

thanks
-- PMM