Simon Kagstrom wrote:
> (I've edited the patch comments to show the reasoning)
>
> At Tue, 14 Nov 2006 13:44:31 +0200,
> Avi Kivity wrote:
>
>> /* Initialize kvm */
>> if (kvm_allowed) {
>> if (kvm_qemu_init() < 0) {
>> fprintf(stderr, "Could not initialize KVM\n");
>> kvm_allowed = 0;
>> }
>> }
>> /* Fall back to normal qemu if KVM fails */
>> if (!kvm_allowed) {
>> phys_ram_base = qemu_vmalloc(phys_ram_size);
>> if (!phys_ram_base) {
>> fprintf(stderr, "Could not allocate physical memory\n");
>> exit(1);
>> }
>> }
>>
>> This bit is rather awkward, since it disabled kvm after it has been
>> enabled. It's a cleaner to never enable kvm_allowed in the first place
>> (i.e. during argument parsing).
>>
>
> OK. My reasoning was this: qemu with KVM defaults to use kvm and
> therefore tries to enable it (kvm_qemu_init), if it cannot be
> initialized, it will disable it again.
>
Yes, but it leads to code duplication (there's a qemu_vmalloc() outside
USE_KVM). Also, if some code prior to this depended on kvm_allowed, it
would be invalidated.
Best to set kvm_allowed once you're sure it's really allowed, and then
don't touch it.
> I can submit a new patch where kvm_init and kvm_create is separated
> and kvm_init is done at argument-parsing time if you like, but
> personally I think this way is clearer :-)
>
>
>> How about a
>>
>> int libkvm_init(kvm_libcontext_t *libcontext);
>> void libkvm_exit(kvm_libcontext_t libcontext);
>>
>> int kvm_create(kvm_libcontext_t libcontext, kvm_context_t* vm);
>> void kvm_destroy(kvm_context_t *vm);
>>
I meant kvm_destroy(kvm_context_t vm);
>> at present, libkvm_init() can just call access(2) on /dev/kvm. In the
>> future, it is planned that open("/dev/kvm") will not create a VM (it
>> just opens a communication channel to the driver), so that libkvm_init()
>> will open the device and store the fd.
>>
>
> Personally, I would prefer something like
>
> int kvm_open(kvm_t *kvm;)
> void kvm_close(kvm_t *kvm);
>
> int kvm_create_vm(kvm_t *kvm, kvm_context_t *vm);
> void kvm_destroy_vm(kvm_t *kvm, kvm_context_t *vm);
>
That means that if sizeof(kvm_t) changes, we need to recompile
everything. With kvm_context_t and kvm_t as opaque pointers, we preserve
binary compatibility when their definitions change.
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
