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

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

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