Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On Tue, 29 May 2018 12:45:28 +0200 Paolo Bonzini wrote: > On 29/05/2018 11:53, Cornelia Huck wrote: > > On Mon, 28 May 2018 21:36:31 -0300 > > Philippe Mathieu-Daudé wrote: > > > >> On 05/28/2018 09:06 PM, Michael S. Tsirkin wrote: > >>> On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: > On Mon, 28 May 2018 20:26:59 -0300 > Philippe Mathieu-Daudé wrote: > > -ENOCOMMITLOG > >> > >> Oops sorry Alex, I meant to add some, but missed this while rebasing. > >> > Why? Tangible benefit. Looks like noise. Thanks, > > >>> I agree it should have a commit log, but .c files > >>> should be self-sufficient not rely on .h files > >>> pulling in headers for symbols the .h does not use > >>> itself. > >> > >> I meant: > >> > >> No declaration of "hw/vfio/vfio-common.h" directly requires to include > >> the "exec/address-spaces.h" header. To simplify dependencies and > >> ease following cleanup of "exec/address-spaces.h", directly include > >> it in the source file where the declaration are used. > >> > >>> This is better because it makes refactoring easier. > >>> > > Signed-off-by: Philippe Mathieu-Daudé > > --- > > include/hw/vfio/vfio-common.h | 1 - > > hw/vfio/ccw.c | 1 + > > hw/vfio/platform.c| 1 + > > 3 files changed, 2 insertions(+), 1 deletion(-) > > > > With the description added: > > > > Acked-by: Cornelia Huck > > > > Alex, is the description okay with you too? Yes, if someone wants to roll this on on commit, Acked-by: Alex Williamson
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On 29/05/2018 11:53, Cornelia Huck wrote: > On Mon, 28 May 2018 21:36:31 -0300 > Philippe Mathieu-Daudé wrote: > >> On 05/28/2018 09:06 PM, Michael S. Tsirkin wrote: >>> On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: On Mon, 28 May 2018 20:26:59 -0300 Philippe Mathieu-Daudé wrote: -ENOCOMMITLOG >> >> Oops sorry Alex, I meant to add some, but missed this while rebasing. >> Why? Tangible benefit. Looks like noise. Thanks, >>> I agree it should have a commit log, but .c files >>> should be self-sufficient not rely on .h files >>> pulling in headers for symbols the .h does not use >>> itself. >> >> I meant: >> >> No declaration of "hw/vfio/vfio-common.h" directly requires to include >> the "exec/address-spaces.h" header. To simplify dependencies and >> ease following cleanup of "exec/address-spaces.h", directly include >> it in the source file where the declaration are used. >> >>> This is better because it makes refactoring easier. >>> > Signed-off-by: Philippe Mathieu-Daudé > --- > include/hw/vfio/vfio-common.h | 1 - > hw/vfio/ccw.c | 1 + > hw/vfio/platform.c| 1 + > 3 files changed, 2 insertions(+), 1 deletion(-) > > With the description added: > > Acked-by: Cornelia Huck > Alex, is the description okay with you too? Thanks, Paolo
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On Mon, 28 May 2018 21:36:31 -0300 Philippe Mathieu-Daudé wrote: > On 05/28/2018 09:06 PM, Michael S. Tsirkin wrote: > > On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: > >> On Mon, 28 May 2018 20:26:59 -0300 > >> Philippe Mathieu-Daudé wrote: > >> > >> -ENOCOMMITLOG > > Oops sorry Alex, I meant to add some, but missed this while rebasing. > > >> Why? Tangible benefit. Looks like noise. Thanks, > >> > > I agree it should have a commit log, but .c files > > should be self-sufficient not rely on .h files > > pulling in headers for symbols the .h does not use > > itself. > > I meant: > > No declaration of "hw/vfio/vfio-common.h" directly requires to include > the "exec/address-spaces.h" header. To simplify dependencies and > ease following cleanup of "exec/address-spaces.h", directly include > it in the source file where the declaration are used. > > > This is better because it makes refactoring easier. > > > >>> Signed-off-by: Philippe Mathieu-Daudé > >>> --- > >>> include/hw/vfio/vfio-common.h | 1 - > >>> hw/vfio/ccw.c | 1 + > >>> hw/vfio/platform.c| 1 + > >>> 3 files changed, 2 insertions(+), 1 deletion(-) With the description added: Acked-by: Cornelia Huck
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On 05/28/2018 09:06 PM, Michael S. Tsirkin wrote: > On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: >> On Mon, 28 May 2018 20:26:59 -0300 >> Philippe Mathieu-Daudé wrote: >> >> -ENOCOMMITLOG Oops sorry Alex, I meant to add some, but missed this while rebasing. >> Why? Tangible benefit. Looks like noise. Thanks, >> > I agree it should have a commit log, but .c files > should be self-sufficient not rely on .h files > pulling in headers for symbols the .h does not use > itself. I meant: No declaration of "hw/vfio/vfio-common.h" directly requires to include the "exec/address-spaces.h" header. To simplify dependencies and ease following cleanup of "exec/address-spaces.h", directly include it in the source file where the declaration are used. > This is better because it makes refactoring easier. > >>> Signed-off-by: Philippe Mathieu-Daudé >>> --- >>> include/hw/vfio/vfio-common.h | 1 - >>> hw/vfio/ccw.c | 1 + >>> hw/vfio/platform.c| 1 + >>> 3 files changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >>> index d9360148e6..8264a65fa5 100644 >>> --- a/include/hw/vfio/vfio-common.h >>> +++ b/include/hw/vfio/vfio-common.h >>> @@ -22,7 +22,6 @@ >>> #define HW_VFIO_VFIO_COMMON_H >>> >>> #include "qemu-common.h" >>> -#include "exec/address-spaces.h" >>> #include "exec/memory.h" >>> #include "qemu/queue.h" >>> #include "qemu/notify.h" >>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c >>> index e67392c5f9..76e4e8c652 100644 >>> --- a/hw/vfio/ccw.c >>> +++ b/hw/vfio/ccw.c >>> @@ -22,6 +22,7 @@ >>> #include "hw/vfio/vfio-common.h" >>> #include "hw/s390x/s390-ccw.h" >>> #include "hw/s390x/ccw-device.h" >>> +#include "exec/address-spaces.h" >>> #include "qemu/error-report.h" >>> >>> #define TYPE_VFIO_CCW "vfio-ccw" >>> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c >>> index 5c921c27ba..57c4a0ee2b 100644 >>> --- a/hw/vfio/platform.c >>> +++ b/hw/vfio/platform.c >>> @@ -24,6 +24,7 @@ >>> #include "qemu/range.h" >>> #include "sysemu/sysemu.h" >>> #include "exec/memory.h" >>> +#include "exec/address-spaces.h" >>> #include "qemu/queue.h" >>> #include "hw/sysbus.h" >>> #include "trace.h"
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: > On Mon, 28 May 2018 20:26:59 -0300 > Philippe Mathieu-Daudé wrote: > > -ENOCOMMITLOG > > Why? Tangible benefit. Looks like noise. Thanks, > > Alex I agree it should have a commit log, but .c files should be self-sufficient not rely on .h files pulling in headers for symbols the .h does not use itself. This is better because it makes refactoring easier. > > Signed-off-by: Philippe Mathieu-Daudé > > --- > > include/hw/vfio/vfio-common.h | 1 - > > hw/vfio/ccw.c | 1 + > > hw/vfio/platform.c| 1 + > > 3 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > > index d9360148e6..8264a65fa5 100644 > > --- a/include/hw/vfio/vfio-common.h > > +++ b/include/hw/vfio/vfio-common.h > > @@ -22,7 +22,6 @@ > > #define HW_VFIO_VFIO_COMMON_H > > > > #include "qemu-common.h" > > -#include "exec/address-spaces.h" > > #include "exec/memory.h" > > #include "qemu/queue.h" > > #include "qemu/notify.h" > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > > index e67392c5f9..76e4e8c652 100644 > > --- a/hw/vfio/ccw.c > > +++ b/hw/vfio/ccw.c > > @@ -22,6 +22,7 @@ > > #include "hw/vfio/vfio-common.h" > > #include "hw/s390x/s390-ccw.h" > > #include "hw/s390x/ccw-device.h" > > +#include "exec/address-spaces.h" > > #include "qemu/error-report.h" > > > > #define TYPE_VFIO_CCW "vfio-ccw" > > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > > index 5c921c27ba..57c4a0ee2b 100644 > > --- a/hw/vfio/platform.c > > +++ b/hw/vfio/platform.c > > @@ -24,6 +24,7 @@ > > #include "qemu/range.h" > > #include "sysemu/sysemu.h" > > #include "exec/memory.h" > > +#include "exec/address-spaces.h" > > #include "qemu/queue.h" > > #include "hw/sysbus.h" > > #include "trace.h"
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On Mon, 28 May 2018 20:26:59 -0300 Philippe Mathieu-Daudé wrote: -ENOCOMMITLOG Why? Tangible benefit. Looks like noise. Thanks, Alex > Signed-off-by: Philippe Mathieu-Daudé > --- > include/hw/vfio/vfio-common.h | 1 - > hw/vfio/ccw.c | 1 + > hw/vfio/platform.c| 1 + > 3 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index d9360148e6..8264a65fa5 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -22,7 +22,6 @@ > #define HW_VFIO_VFIO_COMMON_H > > #include "qemu-common.h" > -#include "exec/address-spaces.h" > #include "exec/memory.h" > #include "qemu/queue.h" > #include "qemu/notify.h" > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index e67392c5f9..76e4e8c652 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -22,6 +22,7 @@ > #include "hw/vfio/vfio-common.h" > #include "hw/s390x/s390-ccw.h" > #include "hw/s390x/ccw-device.h" > +#include "exec/address-spaces.h" > #include "qemu/error-report.h" > > #define TYPE_VFIO_CCW "vfio-ccw" > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > index 5c921c27ba..57c4a0ee2b 100644 > --- a/hw/vfio/platform.c > +++ b/hw/vfio/platform.c > @@ -24,6 +24,7 @@ > #include "qemu/range.h" > #include "sysemu/sysemu.h" > #include "exec/memory.h" > +#include "exec/address-spaces.h" > #include "qemu/queue.h" > #include "hw/sysbus.h" > #include "trace.h"
[Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
Signed-off-by: Philippe Mathieu-Daudé --- include/hw/vfio/vfio-common.h | 1 - hw/vfio/ccw.c | 1 + hw/vfio/platform.c| 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index d9360148e6..8264a65fa5 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -22,7 +22,6 @@ #define HW_VFIO_VFIO_COMMON_H #include "qemu-common.h" -#include "exec/address-spaces.h" #include "exec/memory.h" #include "qemu/queue.h" #include "qemu/notify.h" diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index e67392c5f9..76e4e8c652 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -22,6 +22,7 @@ #include "hw/vfio/vfio-common.h" #include "hw/s390x/s390-ccw.h" #include "hw/s390x/ccw-device.h" +#include "exec/address-spaces.h" #include "qemu/error-report.h" #define TYPE_VFIO_CCW "vfio-ccw" diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index 5c921c27ba..57c4a0ee2b 100644 --- a/hw/vfio/platform.c +++ b/hw/vfio/platform.c @@ -24,6 +24,7 @@ #include "qemu/range.h" #include "sysemu/sysemu.h" #include "exec/memory.h" +#include "exec/address-spaces.h" #include "qemu/queue.h" #include "hw/sysbus.h" #include "trace.h" -- 2.17.0
