Re: [Qemu-devel] [PATCH V6 04/15] Introduce -accel command option.

[Anthony PERARD]

On Mon, 15 Nov 2010, Anthony PERARD wrote:

> On Mon, 15 Nov 2010, Alexander Graf wrote:
>
> >
> > On 21.10.2010, at 19:36, address@hidden wrote:
> >
> > > From: Anthony PERARD <address@hidden>
> > >
> > > This option gives the ability to switch one "accelerator" like kvm, xen
> > > or the default one tcg. We can specify more than one accelerator by
> > > separate them by a comma. QEMU will try each one and use the first whose
> > > works.
> > >
> > > So,
> > >
> > > -accel xen,kvm,tcg
> > >
> > > which would try Xen support first, then KVM and finaly tcg if none of
> > > the other works.
> > >
> > > Signed-off-by: Anthony PERARD <address@hidden>
> > > ---
> > > qemu-options.hx |   10 ++++++
> > > vl.c            |   86 
> > > ++++++++++++++++++++++++++++++++++++++++++++++++-------
> > > 2 files changed, 85 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/vl.c b/vl.c
> > > index df414ef..40a26ee 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -1750,6 +1750,74 @@ static int debugcon_parse(const char *devname)
> > >     return 0;
> > > }
> > >
> > > +static struct {
> > > +    const char *opt_name;
> > > +    const char *name;
> > > +    int (*available)(void);
> > > +    int (*init)(int smp_cpus);
> > > +    int *allowed;
> > > +} accel_list[] = {
> > > +    { "tcg", "tcg", NULL, NULL, NULL },
> > > +    { "kvm", "KVM", kvm_available, kvm_init, &kvm_allowed },
> >
> > Thinking about this a bit more ...
> >
> > kvm_available is a function pointer that gets #defined to (0) when we don't 
> > have KVM available. I can imagine some compiler might throw a warning on us 
> > for this one day.
> >
> > Is there a valid reason not to do
> >
> > static inline int kvm_enabled()
> > {
> > #ifdef CONFIG_KVM
> >     return kvm_allowed;
> > #else
> >     return 0;
> > #endif
> > }
> >
> > That should compile into the exact same code but be valid for function 
> > pointers.
>
> I will do this change, as well as for the two others patches.

Actually, kvm_available is already a function, not a define.

kvm_enable can be change from define to function, but not in this patch,
because I don't use it.

-- 
Anthony PERARD