We need to be able to restore a domain which we already locked and started a job for it without undoing these steps. This patch factors out internals of qemudDomainRestore into separate functions which work for locked objects. --- src/qemu/qemu_driver.c | 213 ++++++++++++++++++++++++++++++------------------ 1 files changed, 135 insertions(+), 78 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a519c02..d048570 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6014,29 +6014,43 @@ child_cleanup: _exit(exit_code); } -/* TODO: check seclabel restore */ -static int qemudDomainRestore(virConnectPtr conn, - const char *path) { - struct qemud_driver *driver = conn->privateData; - virDomainDefPtr def = NULL; - virDomainObjPtr vm = NULL; - int fd = -1; +static int qemudDomainImageClose(int fd, pid_t read_pid, int *status) +{ + int ret = 0; + + if (fd != -1) + close(fd); + + if (read_pid != -1) { + /* reap the process that read the file */ + while ((ret = waitpid(read_pid, status, 0)) == -1 + && errno == EINTR) { + /* empty */ + } + } else if (status) { + *status = 0; + } + + return ret; +} + +static int qemudDomainImageOpen(struct qemud_driver *driver, + const char *path, + virDomainDefPtr *ret_def, + struct qemud_save_header *ret_header, + pid_t *ret_read_pid) +{ + int fd; pid_t read_pid = -1; - int ret = -1; - char *xml = NULL; struct qemud_save_header header; - virDomainEventPtr event = NULL; - int intermediatefd = -1; - pid_t intermediate_pid = -1; - int childstat; + char *xml = NULL; + virDomainDefPtr def = NULL; - qemuDriverLock(driver); - /* Verify the header and read the XML */ if ((fd = open(path, O_RDONLY)) < 0) { if ((driver->user == 0) || (getuid() != 0)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read domain image")); - goto cleanup; + goto error; } /* Opening as root failed, but qemu runs as a different user @@ -6045,44 +6059,44 @@ static int qemudDomainRestore(virConnectPtr conn, have the necessary authority to read the file. */ if ((fd = qemudOpenAsUID(path, driver->user, &read_pid)) < 0) { /* error already reported */ - goto cleanup; + goto error; } } if (saferead(fd, &header, sizeof(header)) != sizeof(header)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read qemu header")); - goto cleanup; + goto error; } if (memcmp(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)) != 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("image magic is incorrect")); - goto cleanup; + goto error; } if (header.version > QEMUD_SAVE_VERSION) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("image version is not supported (%d > %d)"), header.version, QEMUD_SAVE_VERSION); - goto cleanup; + goto error; } if (header.xml_len <= 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("invalid XML length: %d"), header.xml_len); - goto cleanup; + goto error; } if (VIR_ALLOC_N(xml, header.xml_len) < 0) { virReportOOMError(); - goto cleanup; + goto error; } if (saferead(fd, xml, header.xml_len) != header.xml_len) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML")); - goto cleanup; + goto error; } /* Create a domain from this XML */ @@ -6090,35 +6104,53 @@ static int qemudDomainRestore(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to parse XML")); - goto cleanup; + goto error; } - if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) - goto cleanup; + VIR_FREE(xml); - if (!(vm = virDomainAssignDef(driver->caps, - &driver->domains, - def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); - goto cleanup; - } - def = NULL; + *ret_def = def; + *ret_header = header; + *ret_read_pid = read_pid; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) - goto cleanup; + return fd; + +error: + virDomainDefFree(def); + VIR_FREE(xml); + qemudDomainImageClose(fd, read_pid, NULL); + + return -1; +} + +/* TODO: check seclabel restore */ +static int qemudDomainImageStartVM(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + int fd, + pid_t read_pid, + const struct qemud_save_header *header, + const char *path) +{ + int ret = -1; + virDomainEventPtr event; + int intermediatefd = -1; + pid_t intermediate_pid = -1; + int childstat; + int wait_ret; + int status; - if (header.version == 2) { + if (header->version == 2) { const char *intermediate_argv[3] = { NULL, "-dc", NULL }; - const char *prog = qemudSaveCompressionTypeToString(header.compressed); + const char *prog = qemudSaveCompressionTypeToString(header->compressed); if (prog == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("Invalid compressed save format %d"), - header.compressed); - goto endjob; + header->compressed); + goto out; } - if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { + if (header->compressed != QEMUD_SAVE_FORMAT_RAW) { intermediate_argv[0] = prog; intermediatefd = fd; fd = -1; @@ -6127,29 +6159,27 @@ static int qemudDomainRestore(virConnectPtr conn, qemuReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to start decompression binary %s"), intermediate_argv[0]); - goto endjob; + goto out; } } } + /* Set the migration source and start it up. */ ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd); + if (intermediate_pid != -1) { /* Wait for intermediate process to exit */ while (waitpid(intermediate_pid, &childstat, 0) == -1 && - errno == EINTR); + errno == EINTR) { + /* empty */ + } } if (intermediatefd != -1) close(intermediatefd); - close(fd); + + wait_ret = qemudDomainImageClose(fd, read_pid, &status); fd = -1; if (read_pid != -1) { - int wait_ret; - int status; - /* reap the process that read the file */ - while (((wait_ret = waitpid(read_pid, &status, 0)) == -1) - && (errno == EINTR)) { - /* empty */ - } read_pid = -1; if (wait_ret == -1) { virReportSystemError(errno, @@ -6171,22 +6201,19 @@ static int qemudDomainRestore(virConnectPtr conn, } } } - if (ret < 0) { - if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; - } - goto endjob; - } + + if (ret < 0) + goto out; event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); + if (event) + qemuDomainEventQueue(driver, event); + /* If it was running before, resume it now. */ - if (header.was_running) { + if (header->was_running) { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStartCPUs(priv->mon, conn) < 0) { @@ -6194,38 +6221,68 @@ static int qemudDomainRestore(virConnectPtr conn, qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); qemuDomainObjExitMonitorWithDriver(driver,vm); - goto endjob; + goto out; } qemuDomainObjExitMonitorWithDriver(driver, vm); vm->state = VIR_DOMAIN_RUNNING; if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) { VIR_WARN("Failed to save status on vm %s", vm->def->name); - goto endjob; + goto out; } } + ret = 0; -endjob: - if (vm && - qemuDomainObjEndJob(vm) == 0) +out: + return ret; +} + +static int qemudDomainRestore(virConnectPtr conn, + const char *path) { + struct qemud_driver *driver = conn->privateData; + virDomainDefPtr def = NULL; + virDomainObjPtr vm = NULL; + int fd = -1; + pid_t read_pid = -1; + int ret = -1; + struct qemud_save_header header; + + qemuDriverLock(driver); + + fd = qemudDomainImageOpen(driver, path, &def, &header, &read_pid); + if (fd < 0) + goto cleanup; + + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; + + if (!(vm = virDomainAssignDef(driver->caps, + &driver->domains, + def, true))) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("failed to assign new VM")); + goto cleanup; + } + def = NULL; + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + goto cleanup; + + ret = qemudDomainImageStartVM(conn, driver, vm, fd, + read_pid, &header, path); + + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; + else if (ret < 0 && !vm->persistent) { + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; + } cleanup: virDomainDefFree(def); - VIR_FREE(xml); - if (fd != -1) - close(fd); - if (read_pid != 0) { - /* reap the process that read the file */ - while ((waitpid(read_pid, NULL, 0) == -1) - && (errno == EINTR)) { - /* empty */ - } - } + qemudDomainImageClose(fd, read_pid, NULL); if (vm) virDomainObjUnlock(vm); - if (event) - qemuDomainEventQueue(driver, event); qemuDriverUnlock(driver); return ret; } -- 1.7.1

We need to be able to assign new def to an existing virDomainObj which is already locked. This patch factors out the relevant code from virDomainAssignDef into virDomainObjAssignDef. --- src/conf/domain_conf.c | 36 +++++++++++++++++++++--------------- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 20c9c51..c77ee59 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -808,6 +808,26 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps) return domain; } +void virDomainObjAssignDef(virDomainObjPtr domain, + const virDomainDefPtr def, + bool live) +{ + if (!virDomainObjIsActive(domain)) { + if (live) { + /* save current configuration to be restored on domain shutdown */ + if (!domain->newDef) + domain->newDef = domain->def; + domain->def = def; + } else { + virDomainDefFree(domain->def); + domain->def = def; + } + } else { + virDomainDefFree(domain->newDef); + domain->newDef = def; + } +} + virDomainObjPtr virDomainAssignDef(virCapsPtr caps, virDomainObjListPtr doms, const virDomainDefPtr def, @@ -817,21 +837,7 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps, char uuidstr[VIR_UUID_STRING_BUFLEN]; if ((domain = virDomainFindByUUID(doms, def->uuid))) { - if (!virDomainObjIsActive(domain)) { - if (live) { - /* save current configuration to be restored on domain shutdown */ - if (!domain->newDef) - domain->newDef = domain->def; - domain->def = def; - } else { - virDomainDefFree(domain->def); - domain->def = def; - } - } else { - virDomainDefFree(domain->newDef); - domain->newDef = def; - } - + virDomainObjAssignDef(domain, def, live); return domain; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a7206d3..9f87fc5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -954,6 +954,9 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps, virDomainObjListPtr doms, const virDomainDefPtr def, bool live); +void virDomainObjAssignDef(virDomainObjPtr domain, + const virDomainDefPtr def, + bool live); void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjPtr dom); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bdeab0f..1594a08 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -215,6 +215,7 @@ virDomainSnapshotObjUnref; virDomainSnapshotDefParseString; virDomainSnapshotDefFormat; virDomainSnapshotAssignDef; +virDomainObjAssignDef; # domain_event.h -- 1.7.1

We need a common internal function for starting managed domains to be used during autostart. This patch factors out relevant code from qemudDomainStart into qemudDomainObjStart and makes it use the refactored code for domain restore instead of calling qemudDomainRestore API directly. --- src/qemu/qemu_driver.c | 123 ++++++++++++++++++++++++++++++++--------------- 1 files changed, 84 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d048570..8f5cae1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -143,6 +143,10 @@ static void qemuDomainEventFlush(int timer, void *opaque); static void qemuDomainEventQueue(struct qemud_driver *driver, virDomainEventPtr event); +static int qemudDomainObjStart(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm); + static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, @@ -6287,6 +6291,47 @@ cleanup: return ret; } +static int qemudDomainObjRestore(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + const char *path) +{ + virDomainDefPtr def = NULL; + int fd = -1; + pid_t read_pid = -1; + int ret = -1; + struct qemud_save_header header; + + fd = qemudDomainImageOpen(driver, path, &def, &header, &read_pid); + if (fd < 0) + goto cleanup; + + if (STRNEQ(vm->def->name, def->name) || + memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) { + char vm_uuidstr[VIR_UUID_STRING_BUFLEN]; + char def_uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, vm_uuidstr); + virUUIDFormat(def->uuid, def_uuidstr); + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot restore domain '%s' uuid %s from a file" + " which belongs to domain '%s' uuid %s"), + vm->def->name, vm_uuidstr, + def->name, def_uuidstr); + goto cleanup; + } + + virDomainObjAssignDef(vm, def, true); + def = NULL; + + ret = qemudDomainImageStartVM(conn, driver, vm, fd, + read_pid, &header, path); + +cleanup: + virDomainDefFree(def); + qemudDomainImageClose(fd, read_pid, NULL); + return ret; +} + static char *qemudVMDumpXML(struct qemud_driver *driver, virDomainObjPtr vm, @@ -6561,12 +6606,49 @@ static int qemudNumDefinedDomains(virConnectPtr conn) { } +static int qemudDomainObjStart(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm) +{ + int ret = -1; + char *managed_save; + + /* + * If there is a managed saved state restore it instead of starting + * from scratch. In any case the old state is removed. + */ + managed_save = qemuDomainManagedSavePath(driver, vm); + if ((managed_save) && (virFileExists(managed_save))) { + ret = qemudDomainObjRestore(conn, driver, vm, managed_save); + + if (unlink(managed_save) < 0) { + VIR_WARN("Failed to remove the managed state %s", managed_save); + } + + if (ret == 0) + goto cleanup; + } + + ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1); + if (ret != -1) { + virDomainEventPtr event = + virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_BOOTED); + if (event) + qemuDomainEventQueue(driver, event); + } + +cleanup: + VIR_FREE(managed_save); + return ret; +} + static int qemudDomainStart(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; virDomainEventPtr event = NULL; - char *managed_save = NULL; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6588,50 +6670,13 @@ static int qemudDomainStart(virDomainPtr dom) { goto endjob; } - /* - * If there is a managed saved state restore it instead of starting - * from scratch. In any case the old state is removed. - */ - managed_save = qemuDomainManagedSavePath(driver, vm); - if ((managed_save) && (virFileExists(managed_save))) { - /* - * We should still have a reference left to vm but - * one should check for 0 anyway - */ - if (qemuDomainObjEndJob(vm) == 0) { - vm = NULL; - goto cleanup; - } - - virDomainObjUnlock(vm); - qemuDriverUnlock(driver); - ret = qemudDomainRestore(dom->conn, managed_save); - - if (unlink(managed_save) < 0) { - VIR_WARN("Failed to remove the managed state %s", managed_save); - } - - if (ret == 0) { - /* qemudDomainRestore should have sent the Started/Restore event */ - VIR_FREE(managed_save); - return(ret); - } - qemuDriverLock(driver); - virDomainObjLock(vm); - } - - ret = qemudStartVMDaemon(dom->conn, driver, vm, NULL, -1); - if (ret != -1) - event = virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_BOOTED); + ret = qemudDomainObjStart(dom->conn, driver, vm); endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; cleanup: - VIR_FREE(managed_save); if (vm) virDomainObjUnlock(vm); if (event) -- 1.7.1

--- src/qemu/qemu_driver.c | 34 ++++++++++++++++++---------------- 1 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8f5cae1..d40e29a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -626,29 +626,31 @@ qemuAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq { virDomainObjPtr vm = payload; struct qemuAutostartData *data = opaque; + virErrorPtr err; virDomainObjLock(vm); - if (vm->autostart && - !virDomainObjIsActive(vm)) { - int ret; - - virResetLastError(); - ret = qemudStartVMDaemon(data->conn, data->driver, vm, NULL, -1); - if (ret < 0) { - virErrorPtr err = virGetLastError(); + virResetLastError(); + if (qemuDomainObjBeginJobWithDriver(data->driver, vm) < 0) { + err = virGetLastError(); + VIR_ERROR(_("Failed to start job on VM '%s': %s"), + vm->def->name, + err ? err->message : ""); + } else { + if (vm->autostart && + !virDomainObjIsActive(vm) && + qemudDomainObjStart(data->conn, data->driver, vm) < 0) { + err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, err ? err->message : ""); - } else { - virDomainEventPtr event = - virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_BOOTED); - if (event) - qemuDomainEventQueue(data->driver, event); } + + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; } - virDomainObjUnlock(vm); + + if (vm) + virDomainObjUnlock(vm); } static void -- 1.7.1