Re: [PATCH] Support for QEMU's CVS

Anthony Liguori
2006-12-21T16:20:35Z

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