[10/11] config: Add header file for device config options

Submitted by Alexander Graf on Nov. 19, 2010, 2:56 a.m.

Details

Message ID 1290135413-21462-11-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

Alexander Graf Nov. 19, 2010, 2:56 a.m.
So far we have C preprocessor defines for target and host config
options, but we're lacking any information on which devices are
available.

We do need that information at times though, for example in the
ahci patch where we need to call a legacy init function depending
on whether we have support compiled in or not.

So this patch makes all config-devices options available as header
file and includes it by default.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 Makefile.target |    5 ++++-
 config.h        |    1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

Comments

Blue Swirl Nov. 21, 2010, 12:37 p.m.
On Fri, Nov 19, 2010 at 2:56 AM, Alexander Graf <agraf@suse.de> wrote:
> So far we have C preprocessor defines for target and host config
> options, but we're lacking any information on which devices are
> available.
>
> We do need that information at times though, for example in the
> ahci patch where we need to call a legacy init function depending
> on whether we have support compiled in or not.

That does not seem right. Devices should not care about what other
devices may exist. Perhaps stub style approach would be better.
Alexander Graf Nov. 21, 2010, 12:45 p.m.
On 21.11.2010, at 13:37, Blue Swirl wrote:

> On Fri, Nov 19, 2010 at 2:56 AM, Alexander Graf <agraf@suse.de> wrote:
>> So far we have C preprocessor defines for target and host config
>> options, but we're lacking any information on which devices are
>> available.
>> 
>> We do need that information at times though, for example in the
>> ahci patch where we need to call a legacy init function depending
>> on whether we have support compiled in or not.
> 
> That does not seem right. Devices should not care about what other
> devices may exist. Perhaps stub style approach would be better.

Well, for the -drive parameter we need to know what devices we can create and I'd like to keep that code as close as possible to the actual device code.

So the stub alternative would be to create a stub .c file for each device that could get created during -drive. I'm not sure that is a good idea :).

Another alternative would be to move the instantiation code to somewhere generic. But that sounds rather ugly to me.

Also, devices really shouldn't care about other devices' availability. Machine descriptions should care, and that's what this patch is there for :).


Alex
Blue Swirl Nov. 21, 2010, 12:56 p.m.
On Sun, Nov 21, 2010 at 12:45 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 21.11.2010, at 13:37, Blue Swirl wrote:
>
>> On Fri, Nov 19, 2010 at 2:56 AM, Alexander Graf <agraf@suse.de> wrote:
>>> So far we have C preprocessor defines for target and host config
>>> options, but we're lacking any information on which devices are
>>> available.
>>>
>>> We do need that information at times though, for example in the
>>> ahci patch where we need to call a legacy init function depending
>>> on whether we have support compiled in or not.
>>
>> That does not seem right. Devices should not care about what other
>> devices may exist. Perhaps stub style approach would be better.
>
> Well, for the -drive parameter we need to know what devices we can create and I'd like to keep that code as close as possible to the actual device code.
>
> So the stub alternative would be to create a stub .c file for each device that could get created during -drive. I'm not sure that is a good idea :).
>
> Another alternative would be to move the instantiation code to somewhere generic. But that sounds rather ugly to me.
>
> Also, devices really shouldn't care about other devices' availability. Machine descriptions should care, and that's what this patch is there for :).

Then only machine descriptions should include config-devices.h.
Markus Armbruster Dec. 10, 2010, 12:37 p.m.
Alexander Graf <agraf@suse.de> writes:

> On 21.11.2010, at 13:37, Blue Swirl wrote:
>
>> On Fri, Nov 19, 2010 at 2:56 AM, Alexander Graf <agraf@suse.de> wrote:
>>> So far we have C preprocessor defines for target and host config
>>> options, but we're lacking any information on which devices are
>>> available.
>>> 
>>> We do need that information at times though, for example in the
>>> ahci patch where we need to call a legacy init function depending
>>> on whether we have support compiled in or not.
>> 
>> That does not seem right. Devices should not care about what other
>> devices may exist. Perhaps stub style approach would be better.
>
> Well, for the -drive parameter we need to know what devices we can create and I'd like to keep that code as close as possible to the actual device code.

Stupid question: why can't we just try to create, then check status to
figure out whether it failed because the device isn't available?

[...]
Alexander Graf Dec. 10, 2010, 12:45 p.m.
On 10.12.2010, at 13:37, Markus Armbruster wrote:

> Alexander Graf <agraf@suse.de> writes:
> 
>> On 21.11.2010, at 13:37, Blue Swirl wrote:
>> 
>>> On Fri, Nov 19, 2010 at 2:56 AM, Alexander Graf <agraf@suse.de> wrote:
>>>> So far we have C preprocessor defines for target and host config
>>>> options, but we're lacking any information on which devices are
>>>> available.
>>>> 
>>>> We do need that information at times though, for example in the
>>>> ahci patch where we need to call a legacy init function depending
>>>> on whether we have support compiled in or not.
>>> 
>>> That does not seem right. Devices should not care about what other
>>> devices may exist. Perhaps stub style approach would be better.
>> 
>> Well, for the -drive parameter we need to know what devices we can create and I'd like to keep that code as close as possible to the actual device code.
> 
> Stupid question: why can't we just try to create, then check status to
> figure out whether it failed because the device isn't available?

That's what the later version of this patch that in between is ripped out of the patch set did :)


Alex

Patch hide | download patch | download mbox

diff --git a/Makefile.target b/Makefile.target
index 91e6e74..35862fd 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -1,6 +1,6 @@ 
 # -*- Mode: makefile -*-
 
-GENERATED_HEADERS = config-target.h
+GENERATED_HEADERS = config-target.h config-devices.h
 CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 
 include ../config-host.mak
@@ -40,6 +40,9 @@  kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS)
 config-target.h: config-target.h-timestamp
 config-target.h-timestamp: config-target.mak
 
+config-devices.h: config-target.h-timestamp
+config-devices.h-timestamp: config-target.mak
+
 all: $(PROGS)
 
 # Dummy command so that make thinks it has done something
diff --git a/config.h b/config.h
index e20f786..080907f 100644
--- a/config.h
+++ b/config.h
@@ -1,2 +1,3 @@ 
 #include "config-host.h"
 #include "config-target.h"
+#include "config-devices.h"