| Message ID | 4A310C58.6050301@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jan Kiszka wrote: > Beth Kon wrote: > >> Building latest git with ./configure --disable-kvm breaks with errors in >> pcspk.c >> > > With latest git, things break much earlier in case your host does not > provide linux/kvm.h because libkvm-all.h includes it unconditionally. > > But qemu-kvm carries linux/kvm.h, so this never happens?
Avi Kivity wrote: > Jan Kiszka wrote: >> Beth Kon wrote: >> >>> Building latest git with ./configure --disable-kvm breaks with errors in >>> pcspk.c >>> >> >> With latest git, things break much earlier in case your host does not >> provide linux/kvm.h because libkvm-all.h includes it unconditionally. >> >> > > But qemu-kvm carries linux/kvm.h, so this never happens? 1. qemu-kvm does not use its own headers when you specify --disble-kvm. That's the technical reason for this bug. 2. Upstream does not, and it's unclear if it ever will (if we push recent headers into kvm-kmod, I think there is no urgent need anymore). At least for code to-be-pushed upstream, we must not rely in this anyway. Jan
Jan Kiszka wrote: >>>> Building latest git with ./configure --disable-kvm breaks with errors in >>>> pcspk.c >>>> >>>> >>> With latest git, things break much earlier in case your host does not >>> provide linux/kvm.h because libkvm-all.h includes it unconditionally. >>> >>> >>> >> But qemu-kvm carries linux/kvm.h, so this never happens? >> > > 1. qemu-kvm does not use its own headers when you specify --disble-kvm. > That's the technical reason for this bug. > Ah, okay. This should be fixed (by including the headers) as long as we continue to carry kvm.h. > 2. Upstream does not, and it's unclear if it ever will (if we push > recent headers into kvm-kmod, I think there is no urgent need > anymore). At least for code to-be-pushed upstream, we must not > rely in this anyway. Yes. Adding the headers to kvm-kmod.h is the right thing technically, but something tells me we'll get a lot of failures by people compiling first and installing later, rather than the sequence needed to make things work: compile and install kvm-kmod, compile and install qemu[-kvm]. Not all of the failures will be visible at compile time.
Avi Kivity wrote: > Jan Kiszka wrote: >>>>> Building latest git with ./configure --disable-kvm breaks with >>>>> errors in >>>>> pcspk.c >>>>> >>>> With latest git, things break much earlier in case your host does not >>>> provide linux/kvm.h because libkvm-all.h includes it unconditionally. >>>> >>>> >>> But qemu-kvm carries linux/kvm.h, so this never happens? >>> >> >> 1. qemu-kvm does not use its own headers when you specify --disble-kvm. >> That's the technical reason for this bug. >> > > Ah, okay. This should be fixed (by including the headers) as long as we > continue to carry kvm.h. > >> 2. Upstream does not, and it's unclear if it ever will (if we push >> recent headers into kvm-kmod, I think there is no urgent need >> anymore). At least for code to-be-pushed upstream, we must not >> rely in this anyway. > > Yes. > > Adding the headers to kvm-kmod.h is the right thing technically, but > something tells me we'll get a lot of failures by people compiling first > and installing later, rather than the sequence needed to make things > work: compile and install kvm-kmod, compile and install qemu[-kvm]. Not > all of the failures will be visible at compile time. > That could (and probably should - independent of in-tree headers) be caught by making all KVM_CAPs mandatory, ie. check for the latest and greatest ones during configure and drop all the #ifdefs from the code. Whatever the strategy will be, it should be one with the clear goal to converge over the same approach with upstream. Jan
Jan Kiszka wrote: >>> 2. Upstream does not, and it's unclear if it ever will (if we push >>> recent headers into kvm-kmod, I think there is no urgent need >>> anymore). At least for code to-be-pushed upstream, we must not >>> rely in this anyway. >>> >> Yes. >> >> Adding the headers to kvm-kmod.h is the right thing technically, but >> something tells me we'll get a lot of failures by people compiling first >> and installing later, rather than the sequence needed to make things >> work: compile and install kvm-kmod, compile and install qemu[-kvm]. Not >> all of the failures will be visible at compile time. >> >> > > That could (and probably should - independent of in-tree headers) be > caught by making all KVM_CAPs mandatory, ie. check for the latest and > greatest ones during configure and drop all the #ifdefs from the code. > Not with out-of-tree headers. qemu-kvm-0.10.x ought to build against Linux 2.6.27, kvm-kmod-2.6.30, and kvm-91. Making all KVM_CAPs mandatory only works if we carry the headers with qemu. > Whatever the strategy will be, it should be one with the clear goal to > converge over the same approach with upstream. > Definitely. In this case I'm still not sure what we want, though.
Avi Kivity wrote: > Jan Kiszka wrote: >>>> 2. Upstream does not, and it's unclear if it ever will (if we push >>>> recent headers into kvm-kmod, I think there is no urgent need >>>> anymore). At least for code to-be-pushed upstream, we must not >>>> rely in this anyway. >>>> >>> Yes. >>> >>> Adding the headers to kvm-kmod.h is the right thing technically, but >>> something tells me we'll get a lot of failures by people compiling first >>> and installing later, rather than the sequence needed to make things >>> work: compile and install kvm-kmod, compile and install qemu[-kvm]. Not >>> all of the failures will be visible at compile time. >>> >>> >> >> That could (and probably should - independent of in-tree headers) be >> caught by making all KVM_CAPs mandatory, ie. check for the latest and >> greatest ones during configure and drop all the #ifdefs from the code. >> > > Not with out-of-tree headers. qemu-kvm-0.10.x ought to build against > Linux 2.6.27, kvm-kmod-2.6.30, and kvm-91. > > Making all KVM_CAPs mandatory only works if we carry the headers with qemu. If we continue to carry our own headers, the #ifdefs are pointless. If we don't, the configure checks should issue an overview on all the features that will be missing and give a hint how to resolve this (kvm-kmod...). > >> Whatever the strategy will be, it should be one with the clear goal to >> converge over the same approach with upstream. >> > > Definitely. In this case I'm still not sure what we want, though. > Maybe a good topic for next Tuesday. Jan
Jan Kiszka wrote: >>> That could (and probably should - independent of in-tree headers) be >>> caught by making all KVM_CAPs mandatory, ie. check for the latest and >>> greatest ones during configure and drop all the #ifdefs from the code. >>> >>> >> Not with out-of-tree headers. qemu-kvm-0.10.x ought to build against >> Linux 2.6.27, kvm-kmod-2.6.30, and kvm-91. >> >> Making all KVM_CAPs mandatory only works if we carry the headers with qemu. >> > > If we continue to carry our own headers, the #ifdefs are pointless. Yes. > If > we don't, the configure checks should issue an overview on all the > features that will be missing and give a hint how to resolve this > (kvm-kmod...). > Some features are optional (and we try to make most features optional so as not to force users to upgrade).
diff --git a/hw/pcspk.c b/hw/pcspk.c index 9e1b59a..5b624d1 100644 --- a/hw/pcspk.c +++ b/hw/pcspk.c @@ -51,10 +51,9 @@ static const char *s_spk = "pcspk"; static PCSpkState pcspk_state; #ifdef USE_KVM_PIT -static void kvm_get_pit_ch2(PITState *pit, - struct kvm_pit_state *inkernel_state) +static void kvm_get_pit_ch2(PITState *pit, KVMPITState *inkernel_state) { - struct kvm_pit_state pit_state; + KVMPITState pit_state; if (kvm_enabled() && qemu_kvm_pit_in_kernel()) { kvm_get_pit(kvm_context, &pit_state); @@ -68,8 +67,7 @@ static void kvm_get_pit_ch2(PITState *pit, } } -static void kvm_set_pit_ch2(PITState *pit, - struct kvm_pit_state *inkernel_state) +static void kvm_set_pit_ch2(PITState *pit, KVMPITState *inkernel_state) { if (kvm_enabled() && qemu_kvm_pit_in_kernel()) { inkernel_state->channels[2].mode = pit->channels[2].mode; @@ -82,9 +80,9 @@ static void kvm_set_pit_ch2(PITState *pit, } #else static inline void kvm_get_pit_ch2(PITState *pit, - kvm_pit_state *inkernel_state) { } + KVMPITState *inkernel_state) { } static inline void kvm_set_pit_ch2(PITState *pit, - kvm_pit_state *inkernel_state) { } + KVMPITState *inkernel_state) { } #endif static inline void generate_samples(PCSpkState *s) @@ -168,7 +166,7 @@ static uint32_t pcspk_ioport_read(void *opaque, uint32_t addr) static void pcspk_ioport_write(void *opaque, uint32_t addr, uint32_t val) { - struct kvm_pit_state inkernel_state; + KVMPITState inkernel_state; PCSpkState *s = opaque; const int gate = val & 1;