Re: [Qemu-devel] [PATCH 02/11] migration: Remove unneeded includes of migration/vmstate.h

[Juan Quintela]

Laurent Vivier <address@hidden> wrote:
> On 18/05/2017 19:25, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <address@hidden>
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>>  hw/core/qdev.c                   | 1 -
>>  include/hw/acpi/memory_hotplug.h | 1 -
>>  include/hw/acpi/pcihp.h          | 1 -
>>  include/hw/pci/shpc.h            | 1 -
>>  include/net/net.h                | 1 -
>>  target/alpha/cpu.c               | 1 -
>>  target/hppa/cpu.c                | 1 -
>>  target/s390x/cpu.c               | 1 -
>>  target/tilegx/cpu.c              | 1 -
>>  9 files changed, 9 deletions(-)
>> 
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 71ff95f..0ce45a2 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -37,7 +37,6 @@
>>  #include "hw/boards.h"
>>  #include "hw/sysbus.h"
>>  #include "qapi-event.h"
>> -#include "migration/vmstate.h"
>>  
>>  bool qdev_hotplug = false;
>>  static bool qdev_hot_added = false;
>
> This one uses "VMStateDescription", "vmstate_check_only_migratable",
> "vmstate_register_with_alias_id" and "vmstate_unregister" from
> migration/savevm.h

#includes "hw/qdev.h"

which 1st include is

#include "hw/hw.h"

which include "migration/vmstate.h"

After this series the only include file that includes
migration/vmstate.h is hw/hw.h.  Removing it makes no sense (IMHO)
because it would mean including it directly in more than half of the tree.

>
>> diff --git a/include/hw/acpi/memory_hotplug.h 
>> b/include/hw/acpi/memory_hotplug.h
>> index db8ebc9..77c6576 100644
>> --- a/include/hw/acpi/memory_hotplug.h
>> +++ b/include/hw/acpi/memory_hotplug.h
>> @@ -3,7 +3,6 @@
>>  
>>  #include "hw/qdev-core.h"
>>  #include "hw/acpi/acpi.h"
>> -#include "migration/vmstate.h"
>>  #include "hw/acpi/aml-build.h"
>>  
>>  /**
>
> This one uses "VMSTATE_STRUCT()" which is defined in migration/vmstate.h

#include "hw/acpi/acpi.h"

which includes:

#include "hw/acpi/acpi_dev_interface.h"

->

#include "hw/boards.h"

->

#include "hw/hw.h"

>> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
>> index 04528b7..8a65f99 100644
>> --- a/include/hw/acpi/pcihp.h
>> +++ b/include/hw/acpi/pcihp.h
>> @@ -28,7 +28,6 @@
>>  #define HW_ACPI_PCIHP_H
>>  
>>  #include "hw/acpi/acpi.h"
>> -#include "migration/vmstate.h"
>>  #include "hw/hotplug.h"
>>  
>>  #define ACPI_PCIHP_IO_BASE_PROP "acpi-pcihp-io-base"
>
> This one uses "VMSTATE_STRUCT()" which is defined in migration/vmstate.h

see previous one

>> diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h
>> index b208554..71e836b 100644
>> --- a/include/hw/pci/shpc.h
>> +++ b/include/hw/pci/shpc.h
>> @@ -3,7 +3,6 @@
>>  
>>  #include "qemu-common.h"
>>  #include "exec/memory.h"
>> -#include "migration/vmstate.h"
>>  #include "hw/hotplug.h"
>>  #include "hw/pci/pci.h"
>
> This one uses "VMSTATE_BUFFER_UNSAFE_INFO_TEST()" which is defined in
> migration/vmstate.h.

#include "hw/pci/pci.h"
->
#include "hw/qedev.h"
->
#included "hw/hw.h"

>>  
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 99b28d5..ba663bd 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -6,7 +6,6 @@
>>  #include "qapi/qmp/qdict.h"
>>  #include "qemu/option.h"
>>  #include "net/queue.h"
>> -#include "migration/vmstate.h"
>>  #include "qapi-types.h"
>>  
>>  #define MAC_FMT "%02X:%02X:%02X:%02X:%02X:%02X"
>
> This one uses "vmstate_offset_array()" which is defined in
> migration/vmstate.h
>
> Perhaps you can explain why you think they are unneeded?

In all previous ones, they arrive to hw/hw.h for means that are not
going to be removed.  For this one, I am getting trouble finding a
normal chain.

#include "qemu/osdep.h"
#include "net/net.h"

with this change works.  But I am not able to find *why*.

So, I am dropping this last bit.

Is that ok for you?

Later, Juan.