diff mbox

[v3,5/8] s390x: Dump-skeys hmp support

Message ID 1441018834-8993-6-git-send-email-cornelia.huck@de.ibm.com
State New
Headers show

Commit Message

Cornelia Huck Aug. 31, 2015, 11 a.m. UTC 
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

Add dump-skeys command to the human monitor.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hmp-commands.hx                 | 16 ++++++++++++++++
 hw/s390x/s390-skeys.c           | 12 ++++++++++++
 include/hw/s390x/storage-keys.h |  2 ++
 monitor.c                       |  4 ++++
 4 files changed, 34 insertions(+)

Comments

Eric Blake Aug. 31, 2015, 4:30 p.m. UTC | #1 
On 08/31/2015 05:00 AM, Cornelia Huck wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> Add dump-skeys command to the human monitor.
> 
> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hmp-commands.hx                 | 16 ++++++++++++++++
>  hw/s390x/s390-skeys.c           | 12 ++++++++++++
>  include/hw/s390x/storage-keys.h |  2 ++
>  monitor.c                       |  4 ++++
>  4 files changed, 34 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..803ff91 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>              together with begin.
>  ETEXI
>  
> +#if defined(TARGET_S390X)
> +    {
> +        .name       = "dump-skeys",

Most HMP commands use '_', not '-', for word separation.
Jason J. Herne Aug. 31, 2015, 6:57 p.m. UTC | #2 
On 08/31/2015 12:30 PM, Eric Blake wrote:
> On 08/31/2015 05:00 AM, Cornelia Huck wrote:
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> Add dump-skeys command to the human monitor.
>>
>> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> ---
>>   hmp-commands.hx                 | 16 ++++++++++++++++
>>   hw/s390x/s390-skeys.c           | 12 ++++++++++++
>>   include/hw/s390x/storage-keys.h |  2 ++
>>   monitor.c                       |  4 ++++
>>   4 files changed, 34 insertions(+)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index d3b7932..803ff91 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>>               together with begin.
>>   ETEXI
>>
>> +#if defined(TARGET_S390X)
>> +    {
>> +        .name       = "dump-skeys",
>
> Most HMP commands use '_', not '-', for word separation.
>

I patterned my new command after dump-guest-memory since the functionality
was similar. Though it is easy enough to change if you would like.
Cornelia Huck Sept. 1, 2015, 2:30 p.m. UTC | #3 
On Mon, 31 Aug 2015 14:57:51 -0400
"Jason J. Herne" <jjherne@linux.vnet.ibm.com> wrote:

> On 08/31/2015 12:30 PM, Eric Blake wrote:
> > On 08/31/2015 05:00 AM, Cornelia Huck wrote:
> >> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> >>
> >> Add dump-skeys command to the human monitor.
> >>
> >> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> >> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> >> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> >> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> >> ---
> >>   hmp-commands.hx                 | 16 ++++++++++++++++
> >>   hw/s390x/s390-skeys.c           | 12 ++++++++++++
> >>   include/hw/s390x/storage-keys.h |  2 ++
> >>   monitor.c                       |  4 ++++
> >>   4 files changed, 34 insertions(+)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index d3b7932..803ff91 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
> >>               together with begin.
> >>   ETEXI
> >>
> >> +#if defined(TARGET_S390X)
> >> +    {
> >> +        .name       = "dump-skeys",
> >
> > Most HMP commands use '_', not '-', for word separation.
> >
> 
> I patterned my new command after dump-guest-memory since the functionality
> was similar. Though it is easy enough to change if you would like.

Eric, do you have a strong preference? I think either is fine; I can
either keep it as-is or merge in a change for the pull.
Eric Blake Sept. 1, 2015, 4:05 p.m. UTC | #4 
On 09/01/2015 08:30 AM, Cornelia Huck wrote:

>>>> +++ b/hmp-commands.hx
>>>> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>>>>               together with begin.
>>>>   ETEXI
>>>>
>>>> +#if defined(TARGET_S390X)
>>>> +    {
>>>> +        .name       = "dump-skeys",
>>>
>>> Most HMP commands use '_', not '-', for word separation.
>>>
>>
>> I patterned my new command after dump-guest-memory since the functionality
>> was similar. Though it is easy enough to change if you would like.
> 
> Eric, do you have a strong preference? I think either is fine; I can
> either keep it as-is or merge in a change for the pull.

HMP is not ABI; we can change it at will without worrying about
back-compat issues.  Consistency is nice, so I'd lean towards
consistently using _ throughout HMP, but the preference is not strong
enough so I don't object to keeping this commit as-is for the sake of
merging, especially if dump-guest-memory also needs changing (that is, a
followup patch that changes all HMP to be consistent in one go is not
that much harder, even if this patch adds to the workload of that patch).
Cornelia Huck Sept. 1, 2015, 4:22 p.m. UTC | #5 
On Tue, 1 Sep 2015 10:05:17 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 09/01/2015 08:30 AM, Cornelia Huck wrote:
> 
> >>>> +++ b/hmp-commands.hx
> >>>> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
> >>>>               together with begin.
> >>>>   ETEXI
> >>>>
> >>>> +#if defined(TARGET_S390X)
> >>>> +    {
> >>>> +        .name       = "dump-skeys",
> >>>
> >>> Most HMP commands use '_', not '-', for word separation.
> >>>
> >>
> >> I patterned my new command after dump-guest-memory since the functionality
> >> was similar. Though it is easy enough to change if you would like.
> > 
> > Eric, do you have a strong preference? I think either is fine; I can
> > either keep it as-is or merge in a change for the pull.
> 
> HMP is not ABI; we can change it at will without worrying about
> back-compat issues.  Consistency is nice, so I'd lean towards
> consistently using _ throughout HMP, but the preference is not strong
> enough so I don't object to keeping this commit as-is for the sake of
> merging, especially if dump-guest-memory also needs changing (that is, a
> followup patch that changes all HMP to be consistent in one go is not
> that much harder, even if this patch adds to the workload of that patch).

Given that there are even more commands using - and that we can change
this later on, I'll just leave it as-is.
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..803ff91 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1053,6 +1053,22 @@  gdb. Without -z|-l|-s, the dump format is ELF.
             together with begin.
 ETEXI
 
+#if defined(TARGET_S390X)
+    {
+        .name       = "dump-skeys",
+        .args_type  = "filename:F",
+        .params     = "",
+        .help       = "Save guest storage keys into file 'filename'.\n",
+        .mhandler.cmd = hmp_dump_skeys,
+    },
+#endif
+
+STEXI
+@item dump-skeys @var{filename}
+@findex dump-skeys
+Save guest storage keys to a file.
+ETEXI
+
     {
         .name       = "snapshot_blkdev",
         .args_type  = "reuse:-n,device:B,snapshot-file:s?,format:s?",
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index ea5297d..0ef3aea 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -65,6 +65,18 @@  static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
     }
 }
 
+void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
+{
+    const char *filename = qdict_get_str(qdict, "filename");
+    Error *err = NULL;
+
+    qmp_dump_skeys(filename, &err);
+    if (err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(err));
+        error_free(err);
+    }
+}
+
 void qmp_dump_skeys(const char *filename, Error **errp)
 {
     S390SKeysState *ss = s390_get_skeys_device();
diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h
index cfd7da7..0d04f19 100644
--- a/include/hw/s390x/storage-keys.h
+++ b/include/hw/s390x/storage-keys.h
@@ -13,6 +13,7 @@ 
 #define __S390_STORAGE_KEYS_H
 
 #include <hw/qdev.h>
+#include "monitor/monitor.h"
 
 #define TYPE_S390_SKEYS "s390-skeys"
 #define S390_SKEYS(obj) \
@@ -52,4 +53,5 @@  void s390_skeys_init(void);
 
 S390SKeysState *s390_get_skeys_device(void);
 
+void hmp_dump_skeys(Monitor *mon, const QDict *qdict);
 #endif /* __S390_STORAGE_KEYS_H */
diff --git a/monitor.c b/monitor.c
index daa3d98..3deba38 100644
--- a/monitor.c
+++ b/monitor.c
@@ -82,6 +82,10 @@ 
 #endif
 #include "hw/lm32/lm32_pic.h"
 
+#if defined(TARGET_S390X)
+#include "hw/s390x/storage-keys.h"
+#endif
+
 /*
  * Supported types:
  *