Re: [PATCH] KVM-USER: Check kvm extensions at runtime

["Gregory Haskins" <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>]

>>> On Wed, May 16, 2007 at  7:47 AM, in message <464AEF5B.3010104-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote: 
> Gregory Haskins wrote:
>> Newer userspace may run on an older kernel.  Therefore we need a way to 
>> check the capabilities of the kernel so that we can downgrade userspace
>> dynamically if necessary
>>
>> Signed- off- by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>> ---
>>
>>  qemu/qemu- kvm.c |   19 +++++++++++++++++++
>>  user/kvmctl.c   |   14 ++++++++++++++
>>  user/kvmctl.h   |    5 +++++
>>  3 files changed, 38 insertions(+), 0 deletions(- )
>>
>> diff -- git a/qemu/qemu- kvm.c b/qemu/qemu- kvm.c
>> index 59e79bf..6096f21 100644
>> ---  a/qemu/qemu- kvm.c
>> +++ b/qemu/qemu- kvm.c
>> @@ - 697,6 +697,25 @@ int kvm_qemu_init()
>>  int kvm_qemu_create_context(void)
>>  {
>>      int i;
>> +    uint64_t ext;
>> +
>> +    if (kvm_check_extension(kvm_context, &ext) < 0) {
>> +	kvm_qemu_destroy();
>> +	return - 1;
>> +    }
>> +
>> +    if (!(ext & KVM_EXTENSION_LAPIC) && kvm_apic_level) {
>> +	/*
>> +	 * Opps... the kernel doesnt support apic- emulation even though
>> +	 * the user wants it.  We must turn this off and warn the user
>> +	 */
>> +	kvm_apic_level = 0;
>> +
>> +	/* FIXME: We should log this officially */
>> +	printf("WARNING: older kernel does not support apic " \
>> +	       "emulation.  Falling back to userspace emulation.  Upgrade " \
>> +	       "your kernel/modules\n");
>> +    }
>>   
> 
> The user may not be in a position to upgrade the kernel.  Think distro 
> kernel and userspace.  A warning is a bit severe.

Ack.  Should I drop the printf all together?  Or just get rid of the "WARNING" and "please upgrade" part?.  Also, is there a better way to log in this thing (instead of to stdout?)

All the rest of your comments: Ack.

> 
>>  
>>      if (kvm_create(kvm_context, phys_ram_size, kvm_apic_level,
>>  		   (void**)&phys_ram_base) < 0) {
>> diff -- git a/user/kvmctl.c b/user/kvmctl.c
>> index ea86426..fcabc1a 100644
>> ---  a/user/kvmctl.c
>> +++ b/user/kvmctl.c
>> @@ - 889,3 +889,17 @@ int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, 
> const sigset_t *sigset)
>>  	free(sigmask);
>>  	return r;
>>  }
>> +
>> +int kvm_check_extension(kvm_context_t kvm, uint64_t *ext)
>> +{
>> +	*ext = 0;
>> +
>> +	int r = ioctl(kvm- >fd, KVM_CHECK_EXTENSION, KVM_APIC_MSG);
>> +	if (r < 0)
>> +		return r;
>>   
> 
> - errno for new code, please.
> 
>> +
>> +	if (r)
>> +		*ext |= KVM_EXTENSION_LAPIC;
>> +
>> +	return 0;
>> +}
>>   
> 
> How about a struct with a field per extension instead of bitmasks?




-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/