[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Anthony Liguori]

On 12/15/2010 09:50 AM, Markus Armbruster wrote:
> We currently enable KVM by default, and when it's not available, we
> print a message and fall back to TCG.  Option -enable-kvm is ignored.
> Option -no-kvm suppresses KVM.
>
> Upstream works differently: KVM is off by default, -enable-kvm
> switches it on.  -enable-kvm terminates the process unsuccessfully if
> KVM is not available.
>
> upstream qemu   |  default  |-enable-kvm
> ----------------+-----------+-----------
> KVM available   | disabled  |  enabled
> KVM unavailable | disabled  |    fail
>
> qemu-kvm        |  default  |-enable-kvm
> ----------------+-----------+-----------
> KVM available   |  enabled* |  enabled
> KVM unavailable | disabled  | disabled*
>
> * differs from upstream
>
> Users of qemu and qemu-kvm need to be aware of these differences to
> enable / disable use of KVM reliably.  This is bothersome.
>
> Consider -enable-kvm when KVM is unavailable: If the user expects
> qemu-kvm behavior (fall back), but qemu fails, he'll likely be
> surprised and unhappy.  If the user expects upstream behavior (fail),
> but qemu-kvm falls back to TCG, the guest runs slow as molasses, and
> the user will likely be confused and unhappy (unless he spots and
> understands the "disable KVM" message).
>
> Switch to upstream semantics: KVM off by default, -enable-kvm switches
> it on, and when it can't, it's fatal.
>
> Having to enable KVM explicitly is annoying, but the proper place to
> address that is upstream.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>    

Backwards compatibility is going to kill us if we try to make this change.

Current qemu-kvm behavior:

default: -accel kvm,tcg
-no-kvm: -accel tcg
-enable-kvm: -accel kvm,tcg

Current upstream behavior

default: -accel tcg
-enable-kvm: -accel kvm

I think we should tie `-accel' to the machine type.  For qemu-kvm, a 
different default machine type should be used than upstream qemu (it 
really should be a configure switch).

For `pc', the default `-accel' behavior should remain 'tcg'.  For 
`kvmpc', the default `-accel' behavior should be 'kvm,tcg'.

-no-kvm should be deprecated.  -enable-kvm should also be deprecated in 
favor of the `-accel' option.

In the short term, it would be a good idea to modify qemu-kvm to switch 
the -enable-kvm semantics to match upstream (fail if KVM isn't 
available).  Adding an alias for 'kvmpc' upstream and qemu-kvm and 
making qemu-kvm default to 'kvmpc' would be helpful for management tools 
too.

Regards,

Anthony Liguori

> ---
>   vl.c |   10 +---------
>   1 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index e3c8919..87e88c2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -247,7 +247,7 @@ static void *boot_set_opaque;
>   static NotifierList exit_notifiers =
>       NOTIFIER_LIST_INITIALIZER(exit_notifiers);
>
> -int kvm_allowed = 1;
> +int kvm_allowed = 0;
>   uint32_t xen_domid;
>   enum xen_mode xen_mode = XEN_EMULATE;
>
> @@ -2436,10 +2436,8 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_smbios:
>                   do_smbios_option(optarg);
>                   break;
> -#ifdef OBSOLETE_KVM_IMPL
>               case QEMU_OPTION_enable_kvm:
>                   kvm_allowed = 1;
> -#endif
>                   break;
>   	    case QEMU_OPTION_no_kvm:
>   		kvm_allowed = 0;
> @@ -2789,18 +2787,12 @@ int main(int argc, char **argv, char **envp)
>       if (kvm_allowed) {
>           int ret = kvm_init(smp_cpus);
>           if (ret<  0) {
> -#if defined(OBSOLETE_KVM_IMPL) || defined(CONFIG_NO_CPU_EMULATION)
>               if (!kvm_available()) {
>                   printf("KVM not supported for this target\n");
>               } else {
>                   fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
>               }
>               exit(1);
> -#endif
> -#ifdef CONFIG_KVM
> -            fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> -            kvm_allowed = 0;
> -#endif
>           }
>       }
>
>    

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 12/15/2010 09:50 AM, Markus Armbruster wrote:
>> We currently enable KVM by default, and when it's not available, we
>> print a message and fall back to TCG.  Option -enable-kvm is ignored.
>> Option -no-kvm suppresses KVM.
>>
>> Upstream works differently: KVM is off by default, -enable-kvm
>> switches it on.  -enable-kvm terminates the process unsuccessfully if
>> KVM is not available.
>>
>> upstream qemu   |  default  |-enable-kvm
>> ----------------+-----------+-----------
>> KVM available   | disabled  |  enabled
>> KVM unavailable | disabled  |    fail
>>
>> qemu-kvm        |  default  |-enable-kvm
>> ----------------+-----------+-----------
>> KVM available   |  enabled* |  enabled
>> KVM unavailable | disabled  | disabled*
>>
>> * differs from upstream
>>
>> Users of qemu and qemu-kvm need to be aware of these differences to
>> enable / disable use of KVM reliably.  This is bothersome.
>>
>> Consider -enable-kvm when KVM is unavailable: If the user expects
>> qemu-kvm behavior (fall back), but qemu fails, he'll likely be
>> surprised and unhappy.  If the user expects upstream behavior (fail),
>> but qemu-kvm falls back to TCG, the guest runs slow as molasses, and
>> the user will likely be confused and unhappy (unless he spots and
>> understands the "disable KVM" message).
>>
>> Switch to upstream semantics: KVM off by default, -enable-kvm switches
>> it on, and when it can't, it's fatal.
>>
>> Having to enable KVM explicitly is annoying, but the proper place to
>> address that is upstream.
>>
>> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>>    
>
> Backwards compatibility is going to kill us if we try to make this change.
>
> Current qemu-kvm behavior:
>
> default: -accel kvm,tcg
> -no-kvm: -accel tcg
> -enable-kvm: -accel kvm,tcg
>
> Current upstream behavior
>
> default: -accel tcg
> -enable-kvm: -accel kvm
>
> I think we should tie `-accel' to the machine type.  For qemu-kvm, a
> different default machine type should be used than upstream qemu (it
> really should be a configure switch).
>
> For `pc', the default `-accel' behavior should remain 'tcg'.  For
> kvmpc', the default `-accel' behavior should be 'kvm,tcg'.
>
> -no-kvm should be deprecated.  -enable-kvm should also be deprecated
> in favor of the `-accel' option.

I'm fine with -accel and deprecating the old options.  But until we have
that:

> In the short term, it would be a good idea to modify qemu-kvm to
> switch the -enable-kvm semantics to match upstream (fail if KVM isn't
> available).

That's what my patch does.

Additionally, it changes the default to match upstream: KVM disabled.

What do you want changed in my patch?

>              Adding an alias for 'kvmpc' upstream and qemu-kvm and
> making qemu-kvm default to 'kvmpc' would be helpful for management
> tools too.

That would address "Having to enable KVM explicitly is annoying".

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Avi Kivity]

On 12/15/2010 07:57 PM, Markus Armbruster wrote:
> >  In the short term, it would be a good idea to modify qemu-kvm to
> >  switch the -enable-kvm semantics to match upstream (fail if KVM isn't
> >  available).
>
> That's what my patch does.
>
> Additionally, it changes the default to match upstream: KVM disabled.
>
> What do you want changed in my patch?

The 'Additionally' bit.  qemu-kvm users rely on the default enabling 
kvm.  Likely they don't rely on -enable-kvm failing is kvm is not 
available (and indeed, they likely expect it to match upstream).  So the 
patch should only change behaviour when -enable-kvm is specified.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Markus Armbruster]

Avi Kivity <avi@redhat.com> writes:

> On 12/15/2010 07:57 PM, Markus Armbruster wrote:
>> >  In the short term, it would be a good idea to modify qemu-kvm to
>> >  switch the -enable-kvm semantics to match upstream (fail if KVM isn't
>> >  available).
>>
>> That's what my patch does.
>>
>> Additionally, it changes the default to match upstream: KVM disabled.
>>
>> What do you want changed in my patch?
>
> The 'Additionally' bit.  qemu-kvm users rely on the default enabling
> kvm.  Likely they don't rely on -enable-kvm failing is kvm is not
> available (and indeed, they likely expect it to match upstream).  So
> the patch should only change behaviour when -enable-kvm is specified.

Like this?

upstream qemu   |  default  |-enable-kvm
----------------+-----------+-----------
KVM available   | disabled  |  enabled
KVM unavailable | disabled  |    fail

qemu-kvm        |  default  |-enable-kvm|  -no-kvm
----------------+-----------+-----------+-----------
KVM available   |  enabled* |  enabled  |  disabled
KVM unavailable | disabled  |    fail   |  disabled

* differs from upstream

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Avi Kivity]

On 12/21/2010 05:41 PM, Markus Armbruster wrote:
> Avi Kivity<avi@redhat.com>  writes:
>
> >  On 12/15/2010 07:57 PM, Markus Armbruster wrote:
> >>  >   In the short term, it would be a good idea to modify qemu-kvm to
> >>  >   switch the -enable-kvm semantics to match upstream (fail if KVM isn't
> >>  >   available).
> >>
> >>  That's what my patch does.
> >>
> >>  Additionally, it changes the default to match upstream: KVM disabled.
> >>
> >>  What do you want changed in my patch?
> >
> >  The 'Additionally' bit.  qemu-kvm users rely on the default enabling
> >  kvm.  Likely they don't rely on -enable-kvm failing is kvm is not
> >  available (and indeed, they likely expect it to match upstream).  So
> >  the patch should only change behaviour when -enable-kvm is specified.
>
> Like this?
>
> upstream qemu   |  default  |-enable-kvm
> ----------------+-----------+-----------
> KVM available   | disabled  |  enabled
> KVM unavailable | disabled  |    fail
>
> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
> ----------------+-----------+-----------+-----------
> KVM available   |  enabled* |  enabled  |  disabled
> KVM unavailable | disabled  |    fail   |  disabled
>
> * differs from upstream

Yes.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Richard W.M. Jones]

On Tue, Dec 21, 2010 at 04:41:03PM +0100, Markus Armbruster wrote:
> Like this?
> 
> upstream qemu   |  default  |-enable-kvm
> ----------------+-----------+-----------
> KVM available   | disabled  |  enabled
> KVM unavailable | disabled  |    fail
> 
> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
> ----------------+-----------+-----------+-----------
> KVM available   |  enabled* |  enabled  |  disabled
> KVM unavailable | disabled  |    fail   |  disabled
> 
> * differs from upstream

libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die"
behaviour.

Avi, can you comment on whether just opening /dev/kvm O_RDWR is a
reasonable way to detect if KVM is available?

Markus, any idea when we might get the -accel option appearing in
released versions of qemu/KVM?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Richard W.M. Jones]

On Tue, Dec 21, 2010 at 04:00:32PM +0000, Richard W.M. Jones wrote:
> Markus, any idea when we might get the -accel option appearing in
> released versions of qemu/KVM?

Sorry, I thought this email wasn't going out to a public list.  I
should be more careful next time.

I'll say instead: We really need both the -accel option and a working
system of qemu capabilities.  Both would be hugely helpful making it
easier to work with qemu.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Markus Armbruster]

Avi Kivity <avi@redhat.com> writes:

> On 12/21/2010 05:41 PM, Markus Armbruster wrote:
>> Avi Kivity<avi@redhat.com>  writes:
>>
>> >  On 12/15/2010 07:57 PM, Markus Armbruster wrote:
>> >>  >   In the short term, it would be a good idea to modify qemu-kvm to
>> >>  >   switch the -enable-kvm semantics to match upstream (fail if KVM isn't
>> >>  >   available).
>> >>
>> >>  That's what my patch does.
>> >>
>> >>  Additionally, it changes the default to match upstream: KVM disabled.
>> >>
>> >>  What do you want changed in my patch?
>> >
>> >  The 'Additionally' bit.  qemu-kvm users rely on the default enabling
>> >  kvm.  Likely they don't rely on -enable-kvm failing is kvm is not
>> >  available (and indeed, they likely expect it to match upstream).  So
>> >  the patch should only change behaviour when -enable-kvm is specified.
>>
>> Like this?
>>
>> upstream qemu   |  default  |-enable-kvm
>> ----------------+-----------+-----------
>> KVM available   | disabled  |  enabled
>> KVM unavailable | disabled  |    fail
>>
>> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
>> ----------------+-----------+-----------+-----------
>> KVM available   |  enabled* |  enabled  |  disabled
>> KVM unavailable | disabled  |    fail   |  disabled
>>
>> * differs from upstream
>
> Yes.

Thanks.  I'll cook up a new patch.

[Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Markus Armbruster]

"Richard W.M. Jones" <rjones@redhat.com> writes:

> On Tue, Dec 21, 2010 at 04:41:03PM +0100, Markus Armbruster wrote:
>> Like this?
>> 
>> upstream qemu   |  default  |-enable-kvm
>> ----------------+-----------+-----------
>> KVM available   | disabled  |  enabled
>> KVM unavailable | disabled  |    fail
>> 
>> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
>> ----------------+-----------+-----------+-----------
>> KVM available   |  enabled* |  enabled  |  disabled
>> KVM unavailable | disabled  |    fail   |  disabled
>> 
>> * differs from upstream
>
> libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die"
> behaviour.

For what it's worth, default gives you exactly that with qemu-kvm.
Maybe that's good enough, on the theory that if you have KVM, you most
likely have libguestfs using qemu-kvm.

> Avi, can you comment on whether just opening /dev/kvm O_RDWR is a
> reasonable way to detect if KVM is available?
>
> Markus, any idea when we might get the -accel option appearing in
> released versions of qemu/KVM?

No idea.  Anthony?

Re: [Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Anthony Liguori]

On 12/21/2010 10:07 AM, Markus Armbruster wrote:
> "Richard W.M. Jones"<rjones@redhat.com>  writes:
>
>    
>> On Tue, Dec 21, 2010 at 04:41:03PM +0100, Markus Armbruster wrote:
>>      
>>> Like this?
>>>
>>> upstream qemu   |  default  |-enable-kvm
>>> ----------------+-----------+-----------
>>> KVM available   | disabled  |  enabled
>>> KVM unavailable | disabled  |    fail
>>>
>>> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
>>> ----------------+-----------+-----------+-----------
>>> KVM available   |  enabled* |  enabled  |  disabled
>>> KVM unavailable | disabled  |    fail   |  disabled
>>>
>>> * differs from upstream
>>>        
>> libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die"
>> behaviour.
>>      
> For what it's worth, default gives you exactly that with qemu-kvm.
> Maybe that's good enough, on the theory that if you have KVM, you most
> likely have libguestfs using qemu-kvm.
>
>    
>> Avi, can you comment on whether just opening /dev/kvm O_RDWR is a
>> reasonable way to detect if KVM is available?
>>
>> Markus, any idea when we might get the -accel option appearing in
>> released versions of qemu/KVM?
>>      
> No idea.  Anthony?
>    

I see no problem with 0.15 if someone cooks up a patch.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics

[Alexander Graf]

On 21.12.2010, at 17:56, Anthony Liguori wrote:

> On 12/21/2010 10:07 AM, Markus Armbruster wrote:
>> "Richard W.M. Jones"<rjones@redhat.com>  writes:
>> 
>>   
>>> On Tue, Dec 21, 2010 at 04:41:03PM +0100, Markus Armbruster wrote:
>>>     
>>>> Like this?
>>>> 
>>>> upstream qemu   |  default  |-enable-kvm
>>>> ----------------+-----------+-----------
>>>> KVM available   | disabled  |  enabled
>>>> KVM unavailable | disabled  |    fail
>>>> 
>>>> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
>>>> ----------------+-----------+-----------+-----------
>>>> KVM available   |  enabled* |  enabled  |  disabled
>>>> KVM unavailable | disabled  |    fail   |  disabled
>>>> 
>>>> * differs from upstream
>>>>       
>>> libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die"
>>> behaviour.
>>>     
>> For what it's worth, default gives you exactly that with qemu-kvm.
>> Maybe that's good enough, on the theory that if you have KVM, you most
>> likely have libguestfs using qemu-kvm.
>> 
>>   
>>> Avi, can you comment on whether just opening /dev/kvm O_RDWR is a
>>> reasonable way to detect if KVM is available?
>>> 
>>> Markus, any idea when we might get the -accel option appearing in
>>> released versions of qemu/KVM?
>>>     
>> No idea.  Anthony?
>>   
> 
> I see no problem with 0.15 if someone cooks up a patch.

Didn't Anthony do one? What happened to the Xen patch set anyways? :)


Alex