Re: [patch] remove vcpu_info array v5

[Avi Kivity <avi@redhat.com>]

Jes Sorensen wrote:
> Avi Kivity wrote:
>> Jes Sorensen wrote:
>>> Move contents of struct vcpu_info directly into CPU_COMMON. Rename
>>> struct qemu_kvm_work_item to qemu_work_item as it really isn't KVM
>>> specific.
>>
>> Well, it is actually kvm specific, since qemu doesn't have vccpu 
>> threads.
>
> Hi Avi,
>
> Anthony suggested I moved things directly in there, after my previous
> patch kept it as a struct.
>

The movement is fine, just keep it named kvm so we know it is kvm specific.

>>> -int kvm_run(kvm_context_t kvm, int vcpu)
>>> +int kvm_run(kvm_context_t kvm, int vcpu, void *env)
>>>   
>>
>> That's a little ugly -- passing two arguments which describe the 
>> vcpu.  How about passing env to kvm_create_vcpu() instead?
>
> I am all in favor of doing this, but in order to do so, we have to
> teach libkvm about CPUState - are you ok with that?

No, libkvm is independent of qemu.

We can make kvm_create_vcpu(kvm_context_t kvm, int vcpu, void *env), and 
pass 'env' as the opaque to all the callbacks, instead of the vcpu number.


>
>>
>> struct kvm_stuff { ... } and put that in as a member, so it's easier 
>> to remember this is a kvm addition.
>
> If we can agree, I really don't care too much. There's already plenty
> of stuff in CPU_COMMON that isn't used by specific users, so I didn't
> think this was an issue.
>

It's more of a maintenance issue so that I can keep track of what is in 
qemu upstream and what is local.  Merges are already painful enough.

>>>  
>>
>> Please keep this in kvm specific files.
>
> Why? There really isn't anything specific about a work queue list
> like this. Making it an available functionality for QEMU shouldn't
> hurt.
>

It's only relevant if you have per-vcpu threads, which qemu doesn't have.

>>> -#define bool _Bool
>>>   
>>
>> why?
>
> It was unused and didn't add any value.
>

So separate patch.

>> Please separate into a patch that does the kvm_run int vcpu -> void 
>> *vcpu conversion, and then the vcpu_info -> env conversion.
>
> As mentioned above, if I am going to go that way, we need to teach
> libkvm about 'env'. I am all in favor, but I am not going to do that
> unless I can get an ack that it's acceptable to do so.

How's my suggestion above?

-- 
error compiling committee.c: too many arguments to function