[libvirt] [PATCH v3 4/5] Add a watchdog action `dump'

[Daniel P. Berrange]

On Wed, Dec 01, 2010 at 06:39:14PM -0700, Eric Blake wrote:
> On 12/01/2010 05:50 PM, Hu Tao wrote:
> > On Tue, Nov 30, 2010 at 03:21:36PM -0700, Eric Blake wrote:
> > <...snip...>
> >>>  
> >>> -libvirt_qemu_la_SOURCES = libvirt-qemu.c
> >>> +libvirt_qemu_la_SOURCES = libvirt-qemu.c util/threadpool.c
> >>
> >> Why is this change necessary?  Shouldn't libvirt_util.la already include
> >> threadpool.c, and the qemu driver already be linking with libvirt_util.la?
> > 
> > Is this ok?
> > 
> > -libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS)
> > +libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) ../src/libvirt_util.la
> 
> Nope; rather...
> 
> > 
> > Or link will fail:
> > 
> >   CCLD   libvirtd
> > libvirtd-libvirtd.o: In function `qemudRunLoop':
> > /mnt/data/kernel/libvirt/daemon/libvirtd.c:2229: undefined reference to `virWorkerPoolNew'
> > /mnt/data/kernel/libvirt/daemon/libvirtd.c:2287: undefined reference to `virWorkerPoolFree'
> 
> That means you need to modify src/libvirt_private.syms to export the new
> public interfaces from threadpool.h (it should be pretty easy to figure
> out what edits to make, the tricky part is realizing you need to touch
> that file in the first place).
> 
> >>> +        if (virAsprintf(&qemu_driver->autoDumpPath, "%s/qemu/dump", base) == -1)
> >>> +            goto out_of_memory;
> >>
> >> However, it does raise an issue.  Is qemu.conf only for privileged
> >> users, or do we have to worry about allowing non-privileged users also
> >> be able to pick up an alternate directory (especially since they can't
> >> dump to /var/log/...)?
> > 
> > qemu.conf is only for privileged users, but non-privileged users who
> > need to analyze dump files should ask admin to specify an auto-dump
> > directory they have right to read.
> > 
> > Or do you have a better idea?
> 
> Is it better to dump a non-privileged log into ~/.libvirt/qemu/dump, so
> that it's automatically user-accessible?  We already use ~/.libvirt for
> other non-privileged files.

I think you're mixing up unprivileged users using the privileged
qemu:///system, with unprivileged users using the unprivileged
driver qemu://session. Only the latter uses ~/.libvirt and this
patch should already be using ~/.libvirt/qemu/dump in that
scenario.

Daniel