Re: [PATCH] Support for QEMU's CVS

[Anthony Liguori <aliguori-NZpS4cJIG2HvQtjrzfazuQ@public.gmane.org>]

Avi Kivity wrote:
> There are four issues I can see:
>
> - if there are multiple sources of kvm userspace, there are bound to 
> be problems with incompatible kernel API and userspace, especially as 
> I have plans for extensive changes to the API.  I'll go and implement 
> a version check now, so that doesn't become a barrier to anything.  A 
> submission to qemu-devel should include this check.

With KQEMU, you have to pair a version of KQEMU with the appropriate 
version of QEMU.  I don't think there's much of a problem doing 
something like this for KVM for the short term until API gets more 
stable (provided there's a version check so the error is useful to the 
user).

> - the API changes are bound to cause more churn than necessary.  I 
> guess we can live with that though.

Yeah, I don't think this is a huge problem.

> - the patch contains some qemu changes that date from the days we did 
> V2E, and are therefore unnecessary.  I'll point them out below.

Right, I noticed a few things that seemed unnecessary.

> - where does libkvm live? an external package?

I would think an external package.

There are a couple more issues I know of:

 - kqemu and kvm don't play very well together at the moment.  We need 
to revisit some of the logic to ensure that kqemu and kvm are never 
enabled at the same time.

 - the USE_KVM guards are probably unnecessary.  We should be able to 
isolate all of the libkvmctl specific stuff to qemu-kvm.c and then stub 
things out as needed.

>> diff -r 2eac80033ea0 hw/usb-hid.c
>> --- a/hw/usb-hid.c    Tue Dec 19 09:31:34 2006 +0000
>> +++ b/hw/usb-hid.c    Wed Dec 20 23:01:25 2006 -0600
>> @@ -169,7 +169,9 @@ static const uint8_t qemu_tablet_config_
>>      0x81,       /*  u8  ep_bEndpointAddress; IN Endpoint 1 */
>>       0x03,       /*  u8  ep_bmAttributes; Interrupt */
>>       0x08, 0x00, /*  u16 ep_wMaxPacketSize; */
>> -    0x03,       /*  u8  ep_bInterval; (255ms -- usb 2.0 spec) */
>> +    /* Temporarily increase usb polling interval to prevent cpu
>> +     * saturation (3 ms is way too often for kvm) */
>> +    0x33,       /*  u8  ep_bInterval; (255ms -- usb 2.0 spec) */
>>   
>
> This might be a good change for qemu, but on the other hand this is a 
> kvm deficiency that is forced on non-kvm users.

Brad Campbell (who wrote the original HID description) has recently 
posted a patch that changes the interval to 0x20.  0x03 was causing a 
performance problems with Windows guests (there's a pretty recent thread 
on qemu-devel about it).

How did you arrive at 0x33 btw?  IIRC, Brad stopped at 0x20 as he saw no 
performance improvement beyond it.  If KVM needs at least 0x33 then 
provided there is no negative impact to QEMU I don't really see a 
problem with it.

kqemu is bound to have the same issue FWIW.

Regards,

Anthony Liguori

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV