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