[libvirt] [PATCH] Fix segfault if no qemu emulator passed
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes. This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though). Attached patch fixes this. Thanks, Cole diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d742c32..23ef050 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -790,7 +790,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!emulator) return -1; - ADD_ARG_LIT(vm->def->emulator); + ADD_ARG_LIT(emulator); ADD_ARG_LIT("-S"); ADD_ARG_LIT("-M"); ADD_ARG_LIT(vm->def->os.machine);
On Mon, Sep 08, 2008 at 11:10:17PM -0400, Cole Robinson wrote:
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes.
This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though).
#%(*&#@(%@. Damn. This is worthy of a brown paper bag release for libvirt to fix this shockingly bad bug.
diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d742c32..23ef050 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -790,7 +790,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!emulator) return -1;
- ADD_ARG_LIT(vm->def->emulator); + ADD_ARG_LIT(emulator); ADD_ARG_LIT("-S"); ADD_ARG_LIT("-M"); ADD_ARG_LIT(vm->def->os.machine);
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Sep 09, 2008 at 09:56:47AM +0100, Daniel P. Berrange wrote:
On Mon, Sep 08, 2008 at 11:10:17PM -0400, Cole Robinson wrote:
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes.
This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though).
#%(*&#@(%@. Damn. This is worthy of a brown paper bag release for libvirt to fix this shockingly bad bug.
On a semi-related point, I recently updated my automated build server to run Fedora 9, which means we now get code coverage reports for the libvirt test suite. http://builder.virt-manager.org/artifacts/libvirt--devel/coverage/ If you look at line 789 you can see we never run any tests where the emulator is NULL, which is why we never caught this bug. Something to fix in the test suite :-) http://builder.virt-manager.org/artifacts/libvirt--devel/coverage/libvirt--d... It also highlights that we really need to create some kind of functional test rig, because we've got alot of code that is simply not possible to unit test because it interacts with host OS state / functionality. That said we ought to be able to get some better code coverage of the libvirtd daemon by leveraging the test hypervisor drive at least. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Mon, Sep 08, 2008 at 11:10:17PM -0400, Cole Robinson wrote:
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes.
This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though).
#%(*&#@(%@. Damn. This is worthy of a brown paper bag release for libvirt to fix this shockingly bad bug.
If we are going to do this I'd really like to get the 2 pending max vcpu patches committed, since it's a big issue that we largely disallow smp kvm guests at this moment.
diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d742c32..23ef050 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -790,7 +790,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!emulator) return -1;
- ADD_ARG_LIT(vm->def->emulator); + ADD_ARG_LIT(emulator); ADD_ARG_LIT("-S"); ADD_ARG_LIT("-M"); ADD_ARG_LIT(vm->def->os.machine);
ACK
Daniel
Thanks, I've committed this. - Cole
On Tue, Sep 09, 2008 at 10:24:51AM -0400, Cole Robinson wrote:
Daniel P. Berrange wrote:
On Mon, Sep 08, 2008 at 11:10:17PM -0400, Cole Robinson wrote:
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes.
This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though).
#%(*&#@(%@. Damn. This is worthy of a brown paper bag release for libvirt to fix this shockingly bad bug.
If we are going to do this I'd really like to get the 2 pending max vcpu patches committed, since it's a big issue that we largely disallow smp kvm guests at this moment.
Do you need more reviews ? I didn't realized we had pending unapplied patches ... Repost if necessary, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard wrote:
On Tue, Sep 09, 2008 at 10:24:51AM -0400, Cole Robinson wrote:
Daniel P. Berrange wrote:
On Mon, Sep 08, 2008 at 11:10:17PM -0400, Cole Robinson wrote:
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes.
This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though). #%(*&#@(%@. Damn. This is worthy of a brown paper bag release for libvirt to fix this shockingly bad bug.
If we are going to do this I'd really like to get the 2 pending max vcpu patches committed, since it's a big issue that we largely disallow smp kvm guests at this moment.
Do you need more reviews ? I didn't realized we had pending unapplied patches ... Repost if necessary, thanks !
Daniel
My maxvcpu patch is dependent on Guido's patch which hasn't been committed yet but has received enough ACKs Thanks, Cole
Hey all, I'd quite like the ability to nice my KVM process, on a home basis this stops my Windows VM locking up my linux desktop when it's under load (or at least limits it) and in a commercial setting it might be nice to offer CPU priority to other customers or company backup-services over customer VPS instances for example. How does it sound? Any thoughts? A quick chat in #virt revealed that a method for adding generic KVM options has been under discussion for ages - I thought i'd throw my two cents in, what about some sort of expression with variables like: <cmdstring>{cmd} {options}</cmdstring> (default) for my nice proposition you could: <cmdstring>/usr/bin/nice {cmd} {options}</cmdstring> Obviously people could use this to add conflicting options which is concerning but perhaps that should be their own lookout - it seems to be a fairly advanced request. Thanks, Henri
On Tue, Sep 09, 2008 at 10:23:59PM +0100, Henri Cook wrote:
Hey all,
I'd quite like the ability to nice my KVM process, on a home basis this stops my Windows VM locking up my linux desktop when it's under load (or at least limits it) and in a commercial setting it might be nice to offer CPU priority to other customers or company backup-services over customer VPS instances for example.
How does it sound? Any thoughts?
A quick chat in #virt revealed that a method for adding generic KVM options has been under discussion for ages - I thought i'd throw my two cents in, what about some sort of expression with variables like:
A method for adding arbitrary KVM options will never be merged in libvirt....
<cmdstring>{cmd} {options}</cmdstring> (default)
for my nice proposition you could:
<cmdstring>/usr/bin/nice {cmd} {options}</cmdstring>
The intent of libvirt is to provide APIs which can be used across all hypervisors. Taking the 'nice' example, this is really a schedular parameter. If we added ability to set 'nice -20' in the XML for KVM, there is no way we could possibly implement this for Xen. So the goal is to find a consistent API representation. Fortunately we do already have a 'schedular parameters' API in libvirt - we simply need to decide how to implement this for KVM - a 'nice' setting is certainly one schedular tunable we'd likely want to support. So if someone wants to implenment the schedular parameters driver API for KVM patches welcomed... Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
A good news - sorry I wasn't aware of the current stance on arbitrary parameters and can say I completely agree with it. Scheduler Parameters API sounds interesting - i'll see if I can find time to look into it - if there's anyone who already knows how it works that sees this as a quick patch though, massive points Henri Daniel P. Berrange wrote:
On Tue, Sep 09, 2008 at 10:23:59PM +0100, Henri Cook wrote:
Hey all,
I'd quite like the ability to nice my KVM process, on a home basis this stops my Windows VM locking up my linux desktop when it's under load (or at least limits it) and in a commercial setting it might be nice to offer CPU priority to other customers or company backup-services over customer VPS instances for example.
How does it sound? Any thoughts?
A quick chat in #virt revealed that a method for adding generic KVM options has been under discussion for ages - I thought i'd throw my two cents in, what about some sort of expression with variables like:
A method for adding arbitrary KVM options will never be merged in libvirt....
<cmdstring>{cmd} {options}</cmdstring> (default)
for my nice proposition you could:
<cmdstring>/usr/bin/nice {cmd} {options}</cmdstring>
The intent of libvirt is to provide APIs which can be used across all hypervisors. Taking the 'nice' example, this is really a schedular parameter. If we added ability to set 'nice -20' in the XML for KVM, there is no way we could possibly implement this for Xen.
So the goal is to find a consistent API representation. Fortunately we do already have a 'schedular parameters' API in libvirt - we simply need to decide how to implement this for KVM - a 'nice' setting is certainly one schedular tunable we'd likely want to support.
So if someone wants to implenment the schedular parameters driver API for KVM patches welcomed...
Regards, Daniel
On Tue, Sep 09, 2008 at 10:55:26PM +0100, Henri Cook wrote:
A good news - sorry I wasn't aware of the current stance on arbitrary parameters and can say I completely agree with it.
Scheduler Parameters API sounds interesting - i'll see if I can find time to look into it - if there's anyone who already knows how it works that sees this as a quick patch though, massive points
http://libvirt.org/html/libvirt-libvirt.html#virDomainSetSchedulerParameters http://libvirt.org/html/libvirt-libvirt.html#virSchedParameter http://libvirt.org/html/libvirt-libvirt.html#virDomainGetSchedulerParameters it's not fantastic, it's a free form API, you need to be consistent in the name and type of parameters returned by Get and allow a subset to be modified by Set. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Am Mittwoch, den 10.09.2008, 07:49 +0200 schrieb Daniel Veillard:
On Tue, Sep 09, 2008 at 10:55:26PM +0100, Henri Cook wrote:
A good news - sorry I wasn't aware of the current stance on arbitrary parameters and can say I completely agree with it.
Scheduler Parameters API sounds interesting - i'll see if I can find time to look into it - if there's anyone who already knows how it works that sees this as a quick patch though, massive points
http://libvirt.org/html/libvirt-libvirt.html#virDomainSetSchedulerParameters
A couple of months ago I tried to implement this for kvm. I failed on rpc though. It should be as "easy" as setting setpriority(2).
http://libvirt.org/html/libvirt-libvirt.html#virSchedParameter
http://libvirt.org/html/libvirt-libvirt.html#virDomainGetSchedulerParameters
it's not fantastic, it's a free form API, you need to be consistent in the name and type of parameters returned by Get and allow a subset to be modified by Set.
Daniel
On Wed, Sep 10, 2008 at 08:28:52AM +0200, Fabian Deutsch wrote:
Am Mittwoch, den 10.09.2008, 07:49 +0200 schrieb Daniel Veillard:
On Tue, Sep 09, 2008 at 10:55:26PM +0100, Henri Cook wrote:
A good news - sorry I wasn't aware of the current stance on arbitrary parameters and can say I completely agree with it.
Scheduler Parameters API sounds interesting - i'll see if I can find time to look into it - if there's anyone who already knows how it works that sees this as a quick patch though, massive points
http://libvirt.org/html/libvirt-libvirt.html#virDomainSetSchedulerParameters
A couple of months ago I tried to implement this for kvm. I failed on rpc though.
If you got as far as writing some code for the QMEU driver do post it to the list even if its not finished/usable. The RPC stuff can be a little tricky to understand for those unfamiliar with it - but it is easy enough for one of us to add.
It should be as "easy" as setting setpriority(2).
That's just one tunable - there are others like sched_setparam, and sched_setschedular Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Sep 09, 2008 at 10:23:59PM +0100, Henri Cook wrote:
<cmdstring>{cmd} {options}</cmdstring> (default)
This isn't going into libvirt because the whole point of libvirt is to be hypervisor independent. Nevertheless, you can do this, and the 'nice' thing, by setting <emulator> to be your own custom shell script. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
Great idea, thanks! Richard W.M. Jones wrote:
On Tue, Sep 09, 2008 at 10:23:59PM +0100, Henri Cook wrote:
<cmdstring>{cmd} {options}</cmdstring> (default)
This isn't going into libvirt because the whole point of libvirt is to be hypervisor independent. Nevertheless, you can do this, and the 'nice' thing, by setting <emulator> to be your own custom shell script.
Rich.
On Fri, Sep 12, 2008 at 10:10:42AM +0100, Richard W.M. Jones wrote:
On Tue, Sep 09, 2008 at 10:23:59PM +0100, Henri Cook wrote:
<cmdstring>{cmd} {options}</cmdstring> (default)
This isn't going into libvirt because the whole point of libvirt is to be hypervisor independent. Nevertheless, you can do this, and the 'nice' thing, by setting <emulator> to be your own custom shell script.
Bear in mind that if you do this, we make *zero* guarentee about compatability with future releases. We reserve the right to change the way we invoke KVM, the args we pass, and OS interaction. So while you can run it via 'nice', it is conceivable that future libvirt will override whatever setting you make. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Makes sense, i'm not looking for a solution immediately so when I get around to it i'll look for a libvirt way of doing things first - adding my own wrapper was an interesting thought but something I would pursue only as a last resort Daniel P. Berrange wrote:
On Fri, Sep 12, 2008 at 10:10:42AM +0100, Richard W.M. Jones wrote:
On Tue, Sep 09, 2008 at 10:23:59PM +0100, Henri Cook wrote:
<cmdstring>{cmd} {options}</cmdstring> (default)
This isn't going into libvirt because the whole point of libvirt is to be hypervisor independent. Nevertheless, you can do this, and the 'nice' thing, by setting <emulator> to be your own custom shell script.
Bear in mind that if you do this, we make *zero* guarentee about compatability with future releases. We reserve the right to change the way we invoke KVM, the args we pass, and OS interaction. So while you can run it via 'nice', it is conceivable that future libvirt will override whatever setting you make.
Daniel
On Tue, Sep 09, 2008 at 09:56:47AM +0100, Daniel P. Berrange wrote:
On Mon, Sep 08, 2008 at 11:10:17PM -0400, Cole Robinson wrote:
There is unfortunately a pretty prevalent segfault in the latest libvirt. If a qemu domain is defined without an emulator specified, libvirtd crashes.
This is doubly unfortunate since current virtinst generates an emulator-less config for all qemu and kvm guests (I'm about to fix this upstream though).
#%(*&#@(%@. Damn. This is worthy of a brown paper bag release for libvirt to fix this shockingly bad bug.
I would not rush, let's isolate the few fixes needed and repush a whole release in a week or two. There have been so many changes to 0.4.5 that this kind of things is likely to happen ... more than once. Like 0.4.4 was a good release, fixing the obviour errors found in 0.4.3 first 10 days it's a clasical pattern :-) Distros with fast update cycles will push the patches, others should monitor and look for 0.4.6 within 1-2 weeks. I will push the patch to Fedoras build, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (6)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard -
Fabian Deutsch -
Henri Cook -
Richard W.M. Jones