From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH] xen/keyhandler: Rework keyhandler
	infrastructure
Date: Tue, 22 Sep 2015 12:10:35 +0100
Message-ID: <5601372B.6080404@citrix.com>
References: <1442238589-2531-1-git-send-email-andrew.cooper3@citrix.com>
	<56014B2602000078000A4492@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <56014B2602000078000A4492@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>, Xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On 22/09/15 11:35, Jan Beulich wrote:
>
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -1858,15 +1858,9 @@ static void vmcs_dump(unsigned char ch)
>>       printk("**************************************\n");
>>   }
>>   
>> -static struct keyhandler vmcs_dump_keyhandler = {
>> -    .diagnostic = 1,
>> -    .u.fn = vmcs_dump,
>> -    .desc = "dump Intel's VMCS"
>> -};
>> -
>>   void __init setup_vmcs_dump(void)
>>   {
>> -    register_keyhandler('v', &vmcs_dump_keyhandler);
>> +    register_keyhandler('v', vmcs_dump, "dump Intel's VMCS", 1);
>>   }
> Mind switching this from "Intel" to VMX as you go, considering that
> it's not just Intel using that structure?

I can certainly see about correcting some of the text.  I suppose this 
is a good time to make it all consistent.


>
>> +
>> +    key_table[key] = k;
> I don't really see the value of the intermediate variable k here.

Nor me - I thing it might have been a way of how the patch developed.


>
>> @@ -1901,15 +1895,9 @@ static void dump_heap(unsigned char key)
>>       }
>>   }
>>   
>> -static struct keyhandler dump_heap_keyhandler = {
>> -    .diagnostic = 1,
>> -    .u.fn = dump_heap,
>> -    .desc = "dump heap info"
>> -};
>> -
>>   static __init int register_heap_trigger(void)
>>   {
>> -    register_keyhandler('H', &dump_heap_keyhandler);
>> +    register_keyhandler('H', dump_heap, "dump heap info", 1);
> Considering the other one in this file is "memory info", just
> "heap info" perhaps?

I will audit all the text in v2.

~Andrew