* [Qemu-devel] QEMU configuration files @ 2008-06-18 18:12 Fabrice Bellard 2008-06-18 19:16 ` [Qemu-devel] " Sebastian Herbszt ` (5 more replies) 0 siblings, 6 replies; 14+ messages in thread From: Fabrice Bellard @ 2008-06-18 18:12 UTC (permalink / raw) To: qemu-devel Hi, My snapshot of the "object based" QEMU configuration system can be found at http://bellard.org/qemu/patches . I only tried it for x86 targets. It is not yet in committable state and comments are welcome ! General ideas: - User preferences and machine definitions are separated. User preferences are in ~/.qemu/config for Unix systems. Machine definitions can override user preferences but I believe it should be the exception. - Command line options override the user preferences and machine definitions. - Machine definitions contain machine parameters and device definitions. Device definitions are used to create new devices not instanciated in the hardcoded machine definition such as PCI and USB devices. There are many details which need clarification, in particular: - PCI, IDE, SCSI and buses naming. It is important if we want to be able to dynamically instantiate complicated bus topologies. - USB port naming - Is it worth specifying board specific network controllers as separate devices (I tried to do that for smc91c111 devices) ? A simpler solution would be to add new machine parameters to do that. - It would be logical to define QEMUDevice for every instanciated device and that register_savevm() use QEMUDevice as parameter, but it requires more changes in the code. - Is it worth handling class defaults parameters ? I find the current implementation too complicated. Fabrice. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: QEMU configuration files 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard @ 2008-06-18 19:16 ` Sebastian Herbszt 2008-06-18 21:08 ` [Qemu-devel] " Anthony Liguori ` (4 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Sebastian Herbszt @ 2008-06-18 19:16 UTC (permalink / raw) To: qemu-devel Fabrice, > My snapshot of the "object based" QEMU configuration system can be found > at http://bellard.org/qemu/patches . I only tried it for x86 targets. It > is not yet in committable state and comments are welcome ! the patch is pretty huge: Makefile.target | 2 device.c | 874 +++++++++++++++++++ device.h | 99 ++ hw/gumstix.c | 10 hw/hw.h | 3 hw/integratorcp.c | 16 hw/mainstone.c | 4 hw/mips_jazz.c | 30 hw/mips_malta.c | 30 hw/mips_r4k.c | 15 hw/pc.c | 77 - hw/ppc_chrp.c | 8 hw/ppc_oldworld.c | 8 hw/ppc_prep.c | 54 - hw/realview.c | 14 hw/sun4u.c | 7 hw/versatilepb.c | 20 vl.c | 2394 +++++++++++++++++++++++++++++++++++++----------------- 18 files changed, 2676 insertions(+), 989 deletions(-) Any chance you could split it into smaller pieces? - Sebastian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard 2008-06-18 19:16 ` [Qemu-devel] " Sebastian Herbszt @ 2008-06-18 21:08 ` Anthony Liguori 2008-06-18 23:49 ` Paul Brook ` (3 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Anthony Liguori @ 2008-06-18 21:08 UTC (permalink / raw) To: qemu-devel Fabrice Bellard wrote: > Hi, > > My snapshot of the "object based" QEMU configuration system can be > found at http://bellard.org/qemu/patches . I only tried it for x86 > targets. It is not yet in committable state and comments are welcome ! I've only done a very quick high level review. I'll try to do a more thorough one soon. My first impressions are very positive. I think it would be better to decentralize class registration but I don't think that's a very hard change to make. > General ideas: > > - User preferences and machine definitions are separated. User > preferences are in ~/.qemu/config for Unix systems. Machine > definitions can override user preferences but I believe it should be > the exception. Ack. I think this is pretty important. > - Command line options override the user preferences and machine > definitions. > > - Machine definitions contain machine parameters and device > definitions. Device definitions are used to create new devices not > instanciated in the hardcoded machine definition such as PCI and USB > devices. > > There are many details which need clarification, in particular: > > - PCI, IDE, SCSI and buses naming. It is important if we want to be > able to dynamically instantiate complicated bus topologies. > - USB port naming > - Is it worth specifying board specific network controllers as > separate devices (I tried to do that for smc91c111 devices) ? A > simpler solution would be to add new machine parameters to do that. > - It would be logical to define QEMUDevice for every instanciated > device and that register_savevm() use QEMUDevice as parameter, but it > requires more changes in the code. Indeed. QEMUDevice is a good abstraction as it can be also be used to introduce per-device locking which will help parallelize things with KVM. Regards, Anthony Liguori > - Is it worth handling class defaults parameters ? I find the current > implementation too complicated. > > Fabrice. > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard 2008-06-18 19:16 ` [Qemu-devel] " Sebastian Herbszt 2008-06-18 21:08 ` [Qemu-devel] " Anthony Liguori @ 2008-06-18 23:49 ` Paul Brook 2008-06-19 0:01 ` Paul Brook 2008-06-19 9:46 ` Fabrice Bellard 2008-06-19 11:40 ` Daniel P. Berrange ` (2 subsequent siblings) 5 siblings, 2 replies; 14+ messages in thread From: Paul Brook @ 2008-06-18 23:49 UTC (permalink / raw) To: qemu-devel > - Machine definitions contain machine parameters and device definitions. > Device definitions are used to create new devices not instanciated in > the hardcoded machine definition such as PCI and USB devices. >... > - Is it worth specifying board specific network controllers as separate > devices (I tried to do that for smc91c111 devices) ? A simpler solution > would be to add new machine parameters to do that. IMHO the goal should be for most (maybe all) devices to be explicitly instantiated, not just the easy USB/PCI ones. That may require additional work to specify things like IRQ routing and memory ranges. For example there's no real reason why qemu needs to know about most of the the ARM boards I've added (e.g. integrator, versatile, realview and stellaris boards). They all use approximately the same set of modular components, just in connected in different combinations. I suspect the same is true of many of the pxa boards, and proably many ColdFire devices. This is maybe slightly different between emulating embedded boards and workstation/server class machines. For workstation/server emulation it's entirely reasonable to arbitrarily add/remove serial and network devices from a "PC" machine based on the user config. For embedded SoC devices you tend to want to emulate a fixed number of devices (matching those present on real hardware) and leave them present but disconnected if the user does not specify appropriate net/serial options. On case where we currently do this is CDROMs. Omitting -cdrom does not remove the cdrom from the machine, it just means an "empty" cdrom drive is created. Paul ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-18 23:49 ` Paul Brook @ 2008-06-19 0:01 ` Paul Brook 2008-06-19 9:46 ` Fabrice Bellard 1 sibling, 0 replies; 14+ messages in thread From: Paul Brook @ 2008-06-19 0:01 UTC (permalink / raw) To: qemu-devel [I apparently missed out a paragraph from my first mail, so the paragraph quoted below didn't make a whole lot of sense.] One issue is whether/how much it's desirable to decouple the emulated hardware configuration from the host interface configuration. e.g. should we always create N serial ports, regardless of how many -serial options are specified. > This is maybe slightly different between emulating embedded boards and > workstation/server class machines. For workstation/server emulation it's > entirely reasonable to arbitrarily add/remove serial and network devices > from a "PC" machine based on the user config. For embedded SoC devices you > tend to want to emulate a fixed number of devices (matching those present > on real hardware) and leave them present but disconnected if the user does > not specify appropriate net/serial options. Paul ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-18 23:49 ` Paul Brook 2008-06-19 0:01 ` Paul Brook @ 2008-06-19 9:46 ` Fabrice Bellard 1 sibling, 0 replies; 14+ messages in thread From: Fabrice Bellard @ 2008-06-19 9:46 UTC (permalink / raw) To: Paul Brook; +Cc: qemu-devel Paul Brook wrote: >> - Machine definitions contain machine parameters and device definitions. >> Device definitions are used to create new devices not instanciated in >> the hardcoded machine definition such as PCI and USB devices. >> ... >> - Is it worth specifying board specific network controllers as separate >> devices (I tried to do that for smc91c111 devices) ? A simpler solution >> would be to add new machine parameters to do that. > > IMHO the goal should be for most (maybe all) devices to be explicitly > instantiated, not just the easy USB/PCI ones. That may require additional > work to specify things like IRQ routing and memory ranges. Good. So you suggest that I go on transforming the remaining network devices into "generic" devices. In order to keep the compatibility with the current command line options, it will be necessary to add a default network adapter name in each machine description [it would be simpler to avoid being compatible with the current command line options, but I believe most people are against that]. > For example there's no real reason why qemu needs to know about most of the > the ARM boards I've added (e.g. integrator, versatile, realview and stellaris > boards). They all use approximately the same set of modular components, just > in connected in different combinations. I suspect the same is true of many > of the pxa boards, and proably many ColdFire devices. If suitable naming for the buses are defined and if the appropriate device classes are created, I believe the current system can be used to describe a complete machine. >> One issue is whether/how much it's desirable to decouple the emulated hardware >> configuration from the host interface configuration. e.g. should we always >> create N serial ports, regardless of how many -serial options are specified. > This is maybe slightly different between emulating embedded boards and > workstation/server class machines. For workstation/server emulation it's > entirely reasonable to arbitrarily add/remove serial and network devices from > a "PC" machine based on the user config. For embedded SoC devices you tend to > want to emulate a fixed number of devices (matching those present on real > hardware) and leave them present but disconnected if the user does not > specify appropriate net/serial options. > > On case where we currently do this is CDROMs. Omitting -cdrom does not remove > the cdrom from the machine, it just means an "empty" cdrom drive is created. My current solution is to keep the machine.serialN and machine.parallelN parameters to avoid changing too much code in the C machine definitions. The user can still disable these implicit devices and explicitly instantiate serial or parallel ports, provided there are generic devices classes for them (which is currently not the case). It could be possible to do something similar to the network and drives devices: - the command line interface is still backward compatible (i.e. each C machine description gives the necessary information to instantiate default serial or parallel ports) ; - when using configuration files, no serial or parallel ports are instantiated by default and the user must explicitly declare them. Fabrice. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard ` (2 preceding siblings ...) 2008-06-18 23:49 ` Paul Brook @ 2008-06-19 11:40 ` Daniel P. Berrange 2008-06-19 11:52 ` Fabrice Bellard 2008-06-19 11:56 ` [Qemu-devel] " Sebastian Herbszt 2008-06-20 13:11 ` Fabrice Bellard 2008-06-24 15:50 ` [Qemu-devel] " Ian Jackson 5 siblings, 2 replies; 14+ messages in thread From: Daniel P. Berrange @ 2008-06-19 11:40 UTC (permalink / raw) To: qemu-devel On Wed, Jun 18, 2008 at 08:12:52PM +0200, Fabrice Bellard wrote: > Hi, > > My snapshot of the "object based" QEMU configuration system can be found > at http://bellard.org/qemu/patches . I only tried it for x86 targets. It > is not yet in committable state and comments are welcome ! > > General ideas: > > - User preferences and machine definitions are separated. User > preferences are in ~/.qemu/config for Unix systems. Machine definitions > can override user preferences but I believe it should be the exception. I'd like the ability to explicitly not use any user preferences at all, rather than having to override each individual setting. This would make it easier for me to invoke QEMU from libvirt with a predictable configuration that I can guarnetee to be identical on any host regardless of how a user my have their preferences setup. It could also be useful to be able to specify a alternative preferences file instead of the default $HOME/.qemu/config Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-19 11:40 ` Daniel P. Berrange @ 2008-06-19 11:52 ` Fabrice Bellard 2008-06-19 14:36 ` Daniel P. Berrange 2008-06-19 11:56 ` [Qemu-devel] " Sebastian Herbszt 1 sibling, 1 reply; 14+ messages in thread From: Fabrice Bellard @ 2008-06-19 11:52 UTC (permalink / raw) To: Daniel P. Berrange, qemu-devel Daniel P. Berrange wrote: > On Wed, Jun 18, 2008 at 08:12:52PM +0200, Fabrice Bellard wrote: >> Hi, >> >> My snapshot of the "object based" QEMU configuration system can be found >> at http://bellard.org/qemu/patches . I only tried it for x86 targets. It >> is not yet in committable state and comments are welcome ! >> >> General ideas: >> >> - User preferences and machine definitions are separated. User >> preferences are in ~/.qemu/config for Unix systems. Machine definitions >> can override user preferences but I believe it should be the exception. > > I'd like the ability to explicitly not use any user preferences at > all, rather than having to override each individual setting. This > would make it easier for me to invoke QEMU from libvirt with a predictable > configuration that I can guarnetee to be identical on any host regardless > of how a user my have their preferences setup. OK. > It could also be useful to be able to specify a alternative preferences > file instead of the default $HOME/.qemu/config Right. I plan to reserve '-f' for that and to use another option (or none depending on the file extension) to specify the machine description. Fabrice. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-19 11:52 ` Fabrice Bellard @ 2008-06-19 14:36 ` Daniel P. Berrange 0 siblings, 0 replies; 14+ messages in thread From: Daniel P. Berrange @ 2008-06-19 14:36 UTC (permalink / raw) To: Fabrice Bellard; +Cc: qemu-devel On Thu, Jun 19, 2008 at 01:52:03PM +0200, Fabrice Bellard wrote: > Daniel P. Berrange wrote: > >On Wed, Jun 18, 2008 at 08:12:52PM +0200, Fabrice Bellard wrote: > >>Hi, > >> > >>My snapshot of the "object based" QEMU configuration system can be found > >>at http://bellard.org/qemu/patches . I only tried it for x86 targets. It > >>is not yet in committable state and comments are welcome ! > >> > >>General ideas: > >> > >>- User preferences and machine definitions are separated. User > >>preferences are in ~/.qemu/config for Unix systems. Machine definitions > >>can override user preferences but I believe it should be the exception. > > > >I'd like the ability to explicitly not use any user preferences at > >all, rather than having to override each individual setting. This > >would make it easier for me to invoke QEMU from libvirt with a predictable > >configuration that I can guarnetee to be identical on any host regardless > >of how a user my have their preferences setup. > > OK. > > >It could also be useful to be able to specify a alternative preferences > >file instead of the default $HOME/.qemu/config > > Right. I plan to reserve '-f' for that and to use another option (or > none depending on the file extension) to specify the machine description. AH, with that I could use 'qemu -f /dev/zero' to stop the user preferences being loaded I imagine. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: QEMU configuration files 2008-06-19 11:40 ` Daniel P. Berrange 2008-06-19 11:52 ` Fabrice Bellard @ 2008-06-19 11:56 ` Sebastian Herbszt 1 sibling, 0 replies; 14+ messages in thread From: Sebastian Herbszt @ 2008-06-19 11:56 UTC (permalink / raw) To: Daniel P. Berrange, qemu-devel From: "Daniel P. Berrange" > It could also be useful to be able to specify a alternative preferences > file instead of the default $HOME/.qemu/config I think it's possible with the new "-f" option: + case QEMU_OPTION_f: + if (qemu_parse_config(&machine_dev, optarg, + QEMU_CONF_ERR_FILE) < 0) + exit(1); + has_config = 1; + break; - Sebastian ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: QEMU configuration files 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard ` (3 preceding siblings ...) 2008-06-19 11:40 ` Daniel P. Berrange @ 2008-06-20 13:11 ` Fabrice Bellard 2008-06-21 8:43 ` Blue Swirl 2008-06-24 15:50 ` [Qemu-devel] " Ian Jackson 5 siblings, 1 reply; 14+ messages in thread From: Fabrice Bellard @ 2008-06-20 13:11 UTC (permalink / raw) To: qemu-devel Hi, I have just updated the config file patch at http://bellard.org/qemu/patches . Now all targets are supported. There is a small documentation in config.txt. Fabrice. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] Re: QEMU configuration files 2008-06-20 13:11 ` Fabrice Bellard @ 2008-06-21 8:43 ` Blue Swirl 0 siblings, 0 replies; 14+ messages in thread From: Blue Swirl @ 2008-06-21 8:43 UTC (permalink / raw) To: qemu-devel On 6/20/08, Fabrice Bellard <fabrice@bellard.org> wrote: > Hi, > > I have just updated the config file patch at > http://bellard.org/qemu/patches . Now all targets are supported. There is a > small documentation in config.txt. Seems to work for Sparc. About buses, maybe the config file syntax used in TME gives some ideas: board0: tme/machine/sun4 name Calvin cpu0 at board0: tme/ic/cy7c601 fpu-type tms390-c602A fpu-compliance partial fpu-incomplete trap sbus0: tme/generic/bus size 4GB slot-addr 0xf8000000 slot-size 32MB slot 0 slot 1 slot 2 slot 3 sbus0 controller at board0 sbus alias mainbus0 sbus0 ram0 at mainbus0 addr 0x0: tme/host/posix/memory ram 16MB rom0 at mainbus0 addr 0xf6000000: tme/host/posix/memory rom sun4-75-rev-2.9.bin zs0 at mainbus0 addr 0xf1000000 ipl 12: tme/machine/sun4/zs cgthree0 at sbus0 slot 1 offset 0x0 ipl 7: tme/bus/sbus/cgthree cgthreerom0 at sbus0 slot 1 offset 0x0: tme/host/posix/memory rom "SUNW,501-1415.bin" Also CPUs, ROMs and RAMs could be handled to complement the CPU feature system. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard ` (4 preceding siblings ...) 2008-06-20 13:11 ` Fabrice Bellard @ 2008-06-24 15:50 ` Ian Jackson 2008-06-24 19:50 ` Jamie Lokier 5 siblings, 1 reply; 14+ messages in thread From: Ian Jackson @ 2008-06-24 15:50 UTC (permalink / raw) To: qemu-devel Fabrice Bellard writes ("[Qemu-devel] QEMU configuration files"): > My snapshot of the "object based" QEMU configuration system can be found > at http://bellard.org/qemu/patches . I only tried it for x86 targets. It > is not yet in committable state and comments are welcome ! This looks like good stuff. I do have some comments on the structure, though. Sorry that these come so late but as you can see I wanted to do a thorough job. If you like some of these ideas I'd be happy to provide a concrete patch. So: I think it would be better to have a declarative static data structure describing the options, than to have the data structure which describes the options constructed dynamically. This generally involves less code in the end, and also makes it much easier to do some of the other things I'm about to suggest. So for example: + void isa_ne2000_class_init(void) + { + static const QEMUDeviceField fields[]= { + { "iobase", QEMU_CTYPE_INT }, + { "irq", QEMU_CTYPE_STR }, + { 0 } /* sentinel */ + }; + static const QEMUDeviceClass class= { + "ne2k_isa", "NE2000 ISA", fields, + .dev_init = isa_ne2000_dev_init + }; + qemu_class_register(&class); + } This is the way the block backend drivers generally already work. In the rare cases where it is actually necessary to vary the details at runtime, it is of course still possible to allocate these structures with malloc and fill them in. My suggestion is that making the structure transparent will avoid one level of tail-chasing when figuring out what's going on, and make it easier to push functionality out of the individual driver models etc. into the common option parsing code. Your version defers checking of the supplied values until the point of use. For example, the values of "cyls" etc. for disks are range-checked in drive_init. This is less than ideal; generally it's better to check when the value is first read (from the command line or config file). This can greatly improve the error reporting, and can avoid going through a lot of needless startup work (which may even need to be undone) if the invocation is doomed. So I would generally suggest providing the generic parsing code with not just a type, but also enough information to rangecheck or fully syntax check the provided value. In the general case this would be a function pointer which would use some values (such as the minimum and maximum values, or other information) which were specified in the same place as the option. So I would suggest something like this, instead: + void qemu_fieldcheck_int(const QEMUDeviceField*, + const char *value); + + void drive_class_register(void) + { + static const QEMUDeviceField fields[]= { + ... + { "cyls", qemu_fieldcheck_int, .min=1, .max=16383 } + ... + { 0 } /* sentinel */ Finally, your arrangements mean that each user of a configuration option to has to call qemu_device_get_* and then store the value of the option in its own data structure. This is because the values are stored, as parsed, indexed by the option name as a string. I think that call is an unnecessary and unhelpful indirection; it means we have to write a lot of boilerplate but doesn't really buy us anything. I would suggest instead that we have the option parser store the option values directly into the class's struct, at parse time. We can do this by supplying the parser, in the option table, with the offset of the actual field in the class specific struct: + typedef struct NE2000State { + ... + int irq_option, iobase_option; + ... + } NE2000State; + + void isa_ne2000_class_init(void) + { + static const QEMUDeviceField fields[]= { + { "iobase", offsetof(NE2000State,iobase_option), + qemu_store_int, .default_int=0, max=INT_MAX }, + { "irq", offsetof(NE2000State,irq_option), + qemu_store_irq, .default_int=0 } + { 0 } /* sentinel */ This means that at the point of use we can completely eliminate code like this, because the functionality is in qemu_store_int: - cyls = qemu_device_get_int(dev, "cyls"); - ... - if (cyls || heads || secs) { - if (cyls < 1 || cyls > 16383) { - fprintf(stderr, "qemu: '%d' invalid physical cyls number\n", cyls); - return -1; - } Doing this also makes it easy to provide field name aliases: we just add a duplicate entry to the table, pointing at the same option struct member. Field name aliases will be needed for backward compatibility the first time we want to change a field name, and can be useful for brevity. This is somewhat clumsy but is much better with some use of macros: + void isa_ne2000_class_init(void) + { + typedef NE2000State ClassState; + static const QEMUDeviceField fields[]= { + { FIELD_INT(iobase, 0), .max=INT_MAX }, + { FIELD_INT(irq, 0) }, + { 0 } /* sentinel */ + }; The required macros are something like this: + #define FIELD_INT(name,default) \ + #name, \ + offsetof(ClassState, name##_option), \ + qemu_store_int, \ + .default_int = (default) + + #define FIELD_IRQ(name,default) \ + #name, \ + offsetof(ClassState, name##_option), \ + qemu_store_irq, \ + .default_int = (default) The types would look something like this: + typedef void QEMUStoreDeviceFieldFunction( + const struct QEMUDeviceField *field_info, + const char *fieldname, const char *value + ); + + extern QEMUStoreDeviceFieldFunction + qemu_store_int, qemu_store_str, qemu_store_irq; + + typedef struct QEMUDeviceField { + const char *name; + ptrdiff_t store_offset; + QEMUStoreDeviceFieldFunction *parser_function; + /* remaining fields are used by specific parsers: */ + int default_int, min, max; + const char *default_str; + } QEMUDeviceField; Regards, Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] QEMU configuration files 2008-06-24 15:50 ` [Qemu-devel] " Ian Jackson @ 2008-06-24 19:50 ` Jamie Lokier 0 siblings, 0 replies; 14+ messages in thread From: Jamie Lokier @ 2008-06-24 19:50 UTC (permalink / raw) To: qemu-devel Ian Jackson wrote: > I think it would be better to have a declarative static data structure > describing the options, than to have the data structure which > describes the options constructed dynamically. This generally > involves less code in the end, and also makes it much easier to do > some of the other things I'm about to suggest. > > So for example: > > + void isa_ne2000_class_init(void) > + { > + static const QEMUDeviceField fields[]= { > + { "iobase", QEMU_CTYPE_INT }, > + { "irq", QEMU_CTYPE_STR }, > + { 0 } /* sentinel */ > + }; > + static const QEMUDeviceClass class= { > + "ne2k_isa", "NE2000 ISA", fields, > + .dev_init = isa_ne2000_dev_init > + }; > + qemu_class_register(&class); > + } > > This is the way the block backend drivers generally already work. Yes, if it can be tidy, clear and compact. > So I would generally suggest providing the generic parsing code with > not just a type, but also enough information to rangecheck or fully > syntax check the provided value. In the general case this would be a > function pointer which would use some values (such as the minimum and > maximum values, or other information) which were specified in the same > place as the option. > > So I would suggest something like this, instead: > > + { "cyls", qemu_fieldcheck_int, .min=1, .max=16383 } Yes. > I would suggest instead that we have the option parser store the > option values directly into the class's struct, at parse time. We can > do this by supplying the parser, in the option table, with the offset > of the actual field in the class specific struct: ... > This is somewhat clumsy but is much better with some use of macros: > > + void isa_ne2000_class_init(void) > + { > + typedef NE2000State ClassState; > + static const QEMUDeviceField fields[]= { > + { FIELD_INT(iobase, 0), .max=INT_MAX }, > + { FIELD_INT(irq, 0) }, > + { 0 } /* sentinel */ > + }; Yes again. I do something very similar for options processing in some of my programs recently, and it's been nice to use. It removes a lot of duplicated boilerplate, like the switch statements and so on. I go a little further, letting FIELD_SIGNED work on any signed integer (it's got sizeof()) and FIELD_UNSIGNED, FIELD_STRING, and convenient specials like FIELD_HOSTNAME_OR_IP. It's working out nicely so far. You can also add help text to each option, if you have some pretty way to dump all the option names and their help texts. I'm finding it tidier than maintaining separate documentation strings, and easier to keep it correct. GNU argp (now in Glibc) has some similar ideas. It doesn't let the options point directly to where to store values. It's quite good for help strings and such. While we're here, it would be nice if QEMU's monitor could use this generic config code to show all the config settings, using the same names as the config file - and let you change all the settings too, when that's possible. -- Jamie ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2008-06-24 19:50 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-06-18 18:12 [Qemu-devel] QEMU configuration files Fabrice Bellard 2008-06-18 19:16 ` [Qemu-devel] " Sebastian Herbszt 2008-06-18 21:08 ` [Qemu-devel] " Anthony Liguori 2008-06-18 23:49 ` Paul Brook 2008-06-19 0:01 ` Paul Brook 2008-06-19 9:46 ` Fabrice Bellard 2008-06-19 11:40 ` Daniel P. Berrange 2008-06-19 11:52 ` Fabrice Bellard 2008-06-19 14:36 ` Daniel P. Berrange 2008-06-19 11:56 ` [Qemu-devel] " Sebastian Herbszt 2008-06-20 13:11 ` Fabrice Bellard 2008-06-21 8:43 ` Blue Swirl 2008-06-24 15:50 ` [Qemu-devel] " Ian Jackson 2008-06-24 19:50 ` Jamie Lokier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).