12:41 p.m.
On 01/28/2011 06:21 AM, Daniel P. Berrange wrote:
Move the qemudStartVMDaemon and qemudShutdownVMDaemon
methods into a separate file, renaming them to
qemuProcessStart, qemuProcessStop. All helper methods
called by these are also moved & renamed to match
* src/Makefile.am: Add qemu_process.c/.h
* src/qemu/qemu_command.c: Add emuDomainAssignPCIAddresses
s/ emu/ qemu/
* src/qemu/qemu_command.h: Add VNC port min/max
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
domain event queue helpers
* src/qemu/qemu_driver.c, src/qemu/qemu_driver.h: Remove
all QEMU process startup/shutdown functions
* src/qemu/qemu_process.c, src/qemu/qemu_process.h: Add
all QEMU process startup/shutdown functions
+++ b/src/qemu/qemu_domain.c
@@ -29,6 +29,7 @@
#include "logging.h"
#include "virterror_internal.h"
#include "c-ctype.h"
+#include "event.h"
#include <sys/time.h>
@@ -41,6 +42,61 @@
#define timeval_to_ms(tv) (((tv).tv_sec * 1000ull) + ((tv).tv_usec / 1000))
+static void qemuDomainEventDispatchFunc(virConnectPtr conn,
+ virDomainEventPtr event,
+ virConnectDomainEventGenericCallback cb,
+ void *cbopaque,
+ void *opaque)
+{
+ struct qemud_driver *driver = opaque;
+
+ /* Drop the lock whle dispatching, for sake of re-entrancy */
s/whle/while/
+++ b/src/qemu/qemu_driver.c
+# define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */
+# endif
+
+
+#define timeval_to_ms(tv) (((tv).tv_sec * 1000ull) + ((tv).tv_usec / 1000))
This will fail 'make syntax-check' if you have cppi installed.
-static int doStartCPUs(struct qemud_driver *driver, virDomainObjPtr
vm, virConnectPtr conn)
Hmm, you also did some reorganization as part of moving functions
between files (this appears early in the old qemu_driver.c, but late in
the new qemu_process.c). That's okay, it just took me a bit longer to
review.
+
static void
-qemudAutostartConfigs(struct qemud_driver *driver) {
+qemuAutostartDomains(struct qemud_driver *driver)
Interesting mix of renames and code movement in the same patch. I'm not
going to make you split it now, but it might have been nicer as two commits.
-
-/**
- * qemudRemoveDomainStatus
- *
- * remove all state files of a domain from statedir
- *
- * Returns 0 on success
- */
static int
-qemudRemoveDomainStatus(struct qemud_driver *driver,
- virDomainObjPtr vm)
+qemuSecurityInit(struct qemud_driver *driver)
{
- char ebuf[1024];
- char *file = NULL;
-
- if (virAsprintf(&file, "%s/%s.xml", driver->stateDir,
vm->def->name) < 0) {
- virReportOOMError();
- return(-1);
- }
-
- if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
- VIR_WARN("Failed to remove domain XML for %s: %s",
- vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
- VIR_FREE(file);
+ virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName,
+
driver->allowDiskFormatProbing);
+ if (!mgr)
+ goto error;
Especially in places like this, where the rename interfered with the
diff algorithm to produce a difficult-to-read diff.
-
-struct virReconnectDomainData {
It took me a while to see the rename virReconnectDomainData ->
qemuProcessReconnectData.
-static int qemudStartVMDaemon(virConnectPtr conn,
- struct qemud_driver *driver,
...
- if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def,
priv->monConfig,
- priv->monJSON != 0, qemuCmdFlags,
- migrateFrom, stdin_fd,
- vm->current_snapshot, vmop)))
- goto cleanup;
-
- if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0)
- goto cleanup;
Why was the SetCurrentInactive line commented out in the move?
+static void
+qemuProcessReconnect(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque)
+{
+ if (virDomainObjUnref(obj) > 0) {
+ /* We can't get the monitor back, so must kill the VM
+ * to remove danger of it ending up running twice if
+ * user tries to start it again later */
+ qemudShutdownVMDaemon(driver, obj, 0);
Wouldn't this cause a compile error, since you renamed the function to
qemuProcessStop? Indeed:
cc1: warnings being treated as errors
qemu/qemu_process.c: In function 'qemuProcessReconnect':
qemu/qemu_process.c:1854:9: error: implicit declaration of function
'qemudShutdownVMDaemon' [-Wimplicit-function-declaration]
qemu/qemu_process.c:1854:9: error: nested extern declaration of
'qemudShutdownVMDaemon' [-Wnested-externs]
qemu/qemu_process.c: In function 'qemuProcessStart':
qemu/qemu_process.c:1949:9: error: implicit declaration of function
'fstat' [-Wimplicit-function-declaration]
qemu/qemu_process.c:1949:9: error: nested extern declaration of 'fstat'
[-Wnested-externs]
qemu/qemu_process.c:1954:9: error: implicit declaration of function
'S_ISFIFO' [-Wimplicit-function-declaration]
qemu/qemu_process.c:1954:9: error: nested extern declaration of
'S_ISFIFO' [-Wnested-externs]
Also, you're still missing a clean 'make syntax-check' run; at least
po/POTFILES.in needs work.
I like the idea of this patch, but it needs a bit more work.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org