1. Patchew
  2. QEMU
  3. [Qemu-devel] [RFC PATCH v2 00/22] More patches to disable stuff
  4. View patch

[Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes

Juan Quintela posted 22 patches 7 years, 2 months ago
[Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Juan Quintela 7 years, 2 months ago 
They are all already included in virtio_pci.h.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/virtio/virtio-pci.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 3a01fe90f0..cb2ddd3f41 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -18,24 +18,11 @@
 #include "qemu/osdep.h"
 
 #include "standard-headers/linux/virtio_pci.h"
-#include "hw/virtio/virtio.h"
-#include "hw/virtio/virtio-blk.h"
-#include "hw/virtio/virtio-net.h"
-#include "hw/virtio/virtio-serial.h"
-#include "hw/virtio/virtio-scsi.h"
-#include "hw/virtio/virtio-balloon.h"
-#include "hw/virtio/virtio-input.h"
-#include "hw/pci/pci.h"
+#include "virtio-pci.h"
 #include "qapi/error.h"
-#include "qemu/error-report.h"
-#include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
-#include "hw/loader.h"
 #include "sysemu/kvm.h"
-#include "virtio-pci.h"
 #include "qemu/range.h"
-#include "hw/virtio/virtio-bus.h"
-#include "qapi/visitor.h"
 
 #define VIRTIO_PCI_REGION_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_present(dev))
 
-- 
2.17.1
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Thomas Huth 7 years, 2 months ago 
On 08/08/2018 01:48 PM, Juan Quintela wrote:
> They are all already included in virtio_pci.h.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/virtio/virtio-pci.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 3a01fe90f0..cb2ddd3f41 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -18,24 +18,11 @@
>  #include "qemu/osdep.h"
>  
>  #include "standard-headers/linux/virtio_pci.h"
> -#include "hw/virtio/virtio.h"
> -#include "hw/virtio/virtio-blk.h"
> -#include "hw/virtio/virtio-net.h"
> -#include "hw/virtio/virtio-serial.h"
> -#include "hw/virtio/virtio-scsi.h"
> -#include "hw/virtio/virtio-balloon.h"
> -#include "hw/virtio/virtio-input.h"
> -#include "hw/pci/pci.h"
> +#include "virtio-pci.h"
>  #include "qapi/error.h"
> -#include "qemu/error-report.h"
> -#include "hw/pci/msi.h"
>  #include "hw/pci/msix.h"
> -#include "hw/loader.h"
>  #include "sysemu/kvm.h"
> -#include "virtio-pci.h"
>  #include "qemu/range.h"
> -#include "hw/virtio/virtio-bus.h"
> -#include "qapi/visitor.h"
>  
>  #define VIRTIO_PCI_REGION_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_present(dev))

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Dr. David Alan Gilbert 7 years, 2 months ago 
* Juan Quintela (quintela@redhat.com) wrote:
> They are all already included in virtio_pci.h.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>


cc'ing in mst for virtio goodness.


> ---
>  hw/virtio/virtio-pci.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 3a01fe90f0..cb2ddd3f41 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -18,24 +18,11 @@
>  #include "qemu/osdep.h"
>  
>  #include "standard-headers/linux/virtio_pci.h"
> -#include "hw/virtio/virtio.h"
> -#include "hw/virtio/virtio-blk.h"
> -#include "hw/virtio/virtio-net.h"
> -#include "hw/virtio/virtio-serial.h"
> -#include "hw/virtio/virtio-scsi.h"
> -#include "hw/virtio/virtio-balloon.h"
> -#include "hw/virtio/virtio-input.h"
> -#include "hw/pci/pci.h"
> +#include "virtio-pci.h"
>  #include "qapi/error.h"
> -#include "qemu/error-report.h"
> -#include "hw/pci/msi.h"
>  #include "hw/pci/msix.h"
> -#include "hw/loader.h"
>  #include "sysemu/kvm.h"
> -#include "virtio-pci.h"
>  #include "qemu/range.h"
> -#include "hw/virtio/virtio-bus.h"
> -#include "qapi/visitor.h"
>  
>  #define VIRTIO_PCI_REGION_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_present(dev))
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Michael S. Tsirkin 7 years, 2 months ago 
On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
> > They are all already included in virtio_pci.h.

All I see in virtio_pci.h is:

#include "standard-headers/linux/types.h"

Weird.

BTW what's the point of this patch?  Generally it's best not to depend
on headers including each other, it makes refactoring harder.


> > 
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> 
> cc'ing in mst for virtio goodness.
> 
> 
> > ---
> >  hw/virtio/virtio-pci.c | 15 +--------------
> >  1 file changed, 1 insertion(+), 14 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 3a01fe90f0..cb2ddd3f41 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -18,24 +18,11 @@
> >  #include "qemu/osdep.h"
> >  
> >  #include "standard-headers/linux/virtio_pci.h"
> > -#include "hw/virtio/virtio.h"
> > -#include "hw/virtio/virtio-blk.h"
> > -#include "hw/virtio/virtio-net.h"
> > -#include "hw/virtio/virtio-serial.h"
> > -#include "hw/virtio/virtio-scsi.h"
> > -#include "hw/virtio/virtio-balloon.h"
> > -#include "hw/virtio/virtio-input.h"
> > -#include "hw/pci/pci.h"
> > +#include "virtio-pci.h"
> >  #include "qapi/error.h"
> > -#include "qemu/error-report.h"
> > -#include "hw/pci/msi.h"
> >  #include "hw/pci/msix.h"
> > -#include "hw/loader.h"
> >  #include "sysemu/kvm.h"
> > -#include "virtio-pci.h"
> >  #include "qemu/range.h"
> > -#include "hw/virtio/virtio-bus.h"
> > -#include "qapi/visitor.h"
> >  
> >  #define VIRTIO_PCI_REGION_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_present(dev))
> >  
> > -- 
> > 2.17.1
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Juan Quintela 7 years, 2 months ago 
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote:
>> * Juan Quintela (quintela@redhat.com) wrote:
>> > They are all already included in virtio_pci.h.
>
> All I see in virtio_pci.h is:
>
> #include "standard-headers/linux/types.h"
>
> Weird.
>
> BTW what's the point of this patch?  Generally it's best not to depend
> on headers including each other, it makes refactoring harder.

If you see the following patches, we remove blk, net, serial, scsi,
balloon and input bits from that file, so I was removing includes patch
by patch.

And at the end, I found that we only need that ones.

"virtio-pci.h" does too many things here, I could have split it also,
because the mayority of the bits are only used now inside their own
virtio-foo-pci.c.  But then, there are things that share bits,
virtio-bus-pci is used for lots of stuff, virtio-input-pci bits are used
in virtio-input-host-pci.c, etc,   So I decided to only do the direct
split.

And about including directly all the files that you use, and including
only the files that are extrictly needed, the normal argument is that
the less includes, the faster compiler times.

Later, Juan.
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Michael S. Tsirkin 7 years, 2 months ago 
On Fri, Aug 10, 2018 at 09:34:32AM +0200, Juan Quintela wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote:
> >> * Juan Quintela (quintela@redhat.com) wrote:
> >> > They are all already included in virtio_pci.h.
> >
> > All I see in virtio_pci.h is:
> >
> > #include "standard-headers/linux/types.h"
> >
> > Weird.
> >
> > BTW what's the point of this patch?  Generally it's best not to depend
> > on headers including each other, it makes refactoring harder.
> 
> If you see the following patches, we remove blk, net, serial, scsi,
> balloon and input bits from that file, so I was removing includes patch
> by patch.
> 
> And at the end, I found that we only need that ones.
> 
> "virtio-pci.h" does too many things here, I could have split it also,
> because the mayority of the bits are only used now inside their own
> virtio-foo-pci.c.  But then, there are things that share bits,
> virtio-bus-pci is used for lots of stuff, virtio-input-pci bits are used
> in virtio-input-host-pci.c, etc,   So I decided to only do the direct
> split.
> 
> And about including directly all the files that you use, and including
> only the files that are extrictly needed, the normal argument is that
> the less includes, the faster compiler times.
> 
> Later, Juan.

That's reverse of the direction we have been going.
Pls move includes when you split up the files.

-- 
MST
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Juan Quintela 7 years, 2 months ago 
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Aug 10, 2018 at 09:34:32AM +0200, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> > On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote:
>> >> * Juan Quintela (quintela@redhat.com) wrote:
>> >> > They are all already included in virtio_pci.h.
>> >
>> > All I see in virtio_pci.h is:
>> >
>> > #include "standard-headers/linux/types.h"
>> >
>> > Weird.
>> >
>> > BTW what's the point of this patch?  Generally it's best not to depend
>> > on headers including each other, it makes refactoring harder.
>> 
>> If you see the following patches, we remove blk, net, serial, scsi,
>> balloon and input bits from that file, so I was removing includes patch
>> by patch.
>> 
>> And at the end, I found that we only need that ones.
>> 
>> "virtio-pci.h" does too many things here, I could have split it also,
>> because the mayority of the bits are only used now inside their own
>> virtio-foo-pci.c.  But then, there are things that share bits,
>> virtio-bus-pci is used for lots of stuff, virtio-input-pci bits are used
>> in virtio-input-host-pci.c, etc,   So I decided to only do the direct
>> split.
>> 
>> And about including directly all the files that you use, and including
>> only the files that are extrictly needed, the normal argument is that
>> the less includes, the faster compiler times.
>> 
>> Later, Juan.
>
> That's reverse of the direction we have been going.
> Pls move includes when you split up the files.

Hi

You want this bit of virtio-pci.h into vhost-vsock-pci.h?

#ifdef CONFIG_VHOST_VSOCK
/*
 * vhost-vsock-pci: This extends VirtioPCIProxy.
 */
#define TYPE_VHOST_VSOCK_PCI "vhost-vsock-pci"
#define VHOST_VSOCK_PCI(obj) \
        OBJECT_CHECK(VHostVSockPCI, (obj), TYPE_VHOST_VSOCK_PCI)

struct VHostVSockPCI {
    VirtIOPCIProxy parent_obj;
    VHostVSock vdev;
};
#endif

Except for shared things (input, bus, ...) the rest are only used on
the file that uses them.

Later, Juan.
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Michael S. Tsirkin 7 years, 2 months ago 
On Fri, Aug 10, 2018 at 12:01:44PM +0200, Juan Quintela wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Fri, Aug 10, 2018 at 09:34:32AM +0200, Juan Quintela wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> > On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote:
> >> >> * Juan Quintela (quintela@redhat.com) wrote:
> >> >> > They are all already included in virtio_pci.h.
> >> >
> >> > All I see in virtio_pci.h is:
> >> >
> >> > #include "standard-headers/linux/types.h"
> >> >
> >> > Weird.
> >> >
> >> > BTW what's the point of this patch?  Generally it's best not to depend
> >> > on headers including each other, it makes refactoring harder.
> >> 
> >> If you see the following patches, we remove blk, net, serial, scsi,
> >> balloon and input bits from that file, so I was removing includes patch
> >> by patch.
> >> 
> >> And at the end, I found that we only need that ones.
> >> 
> >> "virtio-pci.h" does too many things here, I could have split it also,
> >> because the mayority of the bits are only used now inside their own
> >> virtio-foo-pci.c.  But then, there are things that share bits,
> >> virtio-bus-pci is used for lots of stuff, virtio-input-pci bits are used
> >> in virtio-input-host-pci.c, etc,   So I decided to only do the direct
> >> split.
> >> 
> >> And about including directly all the files that you use, and including
> >> only the files that are extrictly needed, the normal argument is that
> >> the less includes, the faster compiler times.
> >> 
> >> Later, Juan.
> >
> > That's reverse of the direction we have been going.
> > Pls move includes when you split up the files.
> 
> Hi
> 
> You want this bit of virtio-pci.h into vhost-vsock-pci.h?

In fact can we move it into c file?

> #ifdef CONFIG_VHOST_VSOCK
> /*
>  * vhost-vsock-pci: This extends VirtioPCIProxy.
>  */
> #define TYPE_VHOST_VSOCK_PCI "vhost-vsock-pci"
> #define VHOST_VSOCK_PCI(obj) \
>         OBJECT_CHECK(VHostVSockPCI, (obj), TYPE_VHOST_VSOCK_PCI)
> 
> struct VHostVSockPCI {
>     VirtIOPCIProxy parent_obj;
>     VHostVSock vdev;
> };
> #endif
> 
> Except for shared things (input, bus, ...) the rest are only used on
> the file that uses them.
> 
> Later, Juan.
>
Re: [Qemu-devel] [PATCH v2 09/22] virtio: Remove unneeded includes
Posted by Juan Quintela 7 years, 2 months ago 
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Aug 10, 2018 at 12:01:44PM +0200, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> > On Fri, Aug 10, 2018 at 09:34:32AM +0200, Juan Quintela wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> > On Thu, Aug 09, 2018 at 08:03:03PM +0100, Dr. David Alan Gilbert wrote:
>> >> >> * Juan Quintela (quintela@redhat.com) wrote:
>> >> >> > They are all already included in virtio_pci.h.
>> >> >
>> >> > All I see in virtio_pci.h is:
>> >> >
>> >> > #include "standard-headers/linux/types.h"
>> >> >
>> >> > Weird.
>> >> >
>> >> > BTW what's the point of this patch?  Generally it's best not to depend
>> >> > on headers including each other, it makes refactoring harder.
>> >> 
>> >> If you see the following patches, we remove blk, net, serial, scsi,
>> >> balloon and input bits from that file, so I was removing includes patch
>> >> by patch.
>> >> 
>> >> And at the end, I found that we only need that ones.
>> >> 
>> >> "virtio-pci.h" does too many things here, I could have split it also,
>> >> because the mayority of the bits are only used now inside their own
>> >> virtio-foo-pci.c.  But then, there are things that share bits,
>> >> virtio-bus-pci is used for lots of stuff, virtio-input-pci bits are used
>> >> in virtio-input-host-pci.c, etc,   So I decided to only do the direct
>> >> split.
>> >> 
>> >> And about including directly all the files that you use, and including
>> >> only the files that are extrictly needed, the normal argument is that
>> >> the less includes, the faster compiler times.
>> >> 
>> >> Later, Juan.
>> >
>> > That's reverse of the direction we have been going.
>> > Pls move includes when you split up the files.
>> 
>> Hi
>> 
>> You want this bit of virtio-pci.h into vhost-vsock-pci.h?
>
> In fact can we move it into c file?

Miss-type.  I mean the .c file.

ok, will do for the next round.

Thanks for the comments.

>> #ifdef CONFIG_VHOST_VSOCK
>> /*
>>  * vhost-vsock-pci: This extends VirtioPCIProxy.
>>  */
>> #define TYPE_VHOST_VSOCK_PCI "vhost-vsock-pci"
>> #define VHOST_VSOCK_PCI(obj) \
>>         OBJECT_CHECK(VHostVSockPCI, (obj), TYPE_VHOST_VSOCK_PCI)
>> 
>> struct VHostVSockPCI {
>>     VirtIOPCIProxy parent_obj;
>>     VHostVSock vdev;
>> };
>> #endif
>> 
>> Except for shared things (input, bus, ...) the rest are only used on
>> the file that uses them.
>> 
>> Later, Juan.
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.