Re: [Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
On 21.12.2010, at 17:56, Anthony Liguori wrote: > On 12/21/2010 10:07 AM, Markus Armbruster wrote: >> "Richard W.M. Jones" 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
On 12/21/2010 10:07 AM, Markus Armbruster wrote: "Richard W.M. Jones" 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
"Richard W.M. Jones" 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? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
Avi Kivity writes: > On 12/21/2010 05:41 PM, Markus Armbruster wrote: >> Avi Kivity 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
On Tue, Dec 21, 2010 at 04:00:32PM +, 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
On 12/21/2010 05:41 PM, Markus Armbruster wrote: Avi Kivity 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
Avi Kivity 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
Anthony Liguori 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 >> > > 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". -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu-kvm: Switch to upstream -enable-kvm semantics
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 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 } } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html