Re: [Qemu-devel] [PATCH v3 1/4] hw/virtio: Factorize virtio-mmio headers

[Peter Maydell]

On Thu, 25 Jul 2019 at 10:58, Michael S. Tsirkin <address@hidden> wrote:
>
> On Thu, Jul 25, 2019 at 10:46:00AM +0100, Liam Merwick wrote:
> > On 02/07/2019 13:11, Sergio Lopez wrote:
> > > Put QOM and main struct definition in a separate header file, so it
> > > can be accesed from other components.
> >
> > typo: accesed -> accessed
> >
> > >
> > > This is needed for the microvm machine type implementation.
> > >
> > > Signed-off-by: Sergio Lopez <address@hidden>
> >
> > One nit below, either way
> >
> > Reviewed-by: Liam Merwick <address@hidden>
> >
> > > ---
> > >   hw/virtio/virtio-mmio.c | 35 +-----------------------
> > >   hw/virtio/virtio-mmio.h | 60 +++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 61 insertions(+), 34 deletions(-)
> > >   create mode 100644 hw/virtio/virtio-mmio.h
> > >
> > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> > > index 97b7f35496..87c7fe4d8d 100644
> > > --- a/hw/virtio/virtio-mmio.c
> > > +++ b/hw/virtio/virtio-mmio.c
> > > @@ -26,44 +26,11 @@
> > >   #include "qemu/host-utils.h"
> > >   #include "qemu/module.h"
> > >   #include "sysemu/kvm.h"
> > > -#include "hw/virtio/virtio-bus.h"
> > > +#include "virtio-mmio.h"
> >
> >
> > Virtually all the other includes of virtio-xxx.h files in hw/virtio use the
> > full path - e.g. "hw/virtio/virtio-mmio.h" - maybe do the same to be
> > consistent.
>
> That's for headers under include/.
> Local ones are ok with a short name.

Yes, but we should put this one into include/ as that fits with
our usual arrangement of where we put the headers for devices.

> I'm repeating myself, but still: if you insist on virtio mmio, please
> implement virtio 1 and use that with microvm. We can't keep carrying
> legacy interface into every new machine type.

Agreed (but we've had this discussion on another thread, as you say).

thanks
-- PMM