[libvirt] [PATCH v3 0/7] qemu: Add support for -incoming defer

Traditionally, we pass incoming migration URI on QEMU command line, which has some drawbacks. Depending on the URI QEMU may initialize its migration state immediately without giving us a chance to set any additional migration parameters (this applies mainly for fd: URIs). For some URIs the monitor may be completely blocked from the beginning until migration is finished, which means we may be stuck in qmp_capabilities command without being able to send any QMP commands. QEMU solved this by introducing "defer" parameter for -incoming command line option. This will tell QEMU to prepare for an incoming migration while the actual incoming URI is sent using migrate-incoming QMP command. Before calling this command we can normally talk to the monitor and even set any migration parameters which will be honored by the incoming migration. Jiri Denemark (7): qemu: Introduce qemuProcessInit qemu: Introduce qemuProcessLaunch qemu: Introduce qemuProcessFinishStartup qemu: Separate incoming URI generation from qemuMigrationPrepareAny qemu: Kill QEMU process if Prepare phase fails qemu: Skip starting NBD servers for offline migration qemu: Use qemuProcessLaunch in migration Prepare phase src/qemu/qemu_migration.c | 247 +++++++++++++---------- src/qemu/qemu_process.c | 489 +++++++++++++++++++++++++++++----------------- src/qemu/qemu_process.h | 20 ++ 3 files changed, 466 insertions(+), 290 deletions(-) -- 2.6.3

qemuProcessStart is going to be split in three parts: qemuProcessInit, qemuProcessLaunch, and qemuProcessFinish so that migration Prepare phase can insert additional code in the process. qemuProcessStart will be a small wrapper for all other callers. qemuProcessInit prepares the domain up to the point when priv->qemuCaps is initialized. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 3: - call qemuProcessStop internally if necessary instead of pushing this responsibility to the caller Version 2: - avoid calling qemuProcessStop when starting fails with "VM is already active" src/qemu/qemu_process.c | 133 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 4 ++ 2 files changed, 92 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 68542e7..83a17ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4476,6 +4476,90 @@ qemuProcessMakeDir(virQEMUDriverPtr driver, } +/** + * qemuProcessInit: + * + * Prepares the domain up to the point when priv->qemuCaps is initialized. The + * function calls qemuProcessStop when needed. + * + * Returns 0 on success, -1 on error. + */ +int +qemuProcessInit(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool migration) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + int stopFlags; + int ret = -1; + + VIR_DEBUG("vm=%p name=%s id=%d migration=%d", + vm, vm->def->name, vm->def->id, migration); + + VIR_DEBUG("Beginning VM startup process"); + + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("VM is already active")); + goto cleanup; + } + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto stop; + + /* Some things, paths, ... are generated here and we want them to persist. + * Fill them in prior to setting the domain def as transient. */ + VIR_DEBUG("Generating paths"); + + if (qemuPrepareNVRAM(cfg, vm, migration) < 0) + goto stop; + + /* Do this upfront, so any part of the startup process can add + * runtime state to vm->def that won't be persisted. This let's us + * report implicit runtime defaults in the XML, like vnc listen/socket + */ + VIR_DEBUG("Setting current domain def as transient"); + if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0) + goto stop; + + vm->def->id = qemuDriverAllocateID(driver); + qemuDomainSetFakeReboot(driver, vm, false); + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); + + if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + + /* Run an early hook to set-up missing devices */ + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_PREPARE, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto stop; + + VIR_DEBUG("Determining emulator version"); + virObjectUnref(priv->qemuCaps); + if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, + vm->def->emulator, + vm->def->os.machine))) + goto stop; + + ret = 0; + + cleanup: + virObjectUnref(cfg); + virObjectUnref(caps); + return ret; + + stop: + stopFlags = VIR_QEMU_PROCESS_STOP_NO_RELABEL; + if (migration) + stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stopFlags); + goto cleanup; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4498,7 +4582,7 @@ int qemuProcessStart(virConnectPtr conn, size_t i; char *nodeset = NULL; unsigned int stop_flags; - virQEMUDriverConfigPtr cfg; + virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; unsigned int hostdev_flags = 0; size_t nnicindexes = 0; @@ -4516,6 +4600,9 @@ int qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, -1); + if (qemuProcessInit(driver, vm, !!migrateFrom)) + goto cleanup; + cfg = virQEMUDriverGetConfig(driver); /* From now on until domain security labeling is done: @@ -4534,53 +4621,9 @@ int qemuProcessStart(virConnectPtr conn, /* We don't increase cfg's reference counter here. */ hookData.cfg = cfg; - VIR_DEBUG("Beginning VM startup process"); - - if (virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("VM is already active")); - virObjectUnref(cfg); - return -1; - } - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto error; - /* Some things, paths, ... are generated here and we want them to persist. - * Fill them in prior to setting the domain def as transient. */ - VIR_DEBUG("Generating paths"); - - if (qemuPrepareNVRAM(cfg, vm, !!migrateFrom) < 0) - goto error; - - /* Do this upfront, so any part of the startup process can add - * runtime state to vm->def that won't be persisted. This let's us - * report implicit runtime defaults in the XML, like vnc listen/socket - */ - VIR_DEBUG("Setting current domain def as transient"); - if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0) - goto error; - - vm->def->id = qemuDriverAllocateID(driver); - qemuDomainSetFakeReboot(driver, vm, false); - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); - - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) - driver->inhibitCallback(true, driver->inhibitOpaque); - - /* Run an early hook to set-up missing devices */ - if (qemuProcessStartHook(driver, vm, - VIR_HOOK_QEMU_OP_PREPARE, - VIR_HOOK_SUBOP_BEGIN) < 0) - goto error; - - VIR_DEBUG("Determining emulator version"); - virObjectUnref(priv->qemuCaps); - if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - vm->def->emulator, - vm->def->os.machine))) - goto error; - /* network devices must be "prepared" before hostdevs, because * setting up a network device might create a new hostdev that * will need to be setup. diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index dcb7e28..3948caf 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -81,6 +81,10 @@ int qemuProcessStart(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags); +int qemuProcessInit(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool migration); + typedef enum { VIR_QEMU_PROCESS_STOP_MIGRATED = 1 << 0, VIR_QEMU_PROCESS_STOP_NO_RELABEL = 1 << 1, -- 2.6.3

Once qemuProcessInit was called, qemuProcessLaunch will launch a new QEMU process with stopped virtual CPUs. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: ACKed in version 2 Version 3: - no change Version 2: - save status before running VIR_HOOK_QEMU_OP_STARTED hook - call virSecurityManagerSetAllLabel even if !incoming - move qemuProcessStop out of qemuProcessLaunch to avoid it being called twice src/qemu/qemu_process.c | 315 ++++++++++++++++++++++++++++-------------------- src/qemu/qemu_process.h | 9 ++ 2 files changed, 195 insertions(+), 129 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 83a17ce..1b3cbc0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4560,16 +4560,28 @@ qemuProcessInit(virQEMUDriverPtr driver, } -int qemuProcessStart(virConnectPtr conn, - virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob, - const char *migrateFrom, - int migrateFd, - const char *migratePath, - virDomainSnapshotObjPtr snapshot, - virNetDevVPortProfileOp vmop, - unsigned int flags) +/** + * qemuProcessLaunch: + * + * Launch a new QEMU process with stopped virtual CPUs. + * + * The caller is supposed to call qemuProcessStop with appropriate + * flags in case of failure. + * + * Returns 0 on success, + * -1 on error which happened before devices were labeled and thus + * there is no need to restore them, + * -2 on error requesting security labels to be restored. + */ +int +qemuProcessLaunch(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuProcessIncomingDefPtr incoming, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags) { int ret = -1; int rv; @@ -4581,18 +4593,22 @@ int qemuProcessStart(virConnectPtr conn, struct qemuProcessHookData hookData; size_t i; char *nodeset = NULL; - unsigned int stop_flags; - virQEMUDriverConfigPtr cfg = NULL; + virQEMUDriverConfigPtr cfg; virCapsPtr caps = NULL; unsigned int hostdev_flags = 0; size_t nnicindexes = 0; int *nicindexes = NULL; - qemuProcessIncomingDefPtr incoming = NULL; - VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s migrateFd=%d " - "migratePath=%s snapshot=%p vmop=%d flags=0x%x", - vm, vm->def->name, vm->def->id, asyncJob, NULLSTR(migrateFrom), - migrateFd, NULLSTR(migratePath), snapshot, vmop, flags); + VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d " + "incoming.launchURI=%s incoming.deferredURI=%s " + "incoming.fd=%d incoming.path=%s " + "snapshot=%p vmop=%d flags=0x%x", + vm, vm->def->name, vm->def->id, asyncJob, + NULLSTR(incoming ? incoming->launchURI : NULL), + NULLSTR(incoming ? incoming->deferredURI : NULL), + incoming ? incoming->fd : -1, + NULLSTR(incoming ? incoming->path : NULL), + snapshot, vmop, flags); /* Okay, these are just internal flags, * but doesn't hurt to check */ @@ -4600,21 +4616,8 @@ int qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, -1); - if (qemuProcessInit(driver, vm, !!migrateFrom)) - goto cleanup; - cfg = virQEMUDriverGetConfig(driver); - /* From now on until domain security labeling is done: - * if any operation fails and we goto cleanup, we must not - * restore any security label as we would overwrite labels - * we did not set. */ - stop_flags = VIR_QEMU_PROCESS_STOP_NO_RELABEL; - /* If we fail while doing incoming migration, then we must not - * relabel, as the source is still using the files. */ - if (migrateFrom) - stop_flags |= VIR_QEMU_PROCESS_STOP_MIGRATED; - hookData.conn = conn; hookData.vm = vm; hookData.driver = driver; @@ -4622,7 +4625,7 @@ int qemuProcessStart(virConnectPtr conn, hookData.cfg = cfg; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto error; + goto cleanup; /* network devices must be "prepared" before hostdevs, because * setting up a network device might create a new hostdev that @@ -4630,35 +4633,35 @@ int qemuProcessStart(virConnectPtr conn, */ VIR_DEBUG("Preparing network devices"); if (qemuNetworkPrepareDevices(vm->def) < 0) - goto error; + goto cleanup; /* Must be run before security labelling */ VIR_DEBUG("Preparing host devices"); if (!cfg->relaxedACS) hostdev_flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; - if (!migrateFrom) + if (!incoming) hostdev_flags |= VIR_HOSTDEV_COLD_BOOT; if (qemuHostdevPrepareDomainDevices(driver, vm->def, priv->qemuCaps, hostdev_flags) < 0) - goto error; + goto cleanup; VIR_DEBUG("Preparing chr devices"); if (virDomainChrDefForeach(vm->def, true, qemuProcessPrepareChardevDevice, NULL) < 0) - goto error; + goto cleanup; VIR_DEBUG("Checking domain and device security labels"); if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0) - goto error; + goto cleanup; /* If you are using a SecurityDriver with dynamic labelling, then generate a security label for isolation */ VIR_DEBUG("Generating domain security label (if required)"); if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) { virDomainAuditSecurityLabel(vm, false); - goto error; + goto cleanup; } virDomainAuditSecurityLabel(vm, true); @@ -4667,14 +4670,14 @@ int qemuProcessStart(virConnectPtr conn, char *hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]); if (!hugepagePath) - goto error; + goto cleanup; if (virSecurityManagerSetHugepages(driver->securityManager, vm->def, hugepagePath) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to set huge path in security driver")); VIR_FREE(hugepagePath); - goto error; + goto cleanup; } VIR_FREE(hugepagePath); } @@ -4687,18 +4690,18 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Setting up ports for graphics"); if (qemuProcessSetupGraphics(driver, vm) < 0) - goto error; + goto cleanup; if (virFileMakePath(cfg->logDir) < 0) { virReportSystemError(errno, _("cannot create log directory %s"), cfg->logDir); - goto error; + goto cleanup; } VIR_DEBUG("Creating domain log file"); if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0) - goto error; + goto cleanup; if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) { VIR_DEBUG("Checking for KVM availability"); @@ -4707,15 +4710,15 @@ int qemuProcessStart(virConnectPtr conn, _("Domain requires KVM, but it is not available. " "Check that virtualization is enabled in the host BIOS, " "and host configuration is setup to load the kvm modules.")); - goto error; + goto cleanup; } } if (!qemuValidateCpuMax(vm->def, priv->qemuCaps)) - goto error; + goto cleanup; if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) - goto error; + goto cleanup; /* Get the advisory nodeset from numad if 'placement' of * either <vcpu> or <numatune> is 'auto'. @@ -4724,48 +4727,48 @@ int qemuProcessStart(virConnectPtr conn, nodeset = virNumaGetAutoPlacementAdvice(vm->def->vcpus, virDomainDefGetMemoryActual(vm->def)); if (!nodeset) - goto error; + goto cleanup; VIR_DEBUG("Nodeset returned from numad: %s", nodeset); if (virBitmapParse(nodeset, 0, &priv->autoNodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto error; + goto cleanup; if (!(priv->autoCpuset = virCapabilitiesGetCpusForNodemask(caps, priv->autoNodeset))) - goto error; + goto cleanup; } - if (!migrateFrom && !snapshot && + if (!incoming && !snapshot && virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) - goto error; + goto cleanup; /* "volume" type disk's source must be translated before * cgroup and security setting. */ for (i = 0; i < vm->def->ndisks; i++) { if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) - goto error; + goto cleanup; } if (qemuDomainCheckDiskPresence(driver, vm, flags & VIR_QEMU_PROCESS_START_COLD) < 0) - goto error; + goto cleanup; if (vm->def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parameter 'min_guarantee' " "not supported by QEMU.")); - goto error; + goto cleanup; } if (VIR_ALLOC(priv->monConfig) < 0) - goto error; + goto cleanup; VIR_DEBUG("Preparing monitor state"); if (qemuProcessPrepareMonitorChr(cfg, priv->monConfig, vm->def->name) < 0) - goto error; + goto cleanup; priv->monJSON = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON); priv->monError = false; @@ -4776,7 +4779,7 @@ int qemuProcessStart(virConnectPtr conn, if (!(priv->pidfile = virPidFileBuildPath(cfg->stateDir, vm->def->name))) { virReportSystemError(errno, "%s", _("Failed to build pidfile path.")); - goto error; + goto cleanup; } if (unlink(priv->pidfile) < 0 && @@ -4784,7 +4787,7 @@ int qemuProcessStart(virConnectPtr conn, virReportSystemError(errno, _("Cannot remove stale PID file %s"), priv->pidfile); - goto error; + goto cleanup; } /* @@ -4797,14 +4800,7 @@ int qemuProcessStart(virConnectPtr conn, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { VIR_DEBUG("Assigning domain PCI addresses"); if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) - goto error; - } - - if (migrateFrom) { - incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, - migrateFd, migratePath); - if (!incoming) - goto error; + goto cleanup; } VIR_DEBUG("Building emulator command line"); @@ -4816,7 +4812,7 @@ int qemuProcessStart(virConnectPtr conn, qemuCheckFips(), priv->autoNodeset, &nnicindexes, &nicindexes))) - goto error; + goto cleanup; if (incoming && incoming->fd != -1) virCommandPassFD(cmd, incoming->fd, 0); @@ -4827,13 +4823,13 @@ int qemuProcessStart(virConnectPtr conn, */ if (qemuProcessMakeDir(driver, vm, cfg->libDir) < 0 || qemuProcessMakeDir(driver, vm, cfg->channelTargetDir) < 0) - goto error; + goto cleanup; /* now that we know it is about to start call the hook if present */ if (qemuProcessStartHook(driver, vm, VIR_HOOK_QEMU_OP_START, VIR_HOOK_SUBOP_BEGIN) < 0) - goto error; + goto cleanup; qemuLogOperation(vm, "starting up", logfile, cmd); @@ -4850,7 +4846,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Setting up raw IO"); if (qemuProcessSetupRawIO(driver, vm, cmd) < 0) - goto error; + goto cleanup; virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses); @@ -4860,7 +4856,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Setting up security labelling"); if (virSecurityManagerSetChildProcessLabel(driver->securityManager, vm->def, cmd) < 0) - goto error; + goto cleanup; virCommandSetOutputFD(cmd, &logfile); virCommandSetErrorFD(cmd, &logfile); @@ -4870,7 +4866,7 @@ int qemuProcessStart(virConnectPtr conn, virCommandRequireHandshake(cmd); if (virSecurityManagerPreFork(driver->securityManager) < 0) - goto error; + goto cleanup; rv = virCommandRun(cmd, NULL); virSecurityManagerPostFork(driver->securityManager); @@ -4890,37 +4886,38 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Writing early domain status to disk"); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Waiting for handshake from child"); if (virCommandHandshakeWait(cmd) < 0) { /* Read errors from child that occurred between fork and exec. */ qemuProcessReadChildErrors(driver, vm, pos); - goto error; + goto cleanup; } VIR_DEBUG("Setting up domain cgroup (if required)"); if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0) - goto error; + goto cleanup; /* This must be done after cgroup placement to avoid resetting CPU * affinity */ if (!vm->def->cputune.emulatorpin && qemuProcessInitCpuAffinity(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, migratePath) < 0) - goto error; + vm->def, + incoming ? incoming->path : NULL) < 0) + goto cleanup; /* Security manager labeled all devices, therefore - * if any operation from now on fails and we goto cleanup, - * where virSecurityManagerRestoreAllLabel() is called - * (hidden under qemuProcessStop) we need to restore labels. */ - stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL; + * if any operation from now on fails, we need to ask the caller to + * restore labels. + */ + ret = -2; - if (migrateFd != -1) { + if (incoming && incoming->fd != -1) { /* if there's an fd to migrate from, and it's a pipe, put the * proper security label on it */ @@ -4928,43 +4925,44 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("setting security label on pipe used for migration"); - if (fstat(migrateFd, &stdin_sb) < 0) { + if (fstat(incoming->fd, &stdin_sb) < 0) { virReportSystemError(errno, - _("cannot stat fd %d"), migrateFd); - goto error; + _("cannot stat fd %d"), incoming->fd); + goto cleanup; } if (S_ISFIFO(stdin_sb.st_mode) && - virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, migrateFd) < 0) - goto error; + virSecurityManagerSetImageFDLabel(driver->securityManager, + vm->def, incoming->fd) < 0) + goto cleanup; } VIR_DEBUG("Labelling done, completing handshake to child"); if (virCommandHandshakeNotify(cmd) < 0) - goto error; + goto cleanup; VIR_DEBUG("Handshake complete, child running"); if (rv == -1) /* The VM failed to start; tear filters before taps */ virDomainConfVMNWFilterTeardown(vm); if (rv == -1) /* The VM failed to start */ - goto error; + goto cleanup; VIR_DEBUG("Setting cgroup for emulator (if required)"); if (qemuSetupCgroupForEmulator(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting affinity of emulator threads"); if (qemuProcessSetEmulatorAffinity(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Waiting for monitor to show up"); if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->qemuCaps, pos) < 0) - goto error; + goto cleanup; /* Failure to connect to agent shouldn't be fatal */ if ((rv = qemuConnectAgent(driver, vm)) < 0) { if (rv == -2) - goto error; + goto cleanup; VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name); @@ -4974,50 +4972,50 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Detecting if required emulator features are present"); if (!qemuProcessVerifyGuestCPU(driver, vm, asyncJob)) - goto error; + goto cleanup; VIR_DEBUG("Setting up post-init cgroup restrictions"); if (qemuSetupCpusetMems(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; VIR_DEBUG("Detecting IOThread PIDs"); if (qemuProcessDetectIOThreadPIDs(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting cgroup for each VCPU (if required)"); if (qemuSetupCgroupForVcpu(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting cgroup for each IOThread (if required)"); if (qemuSetupCgroupForIOThreads(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting VCPU affinities"); if (qemuProcessSetVcpuAffinities(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting affinity of IOThread threads"); if (qemuProcessSetIOThreadsAffinity(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting scheduler parameters"); if (qemuProcessSetSchedulers(vm) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting any required VM passwords"); if (qemuProcessInitPasswords(conn, driver, vm, asyncJob) < 0) - goto error; + goto cleanup; /* If we have -device, then addresses are assigned explicitly. * If not, then we have to detect dynamic ones here */ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { VIR_DEBUG("Determining domain device PCI addresses"); if (qemuProcessInitPCIAddresses(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; } /* set default link states */ @@ -5025,35 +5023,102 @@ int qemuProcessStart(virConnectPtr conn, * enter the monitor */ VIR_DEBUG("Setting network link states"); if (qemuProcessSetLinkStates(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; VIR_DEBUG("Fetching list of active devices"); if (qemuDomainUpdateDeviceList(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; VIR_DEBUG("Updating info of memory devices"); if (qemuDomainUpdateMemoryDeviceInfo(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; VIR_DEBUG("Setting initial memory amount"); if (qemuProcessSetupBalloon(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; /* Since CPUs were not started yet, the balloon could not return the memory * to the host and thus cur_balloon needs to be updated so that GetXMLdesc * and friends return the correct size in case they can't grab the job */ if (!incoming && !snapshot && qemuProcessRefreshBalloonState(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; VIR_DEBUG("Detecting actual memory size for video device"); if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) - goto error; + goto cleanup; + + if (flags & VIR_QEMU_PROCESS_START_AUTODESTROY && + qemuProcessAutoDestroyAdd(driver, vm, conn) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virCommandFree(cmd); + VIR_FORCE_CLOSE(logfile); + virObjectUnref(cfg); + virObjectUnref(caps); + VIR_FREE(nicindexes); + VIR_FREE(nodeset); + return ret; +} + + +int +qemuProcessStart(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + const char *migrateFrom, + int migrateFd, + const char *migratePath, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuProcessIncomingDefPtr incoming = NULL; + unsigned int stopFlags; + bool relabel = false; + int ret = -1; + int rv; + + VIR_DEBUG("conn=%p driver=%p vm=%p name=%s id=%d asyncJob=%s " + "migrateFrom=%s migrateFd=%d migratePath=%s " + "snapshot=%p vmop=%d flags=0x%x", + conn, driver, vm, vm->def->name, vm->def->id, + qemuDomainAsyncJobTypeToString(asyncJob), + NULLSTR(migrateFrom), migrateFd, NULLSTR(migratePath), + snapshot, vmop, flags); + + virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | + VIR_QEMU_PROCESS_START_PAUSED | + VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup); + + if (qemuProcessInit(driver, vm, !!migrateFrom) < 0) + goto cleanup; + + if (migrateFrom) { + incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, + migrateFd, migratePath); + if (!incoming) + goto stop; + } + + if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, + snapshot, vmop, flags)) < 0) { + if (rv == -1) + relabel = true; + goto stop; + } + relabel = true; if (incoming && incoming->deferredURI && qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) - goto error; + goto stop; if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) { VIR_DEBUG("Starting domain CPUs"); @@ -5064,7 +5129,7 @@ int qemuProcessStart(virConnectPtr conn, if (virGetLastError() == NULL) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); - goto error; + goto stop; } } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, @@ -5073,19 +5138,14 @@ int qemuProcessStart(virConnectPtr conn, VIR_DOMAIN_PAUSED_USER); } - if (flags & VIR_QEMU_PROCESS_START_AUTODESTROY && - qemuProcessAutoDestroyAdd(driver, vm, conn) < 0) - goto error; - VIR_DEBUG("Writing domain status to disk"); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto error; + goto stop; - /* finally we can call the 'started' hook script if any */ if (qemuProcessStartHook(driver, vm, VIR_HOOK_QEMU_OP_STARTED, VIR_HOOK_SUBOP_BEGIN) < 0) - goto error; + goto stop; /* Keep watching qemu log for errors during incoming migration, otherwise * unset reporting errors from qemu log. */ @@ -5095,20 +5155,17 @@ int qemuProcessStart(virConnectPtr conn, ret = 0; cleanup: - virCommandFree(cmd); - VIR_FORCE_CLOSE(logfile); virObjectUnref(cfg); - virObjectUnref(caps); - VIR_FREE(nicindexes); - VIR_FREE(nodeset); qemuProcessIncomingDefFree(incoming); return ret; - error: - /* We jump here if we failed to start the VM for any reason, or - * if we failed to initialize the now running VM. kill it off and - * pretend we never started it */ - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags); + stop: + stopFlags = 0; + if (!relabel) + stopFlags |= VIR_QEMU_PROCESS_STOP_NO_RELABEL; + if (migrateFrom) + stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stopFlags); goto cleanup; } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 3948caf..54009c5 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -85,6 +85,15 @@ int qemuProcessInit(virQEMUDriverPtr driver, virDomainObjPtr vm, bool migration); +int qemuProcessLaunch(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuProcessIncomingDefPtr incoming, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags); + typedef enum { VIR_QEMU_PROCESS_STOP_MIGRATED = 1 << 0, VIR_QEMU_PROCESS_STOP_NO_RELABEL = 1 << 1, -- 2.6.3

Finishes starting a new domain launched by qemuProcessLaunch. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 3: - propagate asyncJob into qemuProcessFinishStartup to avoid effectively reverting 668a0fe fix from Jan Version 2: - save status before running VIR_HOOK_QEMU_OP_STARTED hook - rename qemuProcessFinish as qemuProcessFinishStartup - remove unused cfg from qemuProcessStart src/qemu/qemu_process.c | 79 ++++++++++++++++++++++++++++++++----------------- src/qemu/qemu_process.h | 7 +++++ 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b3cbc0..1fe551b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5065,6 +5065,53 @@ qemuProcessLaunch(virConnectPtr conn, } +/** + * qemuProcessFinishStartup: + * + * Finish starting a new domain. + */ +int +qemuProcessFinishStartup(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + bool startCPUs, + virDomainPausedReason pausedReason) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + + if (startCPUs) { + VIR_DEBUG("Starting domain CPUs"); + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_BOOTED, + asyncJob) < 0) { + if (!virGetLastError()) + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("resume operation failed")); + goto cleanup; + } + } else { + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, pausedReason); + } + + VIR_DEBUG("Writing domain status to disk"); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto cleanup; + + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_STARTED, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, @@ -5077,7 +5124,6 @@ qemuProcessStart(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; qemuProcessIncomingDefPtr incoming = NULL; unsigned int stopFlags; @@ -5120,31 +5166,11 @@ qemuProcessStart(virConnectPtr conn, qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) goto stop; - if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) { - VIR_DEBUG("Starting domain CPUs"); - /* Allow the CPUS to start executing */ - if (qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_BOOTED, - asyncJob) < 0) { - if (virGetLastError() == NULL) - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("resume operation failed")); - goto stop; - } - } else { - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, - incoming ? - VIR_DOMAIN_PAUSED_MIGRATION : - VIR_DOMAIN_PAUSED_USER); - } - - VIR_DEBUG("Writing domain status to disk"); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto stop; - - if (qemuProcessStartHook(driver, vm, - VIR_HOOK_QEMU_OP_STARTED, - VIR_HOOK_SUBOP_BEGIN) < 0) + if (qemuProcessFinishStartup(conn, driver, vm, asyncJob, + !(flags & VIR_QEMU_PROCESS_START_PAUSED), + incoming ? + VIR_DOMAIN_PAUSED_MIGRATION : + VIR_DOMAIN_PAUSED_USER) < 0) goto stop; /* Keep watching qemu log for errors during incoming migration, otherwise @@ -5155,7 +5181,6 @@ qemuProcessStart(virConnectPtr conn, ret = 0; cleanup: - virObjectUnref(cfg); qemuProcessIncomingDefFree(incoming); return ret; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 54009c5..978aa3a 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -94,6 +94,13 @@ int qemuProcessLaunch(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags); +int qemuProcessFinishStartup(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + bool startCPUs, + virDomainPausedReason pausedReason); + typedef enum { VIR_QEMU_PROCESS_STOP_MIGRATED = 1 << 0, VIR_QEMU_PROCESS_STOP_NO_RELABEL = 1 << 1, -- 2.6.3

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: ACKed in version 1 Version 3: - no change Version 2: - no change src/qemu/qemu_migration.c | 154 +++++++++++++++++++++++++--------------------- 1 file changed, 85 insertions(+), 69 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0c4c94a..4ab6ab7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3291,6 +3291,80 @@ qemuMigrationPrepareCleanup(virQEMUDriverPtr driver, qemuDomainObjDiscardAsyncJob(driver, vm); } +static char * +qemuMigrationPrepareIncoming(virDomainObjPtr vm, + bool tunnel, + const char *protocol, + const char *listenAddress, + unsigned short port) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *migrateFrom = NULL; + + if (tunnel) { + if (VIR_STRDUP(migrateFrom, "stdio") < 0) + goto cleanup; + } else { + bool encloseAddress = false; + bool hostIPv6Capable = false; + bool qemuIPv6Capable = false; + struct addrinfo *info = NULL; + struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, + .ai_socktype = SOCK_STREAM }; + const char *incFormat; + + if (getaddrinfo("::", NULL, &hints, &info) == 0) { + freeaddrinfo(info); + hostIPv6Capable = true; + } + qemuIPv6Capable = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_IPV6_MIGRATION); + + if (listenAddress) { + if (virSocketAddrNumericFamily(listenAddress) == AF_INET6) { + if (!qemuIPv6Capable) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("qemu isn't capable of IPv6")); + goto cleanup; + } + if (!hostIPv6Capable) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("host isn't capable of IPv6")); + goto cleanup; + } + /* IPv6 address must be escaped in brackets on the cmd line */ + encloseAddress = true; + } else { + /* listenAddress is a hostname or IPv4 */ + } + } else if (qemuIPv6Capable && hostIPv6Capable) { + /* Listen on :: instead of 0.0.0.0 if QEMU understands it + * and there is at least one IPv6 address configured + */ + listenAddress = "::"; + encloseAddress = true; + } else { + listenAddress = "0.0.0.0"; + } + + /* QEMU will be started with + * -incoming protocol:[<IPv6 addr>]:port, + * -incoming protocol:<IPv4 addr>:port, or + * -incoming protocol:<hostname>:port + */ + if (encloseAddress) + incFormat = "%s:[%s]:%d"; + else + incFormat = "%s:%s:%d"; + if (virAsprintf(&migrateFrom, incFormat, + protocol, listenAddress, port) < 0) + goto cleanup; + } + + cleanup: + return migrateFrom; +} + static int qemuMigrationPrepareAny(virQEMUDriverPtr driver, virConnectPtr dconn, @@ -3397,75 +3471,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, } } - if (tunnel) { - /* QEMU will be started with -incoming stdio - * (which qemu_command might convert to exec:cat or fd:n) - */ - if (VIR_STRDUP(migrateFrom, "stdio") < 0) - goto cleanup; - } else { - bool encloseAddress = false; - bool hostIPv6Capable = false; - bool qemuIPv6Capable = false; - virQEMUCapsPtr qemuCaps = NULL; - struct addrinfo *info = NULL; - struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, - .ai_socktype = SOCK_STREAM }; - const char *incFormat; - - if (getaddrinfo("::", NULL, &hints, &info) == 0) { - freeaddrinfo(info); - hostIPv6Capable = true; - } - if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - (*def)->emulator, - (*def)->os.machine))) - goto cleanup; - - qemuIPv6Capable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION); - virObjectUnref(qemuCaps); - - if (listenAddress) { - if (virSocketAddrNumericFamily(listenAddress) == AF_INET6) { - if (!qemuIPv6Capable) { - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("qemu isn't capable of IPv6")); - goto cleanup; - } - if (!hostIPv6Capable) { - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("host isn't capable of IPv6")); - goto cleanup; - } - /* IPv6 address must be escaped in brackets on the cmd line */ - encloseAddress = true; - } else { - /* listenAddress is a hostname or IPv4 */ - } - } else if (qemuIPv6Capable && hostIPv6Capable) { - /* Listen on :: instead of 0.0.0.0 if QEMU understands it - * and there is at least one IPv6 address configured - */ - listenAddress = "::"; - encloseAddress = true; - } else { - listenAddress = "0.0.0.0"; - } - - /* QEMU will be started with - * -incoming protocol:[<IPv6 addr>]:port, - * -incoming protocol:<IPv4 addr>:port, or - * -incoming protocol:<hostname>:port - */ - if (encloseAddress) - incFormat = "%s:[%s]:%d"; - else - incFormat = "%s:%s:%d"; - if (virAsprintf(&migrateFrom, incFormat, - protocol, listenAddress, port) < 0) - goto cleanup; - } - if (!(vm = virDomainObjListAdd(driver->domains, *def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -3520,6 +3525,17 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto endjob; } + virObjectUnref(priv->qemuCaps); + priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, + vm->def->emulator, + vm->def->os.machine); + if (!priv->qemuCaps) + goto endjob; + + if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol, + listenAddress, port))) + goto endjob; + /* Start the QEMU daemon, with the same command-line arguments plus * -incoming $migrateFrom */ -- 2.6.3

Some failure paths in qemuMigrationPrepareAny forgot to kill the just started QEMU process. This patch fixes this by combining 'stop' and 'endjob' label into a new label 'stopjob'. This name was chosen to avoid confusion with the most common semantics of 'endjob'. Normally, 'endjob' is always called at the end of an API to stop the job we entered at the beginning. In qemuMigrationPrepareAny we only want to stop the job in failure path; on success we need to carry the job over to the Finish phase. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: ACKed in version 1 Version 3: - no change Version 2: - no change src/qemu/qemu_migration.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4ab6ab7..9a4f19f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3522,7 +3522,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, (pipe(dataFD) < 0 || virSetCloseExec(dataFD[1]) < 0)) { virReportSystemError(errno, "%s", _("cannot create pipe for tunnelled migration")); - goto endjob; + goto stopjob; } virObjectUnref(priv->qemuCaps); @@ -3530,11 +3530,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, vm->def->emulator, vm->def->os.machine); if (!priv->qemuCaps) - goto endjob; + goto stopjob; if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol, listenAddress, port))) - goto endjob; + goto stopjob; /* Start the QEMU daemon, with the same command-line arguments plus * -incoming $migrateFrom @@ -3545,14 +3545,14 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) { virDomainAuditStart(vm, "migrated", false); - goto endjob; + goto stopjob; } if (tunnel) { if (virFDStreamOpen(st, dataFD[1]) < 0) { virReportSystemError(errno, "%s", _("cannot pass pipe for tunnelled migration")); - goto stop; + goto stopjob; } dataFD[1] = -1; /* 'st' owns the FD now & will close it */ } @@ -3560,17 +3560,17 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (qemuMigrationSetCompression(driver, vm, flags & VIR_MIGRATE_COMPRESSED, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) - goto stop; + goto stopjob; if (STREQ_NULLABLE(protocol, "rdma") && virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) { - goto stop; + goto stopjob; } if (qemuMigrationSetPinAll(driver, vm, flags & VIR_MIGRATE_RDMA_PIN_ALL, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) - goto stop; + goto stopjob; if (mig->lockState) { VIR_DEBUG("Received lockstate %s", mig->lockState); @@ -3593,7 +3593,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (qemuMigrationStartNBDServer(driver, vm, listenAddress, nmigrate_disks, migrate_disks) < 0) { /* error already reported */ - goto endjob; + goto stopjob; } cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } @@ -3608,7 +3608,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, } if (qemuDomainCleanupAdd(vm, qemuMigrationPrepareCleanup) < 0) - goto endjob; + goto stopjob; if (!(flags & VIR_MIGRATE_OFFLINE)) { virDomainAuditStart(vm, "migrated", true); @@ -3647,12 +3647,13 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virNWFilterUnlockFilterUpdates(); return ret; - stop: - virDomainAuditStart(vm, "migrated", false); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, - VIR_QEMU_PROCESS_STOP_MIGRATED); + stopjob: + if (vm->def->id != -1) { + virDomainAuditStart(vm, "migrated", false); + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + VIR_QEMU_PROCESS_STOP_MIGRATED); + } - endjob: qemuMigrationJobFinish(driver, vm); goto cleanup; } -- 2.6.3

NBD storage migration will not work with offline migration anyway and we already checked that the user did not ask for it. Thus it doesn't make sense to keep the code after 'done' label where we jump in case of offline migration. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: ACKed in version 1 Version 3: - no change Version 2: - no change src/qemu/qemu_migration.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 9a4f19f..b6525df 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3418,6 +3418,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, "make sense")); goto cleanup; } + cookieFlags = 0; + } else { + cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS; } if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -3572,6 +3575,16 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto stopjob; + if (mig->nbd && + flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { + if (qemuMigrationStartNBDServer(driver, vm, listenAddress, + nmigrate_disks, migrate_disks) < 0) { + goto stopjob; + } + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + } + if (mig->lockState) { VIR_DEBUG("Received lockstate %s", mig->lockState); VIR_FREE(priv->lockState); @@ -3582,22 +3595,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, } done: - if (flags & VIR_MIGRATE_OFFLINE) - cookieFlags = 0; - else - cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS; - - if (mig->nbd && - flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { - if (qemuMigrationStartNBDServer(driver, vm, listenAddress, - nmigrate_disks, migrate_disks) < 0) { - /* error already reported */ - goto stopjob; - } - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; - } - if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, cookieFlags) < 0) { /* We could tear down the whole guest here, but -- 2.6.3

Using qemuProcess{Init,Launch,FinishStartup} allows us to run pre-migration commands on destination before asking QEMU to wait for incoming migration data. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 3: - adapt to changes in 1/7 and 3/7 Version 2: - adapt to changes in other patches src/qemu/qemu_migration.c | 65 ++++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b6525df..9148a1e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3291,14 +3291,16 @@ qemuMigrationPrepareCleanup(virQEMUDriverPtr driver, qemuDomainObjDiscardAsyncJob(driver, vm); } -static char * +static qemuProcessIncomingDefPtr qemuMigrationPrepareIncoming(virDomainObjPtr vm, bool tunnel, const char *protocol, const char *listenAddress, - unsigned short port) + unsigned short port, + int fd) { qemuDomainObjPrivatePtr priv = vm->privateData; + qemuProcessIncomingDefPtr inc = NULL; char *migrateFrom = NULL; if (tunnel) { @@ -3361,8 +3363,11 @@ qemuMigrationPrepareIncoming(virDomainObjPtr vm, goto cleanup; } + inc = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, fd, NULL); + cleanup: - return migrateFrom; + VIR_FREE(migrateFrom); + return inc; } static int @@ -3393,8 +3398,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, char *xmlout = NULL; unsigned int cookieFlags; virCapsPtr caps = NULL; - char *migrateFrom = NULL; + qemuProcessIncomingDefPtr incoming = NULL; bool taint_hook = false; + bool stopProcess = false; + bool relabel = false; + int rv; virNWFilterReadLockFilterUpdates(); @@ -3528,28 +3536,26 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto stopjob; } - virObjectUnref(priv->qemuCaps); - priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - vm->def->emulator, - vm->def->os.machine); - if (!priv->qemuCaps) + if (qemuProcessInit(driver, vm, true) < 0) goto stopjob; + stopProcess = true; - if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol, - listenAddress, port))) + if (!(incoming = qemuMigrationPrepareIncoming(vm, tunnel, protocol, + listenAddress, port, + dataFD[0]))) goto stopjob; + dataFD[0] = -1; /* the FD is now owned by incoming */ - /* Start the QEMU daemon, with the same command-line arguments plus - * -incoming $migrateFrom - */ - if (qemuProcessStart(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, - migrateFrom, dataFD[0], NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, - VIR_QEMU_PROCESS_START_PAUSED | - VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) { - virDomainAuditStart(vm, "migrated", false); + rv = qemuProcessLaunch(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, + incoming, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, + VIR_QEMU_PROCESS_START_AUTODESTROY); + if (rv < 0) { + if (rv == -2) + relabel = true; goto stopjob; } + relabel = true; if (tunnel) { if (virFDStreamOpen(st, dataFD[1]) < 0) { @@ -3594,6 +3600,15 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_DEBUG("Received no lockstate"); } + if (incoming->deferredURI && + qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + goto stopjob; + + if (qemuProcessFinishStartup(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, + false, VIR_DOMAIN_PAUSED_MIGRATION) < 0) + goto stopjob; + done: if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, cookieFlags) < 0) { @@ -3625,7 +3640,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, ret = 0; cleanup: - VIR_FREE(migrateFrom); + qemuProcessIncomingDefFree(incoming); VIR_FREE(xmlout); VIR_FORCE_CLOSE(dataFD[0]); VIR_FORCE_CLOSE(dataFD[1]); @@ -3645,10 +3660,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, return ret; stopjob: - if (vm->def->id != -1) { + if (stopProcess) { + unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED; + if (!relabel) + stopFlags |= VIR_QEMU_PROCESS_STOP_NO_RELABEL; virDomainAuditStart(vm, "migrated", false); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, - VIR_QEMU_PROCESS_STOP_MIGRATED); + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stopFlags); } qemuMigrationJobFinish(driver, vm); -- 2.6.3

On Wed, Nov 25, 2015 at 10:29:32AM +0100, Jiri Denemark wrote:
Traditionally, we pass incoming migration URI on QEMU command line, which has some drawbacks. Depending on the URI QEMU may initialize its migration state immediately without giving us a chance to set any additional migration parameters (this applies mainly for fd: URIs). For some URIs the monitor may be completely blocked from the beginning until migration is finished, which means we may be stuck in qmp_capabilities command without being able to send any QMP commands.
QEMU solved this by introducing "defer" parameter for -incoming command line option. This will tell QEMU to prepare for an incoming migration while the actual incoming URI is sent using migrate-incoming QMP command. Before calling this command we can normally talk to the monitor and even set any migration parameters which will be honored by the incoming migration.
Jiri Denemark (7): qemu: Introduce qemuProcessInit qemu: Introduce qemuProcessLaunch qemu: Introduce qemuProcessFinishStartup qemu: Separate incoming URI generation from qemuMigrationPrepareAny qemu: Kill QEMU process if Prepare phase fails qemu: Skip starting NBD servers for offline migration qemu: Use qemuProcessLaunch in migration Prepare phase
src/qemu/qemu_migration.c | 247 +++++++++++++---------- src/qemu/qemu_process.c | 489 +++++++++++++++++++++++++++++----------------- src/qemu/qemu_process.h | 20 ++ 3 files changed, 466 insertions(+), 290 deletions(-)
ACK series Pavel

On Wed, Nov 25, 2015 at 15:22:08 +0100, Pavel Hrdina wrote:
On Wed, Nov 25, 2015 at 10:29:32AM +0100, Jiri Denemark wrote:
Traditionally, we pass incoming migration URI on QEMU command line, which has some drawbacks. Depending on the URI QEMU may initialize its migration state immediately without giving us a chance to set any additional migration parameters (this applies mainly for fd: URIs). For some URIs the monitor may be completely blocked from the beginning until migration is finished, which means we may be stuck in qmp_capabilities command without being able to send any QMP commands.
QEMU solved this by introducing "defer" parameter for -incoming command line option. This will tell QEMU to prepare for an incoming migration while the actual incoming URI is sent using migrate-incoming QMP command. Before calling this command we can normally talk to the monitor and even set any migration parameters which will be honored by the incoming migration.
Jiri Denemark (7): qemu: Introduce qemuProcessInit qemu: Introduce qemuProcessLaunch qemu: Introduce qemuProcessFinishStartup qemu: Separate incoming URI generation from qemuMigrationPrepareAny qemu: Kill QEMU process if Prepare phase fails qemu: Skip starting NBD servers for offline migration qemu: Use qemuProcessLaunch in migration Prepare phase
src/qemu/qemu_migration.c | 247 +++++++++++++---------- src/qemu/qemu_process.c | 489 +++++++++++++++++++++++++++++----------------- src/qemu/qemu_process.h | 20 ++ 3 files changed, 466 insertions(+), 290 deletions(-)
ACK series
Thanks, pushed. Jirka
participants (2)
-
Jiri Denemark
-
Pavel Hrdina