[Qemu-devel] [PATCH 16/67] migration: use local path for local headers

Michael S. Tsirkin posted 67 patches 7 years, 5 months ago
[Qemu-devel] [PATCH 16/67] migration: use local path for local headers
Posted by Michael S. Tsirkin 7 years, 5 months ago
When pulling in headers that are in the same directory as C file (as
opposed to one in include/), we should use its relative path, without a
directory. Directory based path works more or less by accident.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 migration/block-dirty-bitmap.c | 2 +-
 migration/page_cache.c         | 2 +-
 migration/ram.c                | 4 ++--
 migration/vmstate.c            | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index dd04f10..09c086d 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -66,7 +66,7 @@
 #include "qemu/error-report.h"
 #include "migration/misc.h"
 #include "migration/migration.h"
-#include "migration/qemu-file.h"
+#include "qemu-file.h"
 #include "migration/vmstate.h"
 #include "migration/register.h"
 #include "qemu/hbitmap.h"
diff --git a/migration/page_cache.c b/migration/page_cache.c
index 96268c3..acc252b 100644
--- a/migration/page_cache.c
+++ b/migration/page_cache.c
@@ -18,7 +18,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "qemu/host-utils.h"
-#include "migration/page_cache.h"
+#include "page_cache.h"
 
 #ifdef DEBUG_CACHE
 #define DPRINTF(fmt, ...) \
diff --git a/migration/ram.c b/migration/ram.c
index 0e90efa..34a34d9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -40,7 +40,7 @@
 #include "migration/misc.h"
 #include "qemu-file.h"
 #include "postcopy-ram.h"
-#include "migration/page_cache.h"
+#include "page_cache.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-migration.h"
@@ -50,7 +50,7 @@
 #include "exec/target_page.h"
 #include "qemu/rcu_queue.h"
 #include "migration/colo.h"
-#include "migration/block.h"
+#include "block.h"
 
 /***********************************************************/
 /* ram save/restore */
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 0b3282c..0a09636 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -14,7 +14,7 @@
 #include "qemu-common.h"
 #include "migration.h"
 #include "migration/vmstate.h"
-#include "migration/savevm.h"
+#include "savevm.h"
 #include "qemu-file.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
-- 
MST


Re: [Qemu-devel] [PATCH 16/67] migration: use local path for local headers
Posted by Juan Quintela 7 years, 5 months ago
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> When pulling in headers that are in the same directory as C file (as
> opposed to one in include/), we should use its relative path, without a
> directory. Directory based path works more or less by accident.

No, it is not by accident.
qemu-version.h and config-host.h are on the root directory.  I could
agree with moving them.

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  migration/block-dirty-bitmap.c | 2 +-
>  migration/page_cache.c         | 2 +-
>  migration/ram.c                | 4 ++--
>  migration/vmstate.c            | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index dd04f10..09c086d 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -66,7 +66,7 @@
>  #include "qemu/error-report.h"
>  #include "migration/misc.h"
>  #include "migration/migration.h"
> -#include "migration/qemu-file.h"
> +#include "qemu-file.h"

Substitite for "./qemu-file.h"?

My wonder here is what happens if we end with a file with the same name in
two places.

Later, Juan.

Re: [Qemu-devel] [PATCH 16/67] migration: use local path for local headers
Posted by Eric Blake 7 years, 5 months ago
On 05/08/2018 07:21 AM, Juan Quintela wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> When pulling in headers that are in the same directory as C file (as
>> opposed to one in include/), we should use its relative path, without a
>> directory. Directory based path works more or less by accident.
> 
> No, it is not by accident.
> qemu-version.h and config-host.h are on the root directory.  I could
> agree with moving them.
> 

>> -#include "migration/qemu-file.h"
>> +#include "qemu-file.h"
> 
> Substitite for "./qemu-file.h"?
> 
> My wonder here is what happens if we end with a file with the same name in
> two places.

We already have at least:

include/qapi/qmp/qjson.h
migration/qjson.h

so it's not necessarily a theoretical question (things work now, but 
renaming one or the other header may be in order as part of 
consolidating everything into the /include hierarchy)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 16/67] migration: use local path for local headers
Posted by Juan Quintela 7 years, 5 months ago
Eric Blake <eblake@redhat.com> wrote:
> On 05/08/2018 07:21 AM, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>> When pulling in headers that are in the same directory as C file (as
>>> opposed to one in include/), we should use its relative path, without a
>>> directory. Directory based path works more or less by accident.
>>
>> No, it is not by accident.
>> qemu-version.h and config-host.h are on the root directory.  I could
>> agree with moving them.
>>
>
>>> -#include "migration/qemu-file.h"
>>> +#include "qemu-file.h"
>>
>> Substitite for "./qemu-file.h"?
>>
>> My wonder here is what happens if we end with a file with the same name in
>> two places.
>
> We already have at least:
>
> include/qapi/qmp/qjson.h
> migration/qjson.h
>
> so it's not necessarily a theoretical question (things work now, but
> renaming one or the other header may be in order as part of
> consolidating everything into the /include hierarchy)

Internal ones also?  The whole reason why I moved them to migration/* in
the first place was to be sure that they are internal, and that nobody
else uses them.

Later, Juan.

PD.  And yes, I realize that we include $(ROOT) in the include path, and
then

#include "migration/migration.h" still works from any place.

sniff

Re: [Qemu-devel] [PATCH 16/67] migration: use local path for local headers
Posted by Eric Blake 7 years, 5 months ago
On 05/08/2018 09:28 AM, Juan Quintela wrote:

>>> My wonder here is what happens if we end with a file with the same name in
>>> two places.
>>
>> We already have at least:
>>
>> include/qapi/qmp/qjson.h
>> migration/qjson.h
>>
>> so it's not necessarily a theoretical question (things work now, but
>> renaming one or the other header may be in order as part of
>> consolidating everything into the /include hierarchy)
> 
> Internal ones also?  The whole reason why I moved them to migration/* in
> the first place was to be sure that they are internal, and that nobody
> else uses them.
> 
> Later, Juan.
> 
> PD.  And yes, I realize that we include $(ROOT) in the include path, and
> then
> 
> #include "migration/migration.h" still works from any place.

I think the goal of this series was to remove $(ROOT) from the include 
path, so that all headers are either internal or obviously shared (and 
one of the review comments mentioned that all of the earlier commits in 
the series should directly mention this goal as part of the commit 
message, rather than claiming just "works more or less by accident"). 
It's been a good RFC for getting feedback, and some of the patches are 
worth applying even if others get reworked, but there are definitely 
some questions to be resolved before v2 of the series.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org