Re: [libvirt] [PATCH v3 4/5] Add a watchdog action `dump'
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
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'
libvirtd-libvirtd.o: In function `qemudDispatchClientRead':
/mnt/data/kernel/libvirt/daemon/libvirtd.c:1778: undefined reference to
`virWorkerPoolSendJob'
../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_driver.o): In function
`qemuHandleDomainWatchdog':
/mnt/data/kernel/libvirt/src/qemu/qemu_driver.c:1224: undefined reference to
`virWorkerPoolSendJob'
../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_driver.o): In function
`qemudShutdown':
/mnt/data/kernel/libvirt/src/qemu/qemu_driver.c:2147: undefined reference to
`virWorkerPoolFree'
../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_driver.o): In function
`qemudStartup':
/mnt/data/kernel/libvirt/src/qemu/qemu_driver.c:2007: undefined reference to
`virWorkerPoolNew'
collect2: ld returned 1 exit status
<...snip...>
> +
> virDomainObjUnlock(vm);
>
> if (watchdogEvent || lifecycleEvent) {
> @@ -1788,6 +1807,9 @@ qemudStartup(int privileged) {
> if (virAsprintf(&qemu_driver->snapshotDir,
> "%s/lib/libvirt/qemu/snapshot", LOCALSTATEDIR) ==
-1)
> goto out_of_memory;
> + if (virAsprintf(&qemu_driver->autoDumpPath,
> + "%s/lib/libvirt/qemu/dump", LOCALSTATEDIR) ==
-1)
At first glance, I'm not quite sure why autoDumpPath is configurable but
not snapshotDir. I guess it has to do with the fact that snapshots are
under libvirt control (the user does not need to know that they exist),
but dump files are intended to be consumed by the user (so the user
should be able to specify an alternate location).
Yes.
> + goto out_of_memory;
> } else {
> uid_t uid = geteuid();
> char *userdir = virGetUserDirectory(uid);
> @@ -1816,6 +1838,8 @@ qemudStartup(int privileged) {
> goto out_of_memory;
> if (virAsprintf(&qemu_driver->snapshotDir,
"%s/qemu/snapshot", base) == -1)
> goto out_of_memory;
> + 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?
--
Thanks,
Hu Tao