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

login
register
mail settings
Submitter Alexander Graf
Date Nov. 19, 2010, 2:56 a.m.
Message ID <1290135413-21462-11-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/72188/
State New
Headers show

Comments

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

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"