Patchwork [v3] virtio-9p: Use relative includes for files in hw

login
register
mail settings
Submitter Stefan Weil
Date April 28, 2011, 8:02 p.m.
Message ID <1304020934-26995-1-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/93287/
State Accepted
Headers show

Comments

Stefan Weil - April 28, 2011, 8:02 p.m.
Commit 353ac78d495ef976242abd868f68d78420861c2c moved the files
without fixing the include paths. It used a modified CFLAGS
to add hw to the include search path, but this breaks builds
where the user wants to set special CFLAGS. Long include paths
also increase compilation time.

Therefore this patch removes the special CFLAGS for virtio
and fixes the include statements by using relative include paths.

v2: Remove special CFLAGS.
v3: Update needed for latest QEMU.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 Makefile.objs                  |    2 --
 Makefile.target                |    2 --
 hw/9pfs/virtio-9p-debug.c      |    5 +++--
 hw/9pfs/virtio-9p-local.c      |    3 ++-
 hw/9pfs/virtio-9p-posix-acl.c  |    2 +-
 hw/9pfs/virtio-9p-xattr-user.c |    2 +-
 hw/9pfs/virtio-9p-xattr.c      |    2 +-
 hw/9pfs/virtio-9p.c            |    4 ++--
 8 files changed, 10 insertions(+), 12 deletions(-)
Anthony Liguori - April 28, 2011, 8:49 p.m.
On 04/28/2011 03:02 PM, Stefan Weil wrote:
> Commit 353ac78d495ef976242abd868f68d78420861c2c moved the files
> without fixing the include paths. It used a modified CFLAGS
> to add hw to the include search path, but this breaks builds
> where the user wants to set special CFLAGS. Long include paths
> also increase compilation time.
>
> Therefore this patch removes the special CFLAGS for virtio
> and fixes the include statements by using relative include paths.
>
> v2: Remove special CFLAGS.
> v3: Update needed for latest QEMU.
>
> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
> ---
>   Makefile.objs                  |    2 --
>   Makefile.target                |    2 --
>   hw/9pfs/virtio-9p-debug.c      |    5 +++--
>   hw/9pfs/virtio-9p-local.c      |    3 ++-
>   hw/9pfs/virtio-9p-posix-acl.c  |    2 +-
>   hw/9pfs/virtio-9p-xattr-user.c |    2 +-
>   hw/9pfs/virtio-9p-xattr.c      |    2 +-
>   hw/9pfs/virtio-9p.c            |    4 ++--
>   8 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 9d8851e..df8cc9c 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -290,8 +290,6 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
>   9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
>
>   hw-obj-$(CONFIG_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
> -$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/

Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?

Regards,

Anthony Liguori

> -
>
>   ######################################################################
>   # libdis
> diff --git a/Makefile.target b/Makefile.target
> index 2501c63..76bbdc7 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -401,8 +401,6 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
>   qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
>   	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h<  $<  >  $@,"  GEN   $(TARGET_DIR)$@")
>
> -9pfs/virtio-9p.o: CFLAGS +=  -I$(SRC_PATH)/hw/
> -
>   clean:
>   	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
>   	rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
> diff --git a/hw/9pfs/virtio-9p-debug.c b/hw/9pfs/virtio-9p-debug.c
> index 6b18842..4636ad5 100644
> --- a/hw/9pfs/virtio-9p-debug.c
> +++ b/hw/9pfs/virtio-9p-debug.c
> @@ -10,8 +10,9 @@
>    * the COPYING file in the top-level directory.
>    *
>    */
> -#include "virtio.h"
> -#include "pc.h"
> +
> +#include "hw/virtio.h"
> +#include "hw/pc.h"
>   #include "virtio-9p.h"
>   #include "virtio-9p-debug.h"
>
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 0a015de..3effc39 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -10,7 +10,8 @@
>    * the COPYING file in the top-level directory.
>    *
>    */
> -#include "virtio.h"
> +
> +#include "hw/virtio.h"
>   #include "virtio-9p.h"
>   #include "virtio-9p-xattr.h"
>   #include<arpa/inet.h>
> diff --git a/hw/9pfs/virtio-9p-posix-acl.c b/hw/9pfs/virtio-9p-posix-acl.c
> index 575abe8..c0ae9d6 100644
> --- a/hw/9pfs/virtio-9p-posix-acl.c
> +++ b/hw/9pfs/virtio-9p-posix-acl.c
> @@ -13,7 +13,7 @@
>
>   #include<sys/types.h>
>   #include<attr/xattr.h>
> -#include "virtio.h"
> +#include "hw/virtio.h"
>   #include "virtio-9p.h"
>   #include "fsdev/file-op-9p.h"
>   #include "virtio-9p-xattr.h"
> diff --git a/hw/9pfs/virtio-9p-xattr-user.c b/hw/9pfs/virtio-9p-xattr-user.c
> index bba13ce..c56039c 100644
> --- a/hw/9pfs/virtio-9p-xattr-user.c
> +++ b/hw/9pfs/virtio-9p-xattr-user.c
> @@ -12,7 +12,7 @@
>    */
>
>   #include<sys/types.h>
> -#include "virtio.h"
> +#include "hw/virtio.h"
>   #include "virtio-9p.h"
>   #include "fsdev/file-op-9p.h"
>   #include "virtio-9p-xattr.h"
> diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c
> index 03c3d3f..f08ce6e 100644
> --- a/hw/9pfs/virtio-9p-xattr.c
> +++ b/hw/9pfs/virtio-9p-xattr.c
> @@ -11,7 +11,7 @@
>    *
>    */
>
> -#include "virtio.h"
> +#include "hw/virtio.h"
>   #include "virtio-9p.h"
>   #include "fsdev/file-op-9p.h"
>   #include "virtio-9p-xattr.h"
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index b5fc52b..ac5a1d0 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -11,8 +11,8 @@
>    *
>    */
>
> -#include "virtio.h"
> -#include "pc.h"
> +#include "hw/virtio.h"
> +#include "hw/pc.h"
>   #include "qemu_socket.h"
>   #include "virtio-9p.h"
>   #include "fsdev/qemu-fsdev.h"
Stefan Weil - April 28, 2011, 9:44 p.m.
Am 28.04.2011 22:49, schrieb Anthony Liguori:
> On 04/28/2011 03:02 PM, Stefan Weil wrote:
>> Commit 353ac78d495ef976242abd868f68d78420861c2c moved the files
>> without fixing the include paths. It used a modified CFLAGS
>> to add hw to the include search path, but this breaks builds
>> where the user wants to set special CFLAGS. Long include paths
>> also increase compilation time.
>>
>> Therefore this patch removes the special CFLAGS for virtio
>> and fixes the include statements by using relative include paths.
>>
>> v2: Remove special CFLAGS.
>> v3: Update needed for latest QEMU.
>>
>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>> ---
>>   Makefile.objs                  |    2 --
>>   Makefile.target                |    2 --
>>   hw/9pfs/virtio-9p-debug.c      |    5 +++--
>>   hw/9pfs/virtio-9p-local.c      |    3 ++-
>>   hw/9pfs/virtio-9p-posix-acl.c  |    2 +-
>>   hw/9pfs/virtio-9p-xattr-user.c |    2 +-
>>   hw/9pfs/virtio-9p-xattr.c      |    2 +-
>>   hw/9pfs/virtio-9p.c            |    4 ++--
>>   8 files changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 9d8851e..df8cc9c 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -290,8 +290,6 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
>>   9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o 
>> virtio-9p-posix-acl.o
>>
>>   hw-obj-$(CONFIG_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
>> -$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
>
> Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?
>
> Regards,
>
> Anthony Liguori

That would be the second best solution (in my opinion).
Its only advantage would be that it minimises the size of the patch
(which is not a good argument if the result has disadvantages).

Do you think that the lines which I removed in Makefile.* look pretty
and maintainable? I don't think so. The new include statements are
much cleaner. They are also in line with other QEMU code - try

                         git grep '"hw/'

to see it yourself.

Regards,

Stefan Weil
Peter Maydell - April 29, 2011, 12:46 a.m.
On 28 April 2011 21:49, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 04/28/2011 03:02 PM, Stefan Weil wrote:
>> -$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
>
> Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?

There aren't any other source files in QEMU which have custom
include paths -- why should 9pfs be a special case?

-- PMM
Stefan Weil - May 27, 2011, 6:27 p.m.
Am 29.04.2011 02:46, schrieb Peter Maydell:
> On 28 April 2011 21:49, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>    
>> On 04/28/2011 03:02 PM, Stefan Weil wrote:
>>      
>>> -$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
>>>        
>> Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?
>>      
> There aren't any other source files in QEMU which have custom
> include paths -- why should 9pfs be a special case?
>
> -- PMM
>    

Was this an acked-by?
The patch is still uncommitted. Are there any objections?

Stefan W.
Aneesh Kumar K.V - May 28, 2011, 4:51 p.m.
On Fri, 27 May 2011 20:27:11 +0200, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 29.04.2011 02:46, schrieb Peter Maydell:
> > On 28 April 2011 21:49, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >    
> >> On 04/28/2011 03:02 PM, Stefan Weil wrote:
> >>      
> >>> -$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
> >>>        
> >> Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?
> >>      
> > There aren't any other source files in QEMU which have custom
> > include paths -- why should 9pfs be a special case?
> >
> > -- PMM
> >    
> 
> Was this an acked-by?
> The patch is still uncommitted. Are there any objections?

I have pulled this patch into the tree. I guess the next pull request
from JV should include that.

-aneesh
jvrao - May 28, 2011, 7:12 p.m.
On 05/28/2011 09:51 AM, Aneesh Kumar K.V wrote:
> On Fri, 27 May 2011 20:27:11 +0200, Stefan Weil<weil@mail.berlios.de>  wrote:
>> Am 29.04.2011 02:46, schrieb Peter Maydell:
>>> On 28 April 2011 21:49, Anthony Liguori<anthony@codemonkey.ws>   wrote:
>>>
>>>> On 04/28/2011 03:02 PM, Stefan Weil wrote:
>>>>
>>>>> -$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
>>>>>
>>>> Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?
>>>>
>>> There aren't any other source files in QEMU which have custom
>>> include paths -- why should 9pfs be a special case?
>>>
>>> -- PMM
>>>
>> Was this an acked-by?
>> The patch is still uncommitted. Are there any objections?
> I have pulled this patch into the tree. I guess the next pull request
> from JV should include that.
Yes, next pull will include this. Thanks.

- JV

> -aneesh

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 9d8851e..df8cc9c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -290,8 +290,6 @@  hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
 
 hw-obj-$(CONFIG_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
-$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
-
 
 ######################################################################
 # libdis
diff --git a/Makefile.target b/Makefile.target
index 2501c63..76bbdc7 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -401,8 +401,6 @@  hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
 qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
 
-9pfs/virtio-9p.o: CFLAGS +=  -I$(SRC_PATH)/hw/
-
 clean:
 	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
 	rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
diff --git a/hw/9pfs/virtio-9p-debug.c b/hw/9pfs/virtio-9p-debug.c
index 6b18842..4636ad5 100644
--- a/hw/9pfs/virtio-9p-debug.c
+++ b/hw/9pfs/virtio-9p-debug.c
@@ -10,8 +10,9 @@ 
  * the COPYING file in the top-level directory.
  *
  */
-#include "virtio.h"
-#include "pc.h"
+
+#include "hw/virtio.h"
+#include "hw/pc.h"
 #include "virtio-9p.h"
 #include "virtio-9p-debug.h"
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 0a015de..3effc39 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -10,7 +10,8 @@ 
  * the COPYING file in the top-level directory.
  *
  */
-#include "virtio.h"
+
+#include "hw/virtio.h"
 #include "virtio-9p.h"
 #include "virtio-9p-xattr.h"
 #include <arpa/inet.h>
diff --git a/hw/9pfs/virtio-9p-posix-acl.c b/hw/9pfs/virtio-9p-posix-acl.c
index 575abe8..c0ae9d6 100644
--- a/hw/9pfs/virtio-9p-posix-acl.c
+++ b/hw/9pfs/virtio-9p-posix-acl.c
@@ -13,7 +13,7 @@ 
 
 #include <sys/types.h>
 #include <attr/xattr.h>
-#include "virtio.h"
+#include "hw/virtio.h"
 #include "virtio-9p.h"
 #include "fsdev/file-op-9p.h"
 #include "virtio-9p-xattr.h"
diff --git a/hw/9pfs/virtio-9p-xattr-user.c b/hw/9pfs/virtio-9p-xattr-user.c
index bba13ce..c56039c 100644
--- a/hw/9pfs/virtio-9p-xattr-user.c
+++ b/hw/9pfs/virtio-9p-xattr-user.c
@@ -12,7 +12,7 @@ 
  */
 
 #include <sys/types.h>
-#include "virtio.h"
+#include "hw/virtio.h"
 #include "virtio-9p.h"
 #include "fsdev/file-op-9p.h"
 #include "virtio-9p-xattr.h"
diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c
index 03c3d3f..f08ce6e 100644
--- a/hw/9pfs/virtio-9p-xattr.c
+++ b/hw/9pfs/virtio-9p-xattr.c
@@ -11,7 +11,7 @@ 
  *
  */
 
-#include "virtio.h"
+#include "hw/virtio.h"
 #include "virtio-9p.h"
 #include "fsdev/file-op-9p.h"
 #include "virtio-9p-xattr.h"
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index b5fc52b..ac5a1d0 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -11,8 +11,8 @@ 
  *
  */
 
-#include "virtio.h"
-#include "pc.h"
+#include "hw/virtio.h"
+#include "hw/pc.h"
 #include "qemu_socket.h"
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"