Re: [Qemu-devel] [PATCH 2/2] machine opts framework

[Anthony Liguori]

On 03/24/2010 03:58 PM, Glauber Costa wrote:
> On Wed, Mar 24, 2010 at 02:43:35PM -0500, Anthony Liguori wrote:
>    
> > On 03/24/2010 02:26 PM, Glauber Costa wrote:
> >      
> > > This patch adds initial support for the -machine option, that allows
> > > command line specification of machine attributes (always relying on safe
> > > defaults). Besides its value per-se, it is the saner way we found to
> > > allow for enabling/disabling of kvm's in-kernel irqchip.
> > >
> > > A machine with in-kernel-irqchip could be specified as:
> > >         -machine irqchip=apic-kvm
> > > And one without it:
> > >         -machine irqchip=apic
> > >
> > > To demonstrate how it'd work, this patch introduces a choice between
> > > "pic" and "apic", pic being the old-style isa thing.
> > >        
> > I started from a different place.  See machine-qemuopts in my staging tree.
> >
> > I think we should combine efforts.
> >
> > Regards,
> >
> >      
> Absolutely. I see little overlap between what we did. Just a comment on yours:
>
> -static void an5206_init(ram_addr_t ram_size,
> -                     const char *boot_device,
> -                     const char *kernel_filename, const char *kernel_cmdline,
> -                     const char *initrd_filename, const char *cpu_model)
> +static void an5206_init(QemuOpts *opts)
>   {
>
> Since we're changing init functions anyway, I believe we should also pass
> a pointer to the machine structure. With that, we can avoing using the global
> current_machine.
>    

Yes, I had the same thought.  For instance, with isa-pc is just pc_init 
with an extra parameter.  If we had a structure like:

typedef struct QEMUPCMachine
{
   QEMUMachine parent;
   int pci_enabled;
} QEMUPCMachine;

Then you wouldn't need those dispatch functions.  Not a huge win for x86 
but for sparc and some arm boards, it's pretty significant.

Regards,

Anthony Liguori