[libvirt] [PATCH] 528575 avoid libvirtd crash on LCX domain autostart

https://bugzilla.redhat.com/show_bug.cgi?id=528575 virsh -c lxc:/// autostart vm1 crash the daemon because the driver autostartDir is never initialized and we do a NULL dereference. The enclosed patch is trivial and avoid the crash, but ... [root@paphio ~]# virsh -c lxc:/// dominfo vm1 Id: - Name: vm1 UUID: d320f760-7541-6633-fd42-4f984fca9f51 OS Type: exe State: shut off CPU(s): 1 Max memory: 500000 kB Used memory: 500000 kB Autostart: enable while the domain is properly flagged as autostarting it doesn't seem to actually autostart, and the driver->autostartDir directory is not created on the fly as I would expect, i.e. /etc/libvirt/lxc/autostart/ still doesn't exist and the domain is marked as off and Autostart: enable I think it would be good to ACK the patch to avoid the crash but we also need to have someone look at the autostart of LXC domains and double check what is going on, maybe I missed something :-) Ozaki, any chance you could look at this ? 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/

On Wed, Oct 21, 2009 at 11:52:42AM +0200, Daniel Veillard wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=528575
virsh -c lxc:/// autostart vm1
crash the daemon because the driver autostartDir is never initialized and we do a NULL dereference. The enclosed patch is trivial and avoid the crash, but ...
[root@paphio ~]# virsh -c lxc:/// dominfo vm1 Id: - Name: vm1 UUID: d320f760-7541-6633-fd42-4f984fca9f51 OS Type: exe State: shut off CPU(s): 1 Max memory: 500000 kB Used memory: 500000 kB Autostart: enable
while the domain is properly flagged as autostarting it doesn't seem to actually autostart, and the driver->autostartDir directory is not created on the fly as I would expect, i.e. /etc/libvirt/lxc/autostart/ still doesn't exist and the domain is marked as off and Autostart: enable
That's because of :
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index 6e4c855..eca8c7c 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -36,6 +36,7 @@ #define LXC_CONFIG_DIR SYSCONF_DIR "/libvirt/lxc" #define LXC_STATE_DIR LOCAL_STATE_DIR "/run/libvirt/lxc" #define LXC_LOG_DIR LOCAL_STATE_DIR "/log/libvirt/lxc" +#define LXC_AUTOSTART_DIR LOCAL_STATE_DIR "/libvirt/lxc/autostart"
LOCAL_STATE_DIR expands to /var The autostart dir should be defined as #define LXC_AUTOSTART_DIR LXC_CONFIG_DIR "/autostart" 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 :|

On Wed, Oct 21, 2009 at 6:55 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Oct 21, 2009 at 11:52:42AM +0200, Daniel Veillard wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=528575
virsh -c lxc:/// autostart vm1
crash the daemon because the driver autostartDir is never initialized and we do a NULL dereference. The enclosed patch is trivial and avoid the crash, but ...
[root@paphio ~]# virsh -c lxc:/// dominfo vm1 Id: - Name: vm1 UUID: d320f760-7541-6633-fd42-4f984fca9f51 OS Type: exe State: shut off CPU(s): 1 Max memory: 500000 kB Used memory: 500000 kB Autostart: enable
while the domain is properly flagged as autostarting it doesn't seem to actually autostart, and the driver->autostartDir directory is not created on the fly as I would expect, i.e. /etc/libvirt/lxc/autostart/ still doesn't exist and the domain is marked as off and Autostart: enable
That's because of :
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index 6e4c855..eca8c7c 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -36,6 +36,7 @@ #define LXC_CONFIG_DIR SYSCONF_DIR "/libvirt/lxc" #define LXC_STATE_DIR LOCAL_STATE_DIR "/run/libvirt/lxc" #define LXC_LOG_DIR LOCAL_STATE_DIR "/log/libvirt/lxc" +#define LXC_AUTOSTART_DIR LOCAL_STATE_DIR "/libvirt/lxc/autostart"
LOCAL_STATE_DIR expands to /var
The autostart dir should be defined as
#define LXC_AUTOSTART_DIR LXC_CONFIG_DIR "/autostart"
Yes, that's correct. I've confirmed it fixes the defect in my environment. So ACK to the revised one. BTW, actually I've never used autostart feature though, when autostart-enabled domains boot up? 'service libvirtd restart' seems not kicking my lxc... ozaki-r
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 :|
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Oct 21, 2009 at 07:31:11PM +0900, Ryota Ozaki wrote:
On Wed, Oct 21, 2009 at 6:55 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Oct 21, 2009 at 11:52:42AM +0200, Daniel Veillard wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=528575
virsh -c lxc:/// autostart vm1
crash the daemon because the driver autostartDir is never initialized and we do a NULL dereference. The enclosed patch is trivial and avoid the crash, but ...
[root@paphio ~]# virsh -c lxc:/// dominfo vm1 Id: - Name: vm1 UUID: d320f760-7541-6633-fd42-4f984fca9f51 OS Type: exe State: shut off CPU(s): 1 Max memory: 500000 kB Used memory: 500000 kB Autostart: enable
while the domain is properly flagged as autostarting it doesn't seem to actually autostart, and the driver->autostartDir directory is not created on the fly as I would expect, i.e. /etc/libvirt/lxc/autostart/ still doesn't exist and the domain is marked as off and Autostart: enable
That's because of :
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index 6e4c855..eca8c7c 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -36,6 +36,7 @@ #define LXC_CONFIG_DIR SYSCONF_DIR "/libvirt/lxc" #define LXC_STATE_DIR LOCAL_STATE_DIR "/run/libvirt/lxc" #define LXC_LOG_DIR LOCAL_STATE_DIR "/log/libvirt/lxc" +#define LXC_AUTOSTART_DIR LOCAL_STATE_DIR "/libvirt/lxc/autostart"
LOCAL_STATE_DIR expands to /var
Dohhh
The autostart dir should be defined as
#define LXC_AUTOSTART_DIR LXC_CONFIG_DIR "/autostart"
and I had the process under gdb, I didn't even looked at the value :-\
Yes, that's correct. I've confirmed it fixes the defect in my environment. So ACK to the revised one.
BTW, actually I've never used autostart feature though, when autostart-enabled domains boot up? 'service libvirtd restart' seems not kicking my lxc...
Thanks a lot for the quick feedback, enclosed patch applied ! 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 (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Ryota Ozaki