Re: [PATCH 1/3] hw/s390x: Declare target specific monitor commands in hmp-target.h

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> "monitor/hmp-target.h" is meant to hold target-specific commands.
> Move s390x specific commands there, slightly simplifying hmp-target.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/s390x/storage-attributes.h | 4 ----
>  include/hw/s390x/storage-keys.h       | 4 ----
>  include/monitor/hmp-target.h          | 5 +++++
>  hw/s390x/s390-skeys.c                 | 2 ++
>  hw/s390x/s390-stattrib.c              | 2 ++
>  monitor/hmp-target.c                  | 5 -----
>  6 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/s390x/storage-attributes.h 
> b/include/hw/s390x/storage-attributes.h
> index 8921a04d51..4916c75936 100644
> --- a/include/hw/s390x/storage-attributes.h
> +++ b/include/hw/s390x/storage-attributes.h
> @@ -13,7 +13,6 @@
>  #define S390_STORAGE_ATTRIBUTES_H
>  
>  #include "hw/qdev-core.h"
> -#include "monitor/monitor.h"
>  #include "qom/object.h"
>  
>  #define TYPE_S390_STATTRIB "s390-storage_attributes"
> @@ -73,7 +72,4 @@ static inline Object *kvm_s390_stattrib_create(void)
>  }
>  #endif
>  
> -void hmp_info_cmma(Monitor *mon, const QDict *qdict);
> -void hmp_migrationmode(Monitor *mon, const QDict *qdict);
> -
>  #endif /* S390_STORAGE_ATTRIBUTES_H */
> diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h
> index aa2ec2aae5..1d9b7ead44 100644
> --- a/include/hw/s390x/storage-keys.h
> +++ b/include/hw/s390x/storage-keys.h
> @@ -13,7 +13,6 @@
>  #define S390_STORAGE_KEYS_H
>  
>  #include "hw/qdev-core.h"
> -#include "monitor/monitor.h"
>  #include "qom/object.h"
>  
>  #define TYPE_S390_SKEYS "s390-skeys"
> @@ -114,7 +113,4 @@ void s390_skeys_init(void);
>  
>  S390SKeysState *s390_get_skeys_device(void);
>  
> -void hmp_dump_skeys(Monitor *mon, const QDict *qdict);
> -void hmp_info_skeys(Monitor *mon, const QDict *qdict);
> -
>  #endif /* S390_STORAGE_KEYS_H */
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index b679aaebbf..024cff0052 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -61,4 +61,9 @@ void hmp_gva2gpa(Monitor *mon, const QDict *qdict);
>  void hmp_gpa2hva(Monitor *mon, const QDict *qdict);
>  void hmp_gpa2hpa(Monitor *mon, const QDict *qdict);
>  
> +void hmp_dump_skeys(Monitor *mon, const QDict *qdict);
> +void hmp_info_skeys(Monitor *mon, const QDict *qdict);
> +void hmp_info_cmma(Monitor *mon, const QDict *qdict);
> +void hmp_migrationmode(Monitor *mon, const QDict *qdict);
> +

Could you please add a comment here saying that these are all s390,
since it's not obvious from their names.
(and if we're lucky the other s390 commands will stay with them).

Dave

>  #endif /* MONITOR_HMP_TARGET_H */
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 5c535d483e..7b2ccb94a5 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -23,6 +23,8 @@
>  #include "sysemu/kvm.h"
>  #include "migration/qemu-file-types.h"
>  #include "migration/register.h"
> +#include "monitor/hmp-target.h"
> +#include "monitor/monitor.h"
>  
>  #define S390_SKEYS_BUFFER_SIZE (128 * KiB)  /* Room for 128k storage keys */
>  #define S390_SKEYS_SAVE_FLAG_EOS 0x01
> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> index c4259b5327..9b4b8d8d0c 100644
> --- a/hw/s390x/s390-stattrib.c
> +++ b/hw/s390x/s390-stattrib.c
> @@ -19,6 +19,8 @@
>  #include "exec/ram_addr.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qdict.h"
> +#include "monitor/hmp-target.h"
> +#include "monitor/monitor.h"
>  #include "cpu.h"
>  
>  /* 512KiB cover 2GB of guest memory */
> diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
> index 1eb72ac1bf..0466474354 100644
> --- a/monitor/hmp-target.c
> +++ b/monitor/hmp-target.c
> @@ -36,11 +36,6 @@
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
>  
> -#if defined(TARGET_S390X)
> -#include "hw/s390x/storage-keys.h"
> -#include "hw/s390x/storage-attributes.h"
> -#endif
> -
>  /* Make devices configuration available for use in hmp-commands*.hx 
> templates */
>  #include CONFIG_DEVICES
>  
> -- 
> 2.41.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/