Skip to contentSkip to conversation

qemu-kvm broken after ./configure --disable-kvm

DISCUSSION Beth Kon 4 participants 10 messages Jun 11, 2009 – Jun 23, 2009 KVM

lore
Share & tools
lore

Beth Kon

author
#0 lore raw

Building latest git with ./configure --disable-kvm breaks with errors in pcspk.c

#1 lore raw
Quoted 2 lines · Beth Kon

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.

I would like to push this task to Glauber as he is already shuffling around most of the involved code: Could you have a look on --disable-kvm too while you are at it? My basic idea would be to get rid of direct qemu-kvm.h includes so that you always obtain the required [proto]types by including kvm.h, independent of CONFIG_KVM and already prepared for upstream where there is no qemu-kvm.h.

Regarding the bugs I left behind in pcspk.c, I would suggest something like

hw/pcspk.c +6 -8
index 9e1b59a..5b624d1 100644
@@ -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;

where KVMPITState is defined as

#ifdef KVM_CAP_PIT
typedef struct kvm_pit_state KVMPITState;
#else
typedef struct { } KVMPITState;
#endif

Thanks,
Jan

#2 lore raw
Quoted 10 lines · Jan Kiszka
Quoted 3 lines · Beth Kon

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?

Signature · 1 line
error compiling committee.c: too many arguments to function
#3 lore raw
Quoted 7 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 3 lines · Beth Kon

Building latest git with ./configure --disable-kvm breaks with errors in pcspk.c

Quoted 6 lines
Quoted 4 lines

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

#4 lore raw
Quoted 9 lines · Jan Kiszka
Quoted 9 lines
Quoted 9 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 4 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#5 lore raw
Quoted 24 lines · Avi Kivity
Quoted 8 lines · Jan Kiszka
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 8 lines

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

#6 lore raw
Quoted 15 lines · Jan Kiszka
Quoted 5 lines
Quoted 5 lines

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.

Quoted 9 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#7 lore raw
Quoted 21 lines · Avi Kivity
Quoted 15 lines · Jan Kiszka
Quoted 5 lines
Quoted 5 lines

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.

Quoted 9 lines

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.

Quoted 4 lines

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.

Quoted 4 lines

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...).

Quoted 7 lines
Quoted 4 lines

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

#8 lore raw
Quoted 5 lines · Jan Kiszka
Quoted 5 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 5 lines

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.

Quoted 5 lines

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).

Signature · 1 line
error compiling committee.c: too many arguments to function
#9 lore raw
In reply to Beth Kon
Quoted 2 lines · Beth Kon

Building latest git with ./configure --disable-kvm breaks with errors in pcspk.c

I'm building from latest git, as well, but I'm not using --disable-kvm.

Still, I seem to be affected by this issue too. Am I missing a header or dependency?

...
CC mips-softmmu/pcspk.o
/tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:85:
error: expected declaration specifiers or ‘...’ before ‘kvm_pit_state’ /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:87:
error: expected declaration specifiers or ‘...’ before ‘kvm_pit_state’ /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c: In function ‘pcspk_callback’:
/tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:114:
error: too many arguments to function ‘kvm_get_pit_ch2’ /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c: In function ‘pcspk_ioport_read’:
/tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:161:
error: too many arguments to function ‘kvm_get_pit_ch2’ /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c: In function ‘pcspk_ioport_write’:
/tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:171:
error: storage size of ‘inkernel_state’ isn’t known /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:175:
error: too many arguments to function ‘kvm_get_pit_ch2’ /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:185:
error: too many arguments to function ‘kvm_set_pit_ch2’ /tmp/tmp.OdSUhAEONG/qemu-kvm-0.10.50.20090622191901/hw/pcspk.c:171:
warning: unused variable ‘inkernel_state’
make[2]: *** [pcspk.o] Error 1
make[1]: *** [subdir-mips-softmmu] Error 2
...

More complete log at:
* http://launchpadlibrarian.net/28223193/buildlog_ubuntu-karmic-amd64.qemu-kvm_0.10.50.20090622162008-0daily1_FAILEDTOBUILD.txt.gz

Thanks,
:-Dustin