Re: [Qemu-devel] [PATCH 2/8] s390: autodetect map private

[Jan Kiszka]

On 2012-06-13 14:33, Alexander Graf wrote:
> On 06/13/2012 01:41 PM, Jan Kiszka wrote:
>> On 2012-06-13 13:27, Christian Borntraeger wrote:
>>> On 13/06/12 12:58, Jan Kiszka wrote:
>>>>> Thinking about this a bit more, how about
>>>>>
>>>>> } else if (!kvm_arch_vmalloc(size,&new_block->host)) {
>>>>> <normal code>
>>>>> }
>>>>>
>>> I like that. Of course, we have to have a generic kvm_arch_vmalloc 
>>> implementation
>>> then.
>> Then better go for kvm_vmalloc calling kvm_arch_vmalloc (in the s390 case).
>>
>> However, I do not like the variation of parameters and return value
>> compared to normal *alloc. Better:
>>
>> memory = kvm_vmalloc(size);
>> if (!memory)
>>      memory = qemu_vmalloc(size);
>>
>> But more regular (when looking at the Xen block) is guarding the call
>> with kvm_enabled() and embedding qemu_vmalloc in kvm_vmalloc.
> 
> So basically
> 
> #ifdef CONFIG_TARGET_S390X
>    } else if (kvm_enabled()) {
>      memory = kvm_vmalloc();
>    } else {
> #endif
> 
> or a generic
> 
> } else if (kvm_enabled()) {
>    memory = kvm_vmalloc();
> } else {
> 
> ? Because that one would mean we always duplicate the common 
> qemu_vmalloc case. But then again, that one's only a single call, so 
> maybe it's ok. Meh - I'll let you decide :).

I'm fine with the #ifdef-free version if providing some comment which
arch requires this special path.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux