Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
CONFIG_VFIO.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 migration/target.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/migration/target.c b/migration/target.c
index 907ebf0..4390bf0 100644
--- a/migration/target.c
+++ b/migration/target.c
@@ -8,18 +8,22 @@
 #include "qemu/osdep.h"
 #include "qapi/qapi-types-migration.h"
 #include "migration.h"
+#include CONFIG_DEVICES
 
 #ifdef CONFIG_VFIO
+
 #include "hw/vfio/vfio-common.h"
-#endif
 
 void populate_vfio_info(MigrationInfo *info)
 {
-#ifdef CONFIG_VFIO
     if (vfio_mig_active()) {
         info->has_vfio = true;
         info->vfio = g_malloc0(sizeof(*info->vfio));
         info->vfio->transferred = vfio_mig_bytes_transferred();
     }
-#endif
 }
+#else
+
+void populate_vfio_info(MigrationInfo *info) {}
+
+#endif /* CONFIG_VFIO */
-- 
1.8.3.1

On Wed, 22 Dec 2021 at 19:45, Steve Sistare <steven.sistare@oracle.com> wrote:
>
> Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
> CONFIG_VFIO.

The commit message says "include CONFIG_DEVICES"...

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  migration/target.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/migration/target.c b/migration/target.c
> index 907ebf0..4390bf0 100644
> --- a/migration/target.c
> +++ b/migration/target.c
> @@ -8,18 +8,22 @@
>  #include "qemu/osdep.h"
>  #include "qapi/qapi-types-migration.h"
>  #include "migration.h"
> +#include CONFIG_DEVICES

...and the code change does do that, but...

>
>  #ifdef CONFIG_VFIO
> +
>  #include "hw/vfio/vfio-common.h"
> -#endif
>
>  void populate_vfio_info(MigrationInfo *info)
>  {
> -#ifdef CONFIG_VFIO
>      if (vfio_mig_active()) {
>          info->has_vfio = true;
>          info->vfio = g_malloc0(sizeof(*info->vfio));
>          info->vfio->transferred = vfio_mig_bytes_transferred();
>      }
> -#endif
>  }
> +#else
> +
> +void populate_vfio_info(MigrationInfo *info) {}
> +
> +#endif /* CONFIG_VFIO */

...it also seems to be making a no-change-of-behaviour rewrite
of the rest of the file. Is there a reason I'm missing for doing
that ?

thanks
-- PMM

On 2/24/2022 1:42 PM, Peter Maydell wrote:
> On Wed, 22 Dec 2021 at 19:45, Steve Sistare <steven.sistare@oracle.com> wrote:
>>
>> Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
>> CONFIG_VFIO.
> 
> The commit message says "include CONFIG_DEVICES"...
> 
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  migration/target.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/target.c b/migration/target.c
>> index 907ebf0..4390bf0 100644
>> --- a/migration/target.c
>> +++ b/migration/target.c
>> @@ -8,18 +8,22 @@
>>  #include "qemu/osdep.h"
>>  #include "qapi/qapi-types-migration.h"
>>  #include "migration.h"
>> +#include CONFIG_DEVICES
> 
> ...and the code change does do that, but...
> 
>>
>>  #ifdef CONFIG_VFIO
>> +
>>  #include "hw/vfio/vfio-common.h"
>> -#endif
>>
>>  void populate_vfio_info(MigrationInfo *info)
>>  {
>> -#ifdef CONFIG_VFIO
>>      if (vfio_mig_active()) {
>>          info->has_vfio = true;
>>          info->vfio = g_malloc0(sizeof(*info->vfio));
>>          info->vfio->transferred = vfio_mig_bytes_transferred();
>>      }
>> -#endif
>>  }
>> +#else
>> +
>> +void populate_vfio_info(MigrationInfo *info) {}
>> +
>> +#endif /* CONFIG_VFIO */
> 
> ...it also seems to be making a no-change-of-behaviour rewrite
> of the rest of the file. Is there a reason I'm missing for doing
> that ?
> 
> thanks
> -- PMM

I'll change the commit message to explain:

    Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
    CONFIG_VFIO, and refactor so only one ifdef is needed when new functions
    are added in a later patch. 
  
The later patch is "vfio-pci: cpr part 1 (fd and dma)"

- Steve

On Thu, 3 Mar 2022 at 15:55, Steven Sistare <steven.sistare@oracle.com> wrote:
>
> On 2/24/2022 1:42 PM, Peter Maydell wrote:
> > ...it also seems to be making a no-change-of-behaviour rewrite
> > of the rest of the file. Is there a reason I'm missing for doing
> > that ?

> I'll change the commit message to explain:
>
>     Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
>     CONFIG_VFIO, and refactor so only one ifdef is needed when new functions
>     are added in a later patch.
>
> The later patch is "vfio-pci: cpr part 1 (fd and dma)"

I'd prefer it if you split this patch into two patches; these two changes
aren't related.

thanks
-- PMM

On 3/3/2022 11:21 AM, Peter Maydell wrote:
> On Thu, 3 Mar 2022 at 15:55, Steven Sistare <steven.sistare@oracle.com> wrote:
>>
>> On 2/24/2022 1:42 PM, Peter Maydell wrote:
>>> ...it also seems to be making a no-change-of-behaviour rewrite
>>> of the rest of the file. Is there a reason I'm missing for doing
>>> that ?
> 
>> I'll change the commit message to explain:
>>
>>     Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
>>     CONFIG_VFIO, and refactor so only one ifdef is needed when new functions
>>     are added in a later patch.
>>
>> The later patch is "vfio-pci: cpr part 1 (fd and dma)"
> 
> I'd prefer it if you split this patch into two patches; these two changes
> aren't related.
> 
> thanks
> -- PMM


Will do - Steve