[libvirt] [PATCH 00/24] 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 (24): qemu: Refactor waiting for completed migration on destination qemu: Refactor the code to build -incoming command line qemu: Don't generate migration URI in qemuBuildCommandLine qemu: Move incoming URI code to qemu_migration qemu: Introduce qemuProcessIncomingDef qemu: Always set async job when starting a domain qemu: Add APIs for migrate-incoming QMP command qemu: Use -incoming defer for migrations qemu: Rename stdin_{fd,path} in qemuProcessStart qemu: Separate hook handling code from qemuProcessStart qemu: Separate graphics handling code from qemuProcessStart qemu: Separate raw IO code from qemuProcessStart qemu: Enter monitor within qemuProcessSetLinkStates qemu: Separate balloon code from qemuProcessStart qemu: Introduce qemuProcessMakeDir qemu: Do not infer flags from other qemuProcessStart arguments qemu: Close logfd when closing monitor qemu: Introduce qemuProcessInit qemu: Introduce qemuProcessLaunch qemu: Introduce qemuProcessFinish 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_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 42 +- src/qemu/qemu_command.h | 4 +- src/qemu/qemu_domain.c | 3 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 74 ++- src/qemu/qemu_migration.c | 357 +++++++---- src/qemu/qemu_migration.h | 11 + src/qemu/qemu_monitor.c | 14 + src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 25 + src/qemu/qemu_monitor_json.h | 5 + src/qemu/qemu_process.c | 881 +++++++++++++++++---------- src/qemu/qemu_process.h | 41 +- tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + tests/qemuxml2argvtest.c | 16 +- 17 files changed, 969 insertions(+), 515 deletions(-) -- 2.6.3
Move the code from qemuMigrationFinish into a dedicated function. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3eee3a5..e6b3484 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2764,6 +2764,28 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, static int +qemuMigrationWaitForDestCompletion(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int rv; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) + return 0; + + VIR_DEBUG("Waiting for incoming migration to complete"); + + while ((rv = qemuMigrationCompleted(driver, vm, asyncJob, NULL, 0)) != 1) { + if (rv < 0 || virDomainObjWait(vm) < 0) + return -1; + } + + return 0; +} + + +static int qemuDomainMigrateGraphicsRelocate(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMigrationCookiePtr cookie, @@ -5731,22 +5753,13 @@ qemuMigrationFinish(virQEMUDriverPtr driver, /* We need to wait for QEMU to process all data sent by the source * before starting guest CPUs. */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) { - int rv; - VIR_DEBUG("Waiting for migration to complete"); - while ((rv = qemuMigrationCompleted(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN, - NULL, 0)) != 1) { - if (rv < 0 || virDomainObjWait(vm) < 0) { - /* There's not much we can do for v2 protocol since the - * original domain on the source host is already gone. - */ - if (v3proto) - goto endjob; - else - break; - } - } + if (qemuMigrationWaitForDestCompletion(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) { + /* There's not much we can do for v2 protocol since the + * original domain on the source host is already gone. + */ + if (v3proto) + goto endjob; } if (!(flags & VIR_MIGRATE_PAUSED)) { -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:03 +0100, Jiri Denemark wrote:
Move the code from qemuMigrationFinish into a dedicated function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-)
ACK, Peter
Move the code from qemuBuildCommandLine into dedicated functions. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Both functions will later be moved to qemu_migration.c src/qemu/qemu_command.c | 75 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..723609d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9136,6 +9136,45 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, return 0; } +static int +qemuBuildIncomingCheckProtocol(virQEMUCapsPtr qemuCaps, + const char *migrateFrom) +{ + if (STRPREFIX(migrateFrom, "rdma")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("incoming RDMA migration is not supported " + "with this QEMU binary")); + return -1; + } + } else if (!STRPREFIX(migrateFrom, "tcp") && + !STRPREFIX(migrateFrom, "exec") && + !STRPREFIX(migrateFrom, "fd") && + !STRPREFIX(migrateFrom, "unix") && + STRNEQ(migrateFrom, "stdio")) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("unknown migration protocol")); + return -1; + } + + return 0; +} + + +static char * +qemuBuildIncomingURI(const char *migrateFrom, + int migrateFd) +{ + char *uri = NULL; + + if (STREQ(migrateFrom, "stdio")) + ignore_value(virAsprintf(&uri, "fd:%d", migrateFd)); + else + ignore_value(VIR_STRDUP(uri, migrateFrom)); + + return uri; +} + qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName, @@ -11006,32 +11045,20 @@ qemuBuildCommandLine(virConnectPtr conn, } if (migrateFrom) { - virCommandAddArg(cmd, "-incoming"); - if (STRPREFIX(migrateFrom, "tcp")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "rdma")) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("incoming RDMA migration is not supported " - "with this QEMU binary")); - goto error; - } - virCommandAddArg(cmd, migrateFrom); - } else if (STREQ(migrateFrom, "stdio")) { - virCommandAddArgFormat(cmd, "fd:%d", migrateFd); - virCommandPassFD(cmd, migrateFd, 0); - } else if (STRPREFIX(migrateFrom, "exec")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "fd")) { - virCommandAddArg(cmd, migrateFrom); + char *migrateURI; + + if (qemuBuildIncomingCheckProtocol(qemuCaps, migrateFrom) < 0) + goto error; + + if (STREQ(migrateFrom, "stdio") && + STRPREFIX(migrateFrom, "fd")) virCommandPassFD(cmd, migrateFd, 0); - } else if (STRPREFIX(migrateFrom, "unix")) { - virCommandAddArg(cmd, migrateFrom); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unknown migration protocol")); + + migrateURI = qemuBuildIncomingURI(migrateFrom, migrateFd); + if (!migrateURI) goto error; - } + virCommandAddArgList(cmd, "-incoming", migrateURI, NULL); + VIR_FREE(migrateURI); } /* QEMU changed its default behavior to not include the virtio balloon -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:04 +0100, Jiri Denemark wrote:
Move the code from qemuBuildCommandLine into dedicated functions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Both functions will later be moved to qemu_migration.c
src/qemu/qemu_command.c | 75 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..723609d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11006,32 +11045,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
if (migrateFrom) { - virCommandAddArg(cmd, "-incoming"); - if (STRPREFIX(migrateFrom, "tcp")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "rdma")) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("incoming RDMA migration is not supported " - "with this QEMU binary")); - goto error; - } - virCommandAddArg(cmd, migrateFrom); - } else if (STREQ(migrateFrom, "stdio")) { - virCommandAddArgFormat(cmd, "fd:%d", migrateFd); - virCommandPassFD(cmd, migrateFd, 0); - } else if (STRPREFIX(migrateFrom, "exec")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "fd")) { - virCommandAddArg(cmd, migrateFrom); + char *migrateURI; + + if (qemuBuildIncomingCheckProtocol(qemuCaps, migrateFrom) < 0) + goto error; + + if (STREQ(migrateFrom, "stdio") && + STRPREFIX(migrateFrom, "fd"))
I doubt that this will ever be true. Peter
On Fri, Nov 13, 2015 at 10:06:08 +0100, Peter Krempa wrote:
On Thu, Nov 12, 2015 at 19:37:04 +0100, Jiri Denemark wrote:
Move the code from qemuBuildCommandLine into dedicated functions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Both functions will later be moved to qemu_migration.c
src/qemu/qemu_command.c | 75 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..723609d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11006,32 +11045,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
if (migrateFrom) { - virCommandAddArg(cmd, "-incoming"); - if (STRPREFIX(migrateFrom, "tcp")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "rdma")) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("incoming RDMA migration is not supported " - "with this QEMU binary")); - goto error; - } - virCommandAddArg(cmd, migrateFrom); - } else if (STREQ(migrateFrom, "stdio")) { - virCommandAddArgFormat(cmd, "fd:%d", migrateFd); - virCommandPassFD(cmd, migrateFd, 0); - } else if (STRPREFIX(migrateFrom, "exec")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "fd")) { - virCommandAddArg(cmd, migrateFrom); + char *migrateURI; + + if (qemuBuildIncomingCheckProtocol(qemuCaps, migrateFrom) < 0) + goto error; + + if (STREQ(migrateFrom, "stdio") && + STRPREFIX(migrateFrom, "fd"))
I doubt that this will ever be true.
Oops, looks like a wrong rebase after Daniel's removal of old migration features. The following patch removes this completely, which is why I didn't catch it during testing :-) Jirka
On Fri, Nov 13, 2015 at 10:28:08 +0100, Jiri Denemark wrote:
On Fri, Nov 13, 2015 at 10:06:08 +0100, Peter Krempa wrote:
On Thu, Nov 12, 2015 at 19:37:04 +0100, Jiri Denemark wrote:
Move the code from qemuBuildCommandLine into dedicated functions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Both functions will later be moved to qemu_migration.c
src/qemu/qemu_command.c | 75 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..723609d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11006,32 +11045,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
if (migrateFrom) { - virCommandAddArg(cmd, "-incoming"); - if (STRPREFIX(migrateFrom, "tcp")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "rdma")) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("incoming RDMA migration is not supported " - "with this QEMU binary")); - goto error; - } - virCommandAddArg(cmd, migrateFrom); - } else if (STREQ(migrateFrom, "stdio")) { - virCommandAddArgFormat(cmd, "fd:%d", migrateFd); - virCommandPassFD(cmd, migrateFd, 0); - } else if (STRPREFIX(migrateFrom, "exec")) { - virCommandAddArg(cmd, migrateFrom); - } else if (STRPREFIX(migrateFrom, "fd")) { - virCommandAddArg(cmd, migrateFrom); + char *migrateURI; + + if (qemuBuildIncomingCheckProtocol(qemuCaps, migrateFrom) < 0) + goto error; + + if (STREQ(migrateFrom, "stdio") && + STRPREFIX(migrateFrom, "fd"))
I doubt that this will ever be true.
Oops, looks like a wrong rebase after Daniel's removal of old migration features. The following patch removes this completely, which is why I didn't catch it during testing :-)
I'm now going through that patch and indeed it's being removed and the condition sanitized. ACK to this one if you change && to ||. Peter
Make callers of qemuBuildCommandLine responsible for providing the URI which should be passed as a parameter for -incoming. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_command.c | 32 ++++++++------------------------ src/qemu/qemu_command.h | 9 +++++++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 14 +++++++++++++- tests/qemuxml2argvtest.c | 15 ++++++++++----- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 723609d..41a212f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9136,7 +9136,7 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, return 0; } -static int +int qemuBuildIncomingCheckProtocol(virQEMUCapsPtr qemuCaps, const char *migrateFrom) { @@ -9161,7 +9161,7 @@ qemuBuildIncomingCheckProtocol(virQEMUCapsPtr qemuCaps, } -static char * +char * qemuBuildIncomingURI(const char *migrateFrom, int migrateFd) { @@ -9194,8 +9194,7 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainChrSourceDefPtr monitor_chr, bool monitor_json, virQEMUCapsPtr qemuCaps, - const char *migrateFrom, - int migrateFd, + const char *migrateURI, virDomainSnapshotObjPtr snapshot, virNetDevVPortProfileOp vmop, qemuBuildCommandLineCallbacksPtr callbacks, @@ -9256,10 +9255,9 @@ qemuBuildCommandLine(virConnectPtr conn, int bootCD = 0, bootFloppy = 0, bootDisk = 0; VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " - "qemuCaps=%p migrateFrom=%s migrateFD=%d " - "snapshot=%p vmop=%d", + "qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d", conn, driver, def, monitor_chr, monitor_json, - qemuCaps, migrateFrom, migrateFd, snapshot, vmop); + qemuCaps, migrateURI, snapshot, vmop); virUUIDFormat(def->uuid, uuid); @@ -9338,7 +9336,7 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; if (qemuBuildCpuArgStr(driver, def, emulator, qemuCaps, - hostarch, &cpu, &hasHwVirt, !!migrateFrom) < 0) + hostarch, &cpu, &hasHwVirt, !!migrateURI) < 0) goto error; if (cpu) { @@ -9353,7 +9351,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (!migrateFrom && !snapshot && + if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0) goto error; @@ -11044,22 +11042,8 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (migrateFrom) { - char *migrateURI; - - if (qemuBuildIncomingCheckProtocol(qemuCaps, migrateFrom) < 0) - goto error; - - if (STREQ(migrateFrom, "stdio") && - STRPREFIX(migrateFrom, "fd")) - virCommandPassFD(cmd, migrateFd, 0); - - migrateURI = qemuBuildIncomingURI(migrateFrom, migrateFd); - if (!migrateURI) - goto error; + if (migrateURI) virCommandAddArgList(cmd, "-incoming", migrateURI, NULL); - VIR_FREE(migrateURI); - } /* QEMU changed its default behavior to not include the virtio balloon * device. Explicitly request it to ensure it will be present. diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d1af3b7..5c65008 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -78,8 +78,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virDomainChrSourceDefPtr monitor_chr, bool monitor_json, virQEMUCapsPtr qemuCaps, - const char *migrateFrom, - int migrateFd, + const char *migrateURI, virDomainSnapshotObjPtr current_snapshot, virNetDevVPortProfileOp vmop, qemuBuildCommandLineCallbacksPtr callbacks, @@ -322,4 +321,10 @@ bool qemuCheckCCWS390AddressSupport(virDomainDefPtr def, virDomainDeviceInfo info, virQEMUCapsPtr qemuCaps, const char *devicename); +int qemuBuildIncomingCheckProtocol(virQEMUCapsPtr qemuCaps, + const char *migrateFrom); + +char *qemuBuildIncomingURI(const char *migrateFrom, + int migrateFd); + #endif /* __QEMU_COMMAND_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92a9961..42f76fa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7268,7 +7268,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, if (!(cmd = qemuBuildCommandLine(conn, driver, def, &monConfig, monitor_json, qemuCaps, - NULL, -1, NULL, + NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, &buildCommandLineCallbacks, true, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4545f77..0bafef9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4185,6 +4185,7 @@ int qemuProcessStart(virConnectPtr conn, size_t nnicindexes = 0; int *nicindexes = NULL; char *tmppath = NULL; + char *migrateURI = NULL; VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s stdin_fd=%d " "stdin_path=%s snapshot=%p vmop=%d flags=0x%x", @@ -4512,16 +4513,26 @@ int qemuProcessStart(virConnectPtr conn, goto error; } + if (migrateFrom) { + if (qemuBuildIncomingCheckProtocol(priv->qemuCaps, migrateFrom) < 0) + goto error; + + if (!(migrateURI = qemuBuildIncomingURI(migrateFrom, stdin_fd))) + goto error; + } + VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON, priv->qemuCaps, - migrateFrom, stdin_fd, snapshot, vmop, + migrateURI, snapshot, vmop, &buildCommandLineCallbacks, false, qemuCheckFips(), priv->autoNodeset, &nnicindexes, &nicindexes))) goto error; + if (migrateFrom && stdin_fd != -1) + virCommandPassFD(cmd, stdin_fd, 0); /* * Create all per-domain directories in order to make sure domain @@ -4907,6 +4918,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_FREE(nicindexes); VIR_FREE(nodeset); VIR_FREE(tmppath); + VIR_FREE(migrateURI); return ret; error: diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1c52828..50a7dff 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -249,8 +249,7 @@ typedef enum { static int testCompareXMLToArgvFiles(const char *xml, const char *cmdline, virQEMUCapsPtr extraFlags, - const char *migrateFrom, - int migrateFd, + const char *migrateURI, virQemuXML2ArgvTestFlags flags) { char *actualargv = NULL; @@ -341,7 +340,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, - migrateFrom, migrateFd, NULL, + migrateURI, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, &testCallbacks, false, (flags & FLAG_FIPS), @@ -408,6 +407,12 @@ testCompareXMLToArgvHelper(const void *data) char *xml = NULL; char *args = NULL; unsigned int flags = info->flags; + char *migrateURI = NULL; + + if (info->migrateFrom && + !(migrateURI = qemuBuildIncomingURI(info->migrateFrom, + info->migrateFd))) + goto cleanup; if (virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", abs_srcdir, info->name) < 0 || @@ -427,10 +432,10 @@ testCompareXMLToArgvHelper(const void *data) goto cleanup; result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, - info->migrateFrom, info->migrateFd, - flags); + migrateURI, flags); cleanup: + VIR_FREE(migrateURI); VIR_FREE(xml); VIR_FREE(args); return result; -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:05 +0100, Jiri Denemark wrote:
Make callers of qemuBuildCommandLine responsible for providing the URI which should be passed as a parameter for -incoming.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_command.c | 32 ++++++++------------------------ src/qemu/qemu_command.h | 9 +++++++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 14 +++++++++++++- tests/qemuxml2argvtest.c | 15 ++++++++++----- 5 files changed, 39 insertions(+), 33 deletions(-)
ACK
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_command.c | 39 --------------------------------------- src/qemu/qemu_command.h | 5 ----- src/qemu/qemu_migration.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 6 ++++++ src/qemu/qemu_process.c | 4 ++-- tests/qemuxml2argvtest.c | 5 +++-- 6 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 41a212f..d196417 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9136,45 +9136,6 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, return 0; } -int -qemuBuildIncomingCheckProtocol(virQEMUCapsPtr qemuCaps, - const char *migrateFrom) -{ - if (STRPREFIX(migrateFrom, "rdma")) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("incoming RDMA migration is not supported " - "with this QEMU binary")); - return -1; - } - } else if (!STRPREFIX(migrateFrom, "tcp") && - !STRPREFIX(migrateFrom, "exec") && - !STRPREFIX(migrateFrom, "fd") && - !STRPREFIX(migrateFrom, "unix") && - STRNEQ(migrateFrom, "stdio")) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("unknown migration protocol")); - return -1; - } - - return 0; -} - - -char * -qemuBuildIncomingURI(const char *migrateFrom, - int migrateFd) -{ - char *uri = NULL; - - if (STREQ(migrateFrom, "stdio")) - ignore_value(virAsprintf(&uri, "fd:%d", migrateFd)); - else - ignore_value(VIR_STRDUP(uri, migrateFrom)); - - return uri; -} - qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 5c65008..7027402 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -321,10 +321,5 @@ bool qemuCheckCCWS390AddressSupport(virDomainDefPtr def, virDomainDeviceInfo info, virQEMUCapsPtr qemuCaps, const char *devicename); -int qemuBuildIncomingCheckProtocol(virQEMUCapsPtr qemuCaps, - const char *migrateFrom); - -char *qemuBuildIncomingURI(const char *migrateFrom, - int migrateFd); #endif /* __QEMU_COMMAND_H__*/ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e6b3484..4d5b966 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2911,6 +2911,46 @@ qemuDomainMigrateOPDRelocate(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, } +int +qemuMigrationCheckIncoming(virQEMUCapsPtr qemuCaps, + const char *migrateFrom) +{ + if (STRPREFIX(migrateFrom, "rdma")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("incoming RDMA migration is not supported " + "with this QEMU binary")); + return -1; + } + } else if (!STRPREFIX(migrateFrom, "tcp") && + !STRPREFIX(migrateFrom, "exec") && + !STRPREFIX(migrateFrom, "fd") && + !STRPREFIX(migrateFrom, "unix") && + STRNEQ(migrateFrom, "stdio")) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("unknown migration protocol")); + return -1; + } + + return 0; +} + + +char * +qemuMigrationIncomingURI(const char *migrateFrom, + int migrateFd) +{ + char *uri = NULL; + + if (STREQ(migrateFrom, "stdio")) + ignore_value(virAsprintf(&uri, "fd:%d", migrateFd)); + else + ignore_value(VIR_STRDUP(uri, migrateFrom)); + + return uri; +} + + /* This is called for outgoing non-p2p migrations when a connection to the * client which initiated the migration was closed but we were waiting for it * to follow up with the next phase, that is, in between diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 8175f4b..ff4fe30 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -199,4 +199,10 @@ void qemuMigrationErrorSave(virQEMUDriverPtr driver, void qemuMigrationErrorReport(virQEMUDriverPtr driver, const char *name); +int qemuMigrationCheckIncoming(virQEMUCapsPtr qemuCaps, + const char *migrateFrom); + +char *qemuMigrationIncomingURI(const char *migrateFrom, + int migrateFd); + #endif /* __QEMU_MIGRATION_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0bafef9..fdd640d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4514,10 +4514,10 @@ int qemuProcessStart(virConnectPtr conn, } if (migrateFrom) { - if (qemuBuildIncomingCheckProtocol(priv->qemuCaps, migrateFrom) < 0) + if (qemuMigrationCheckIncoming(priv->qemuCaps, migrateFrom) < 0) goto error; - if (!(migrateURI = qemuBuildIncomingURI(migrateFrom, stdin_fd))) + if (!(migrateURI = qemuMigrationIncomingURI(migrateFrom, stdin_fd))) goto error; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 50a7dff..e5c1c4f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -17,6 +17,7 @@ # include "qemu/qemu_capabilities.h" # include "qemu/qemu_command.h" # include "qemu/qemu_domain.h" +# include "qemu/qemu_migration.h" # include "datatypes.h" # include "conf/storage_conf.h" # include "cpu/cpu_map.h" @@ -410,8 +411,8 @@ testCompareXMLToArgvHelper(const void *data) char *migrateURI = NULL; if (info->migrateFrom && - !(migrateURI = qemuBuildIncomingURI(info->migrateFrom, - info->migrateFd))) + !(migrateURI = qemuMigrationIncomingURI(info->migrateFrom, + info->migrateFd))) goto cleanup; if (virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:06 +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_command.c | 39 --------------------------------------- src/qemu/qemu_command.h | 5 ----- src/qemu/qemu_migration.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 6 ++++++ src/qemu/qemu_process.c | 4 ++-- tests/qemuxml2argvtest.c | 5 +++-- 6 files changed, 51 insertions(+), 48 deletions(-)
ACK
Incoming migration may require quite a few parameters (URI, fd, path) to be considered while starting QEMU and we will soon add another one. Let's group all of them in a single struct. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 67 +++++++++++++++++++++++++++++++++++++++---------- src/qemu/qemu_process.h | 14 +++++++++++ 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fdd640d..638ad02 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4156,6 +4156,47 @@ qemuLogOperation(virDomainObjPtr vm, goto cleanup; } + +void +qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc) +{ + if (!inc) + return; + + VIR_FREE(inc->launchURI); + VIR_FREE(inc); +} + + +qemuProcessIncomingDefPtr +qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, + const char *migrateFrom, + int fd, + const char *path) +{ + qemuProcessIncomingDefPtr inc = NULL; + + if (qemuMigrationCheckIncoming(qemuCaps, migrateFrom) < 0) + return NULL; + + if (VIR_ALLOC(inc) < 0) + return NULL; + + inc->launchURI = qemuMigrationIncomingURI(migrateFrom, fd); + if (!inc->launchURI) + goto error; + + inc->fd = fd; + inc->path = path; + + return inc; + + error: + qemuProcessIncomingDefFree(inc); + return NULL; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4185,7 +4226,7 @@ int qemuProcessStart(virConnectPtr conn, size_t nnicindexes = 0; int *nicindexes = NULL; char *tmppath = NULL; - char *migrateURI = NULL; + qemuProcessIncomingDefPtr incoming = NULL; VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s stdin_fd=%d " "stdin_path=%s snapshot=%p vmop=%d flags=0x%x", @@ -4514,25 +4555,25 @@ int qemuProcessStart(virConnectPtr conn, } if (migrateFrom) { - if (qemuMigrationCheckIncoming(priv->qemuCaps, migrateFrom) < 0) - goto error; - - if (!(migrateURI = qemuMigrationIncomingURI(migrateFrom, stdin_fd))) + incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, + stdin_fd, stdin_path); + if (!incoming) goto error; } VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON, priv->qemuCaps, - migrateURI, snapshot, vmop, + incoming ? incoming->launchURI : NULL, + snapshot, vmop, &buildCommandLineCallbacks, false, qemuCheckFips(), priv->autoNodeset, &nnicindexes, &nicindexes))) goto error; - if (migrateFrom && stdin_fd != -1) - virCommandPassFD(cmd, stdin_fd, 0); + if (incoming && incoming->fd != -1) + virCommandPassFD(cmd, incoming->fd, 0); /* * Create all per-domain directories in order to make sure domain @@ -4731,7 +4772,7 @@ int qemuProcessStart(virConnectPtr conn, goto error; VIR_DEBUG("Handshake complete, child running"); - if (migrateFrom) + if (incoming) flags |= VIR_QEMU_PROCESS_START_PAUSED; if (rv == -1) /* The VM failed to start; tear filters before taps */ @@ -4852,7 +4893,7 @@ int qemuProcessStart(virConnectPtr conn, /* 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 (!migrateFrom && !snapshot && + if (!incoming && !snapshot && qemuProcessRefreshBalloonState(driver, vm, asyncJob) < 0) goto error; @@ -4873,7 +4914,7 @@ int qemuProcessStart(virConnectPtr conn, } } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, - migrateFrom ? + incoming ? VIR_DOMAIN_PAUSED_MIGRATION : VIR_DOMAIN_PAUSED_USER); } @@ -4905,7 +4946,7 @@ int qemuProcessStart(virConnectPtr conn, /* Keep watching qemu log for errors during incoming migration, otherwise * unset reporting errors from qemu log. */ - if (!migrateFrom) + if (!incoming) qemuMonitorSetDomainLog(priv->mon, -1); ret = 0; @@ -4918,7 +4959,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_FREE(nicindexes); VIR_FREE(nodeset); VIR_FREE(tmppath); - VIR_FREE(migrateURI); + qemuProcessIncomingDefFree(incoming); return ret; error: diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index d40f68d..7eee2b2 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -44,6 +44,20 @@ void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver); int qemuProcessAssignPCIAddresses(virDomainDefPtr def); +typedef struct _qemuProcessIncomingDef qemuProcessIncomingDef; +typedef qemuProcessIncomingDef *qemuProcessIncomingDefPtr; +struct _qemuProcessIncomingDef { + char *launchURI; /* used as a parameter for -incoming command line option */ + int fd; /* for fd:N URI */ + const char *path; /* path associated with fd */ +}; + +qemuProcessIncomingDefPtr qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, + const char *migrateFrom, + int fd, + const char *path); +void qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc); + typedef enum { VIR_QEMU_PROCESS_START_COLD = 1 << 0, VIR_QEMU_PROCESS_START_PAUSED = 1 << 1, -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:07 +0100, Jiri Denemark wrote:
Incoming migration may require quite a few parameters (URI, fd, path) to be considered while starting QEMU and we will soon add another one. Let's group all of them in a single struct.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 67 +++++++++++++++++++++++++++++++++++++++---------- src/qemu/qemu_process.h | 14 +++++++++++ 2 files changed, 68 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fdd640d..638ad02 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4156,6 +4156,47 @@ qemuLogOperation(virDomainObjPtr vm,
[...]
+ +qemuProcessIncomingDefPtr +qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, + const char *migrateFrom, + int fd, + const char *path)
Please add a comment stating that @path is not copied, so it has to be available duirng the lifetime of the struct this allocates.
+{ + qemuProcessIncomingDefPtr inc = NULL; + + if (qemuMigrationCheckIncoming(qemuCaps, migrateFrom) < 0) + return NULL; + + if (VIR_ALLOC(inc) < 0) + return NULL; + + inc->launchURI = qemuMigrationIncomingURI(migrateFrom, fd); + if (!inc->launchURI) + goto error; + + inc->fd = fd; + inc->path = path; + + return inc; + + error: + qemuProcessIncomingDefFree(inc); + return NULL; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm,
ACK
We only started an async job for incoming migration from another host. When we were starting a domain from scratch or restoring from a saved state (migration from file) we didn't set any async job. Let's introduce a new QEMU_ASYNC_JOB_START for these cases. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 72 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_process.c | 30 ++++++++++++++++++++- src/qemu/qemu_process.h | 7 ++++- 5 files changed, 79 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3d2b7a0..4aebd90 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -73,6 +73,7 @@ VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST, "save", "dump", "snapshot", + "start", ); @@ -88,6 +89,7 @@ qemuDomainAsyncJobPhaseToString(qemuDomainAsyncJob job, case QEMU_ASYNC_JOB_SAVE: case QEMU_ASYNC_JOB_DUMP: case QEMU_ASYNC_JOB_SNAPSHOT: + case QEMU_ASYNC_JOB_START: case QEMU_ASYNC_JOB_NONE: case QEMU_ASYNC_JOB_LAST: ; /* fall through */ @@ -111,6 +113,7 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job, case QEMU_ASYNC_JOB_SAVE: case QEMU_ASYNC_JOB_DUMP: case QEMU_ASYNC_JOB_SNAPSHOT: + case QEMU_ASYNC_JOB_START: case QEMU_ASYNC_JOB_NONE: case QEMU_ASYNC_JOB_LAST: ; /* fall through */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 4be998c..4ef25e2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -90,6 +90,7 @@ typedef enum { QEMU_ASYNC_JOB_SAVE, QEMU_ASYNC_JOB_DUMP, QEMU_ASYNC_JOB_SNAPSHOT, + QEMU_ASYNC_JOB_START, QEMU_ASYNC_JOB_LAST } qemuDomainAsyncJob; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 42f76fa..65ccf99 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -155,7 +155,8 @@ static int qemuStateCleanup(void); static int qemuDomainObjStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, - unsigned int flags); + unsigned int flags, + qemuDomainAsyncJob asyncJob); static int qemuDomainGetMaxVcpus(virDomainPtr dom); @@ -287,8 +288,7 @@ qemuAutostartDomain(virDomainObjPtr vm, virResetLastError(); if (vm->autostart && !virDomainObjIsActive(vm)) { - if (qemuDomainObjBeginJob(data->driver, vm, - QEMU_JOB_MODIFY) < 0) { + if (qemuProcessBeginJob(data->driver, vm) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to start job on VM '%s': %s"), vm->def->name, @@ -296,14 +296,15 @@ qemuAutostartDomain(virDomainObjPtr vm, goto cleanup; } - if (qemuDomainObjStart(data->conn, data->driver, vm, flags) < 0) { + if (qemuDomainObjStart(data->conn, data->driver, vm, flags, + QEMU_ASYNC_JOB_START) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, err ? err->message : _("unknown error")); } - qemuDomainObjEndJob(data->driver, vm); + qemuProcessEndJob(data->driver, vm); } ret = 0; @@ -1754,17 +1755,17 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, virObjectRef(vm); def = NULL; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) { + if (qemuProcessBeginJob(driver, vm) < 0) { qemuDomainRemoveInactive(driver, vm); goto cleanup; } - if (qemuProcessStart(conn, driver, vm, QEMU_ASYNC_JOB_NONE, + if (qemuProcessStart(conn, driver, vm, QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); qemuDomainRemoveInactive(driver, vm); goto cleanup; } @@ -1788,7 +1789,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (dom) dom->id = vm->def->id; - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); cleanup: virDomainDefFree(def); @@ -6634,7 +6635,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, int *fd, const virQEMUSaveHeader *header, const char *path, - bool start_paused) + bool start_paused, + qemuDomainAsyncJob asyncJob) { int ret = -1; virObjectEventPtr event; @@ -6663,7 +6665,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ - ret = qemuProcessStart(conn, driver, vm, QEMU_ASYNC_JOB_NONE, + ret = qemuProcessStart(conn, driver, vm, asyncJob, "stdio", *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, VIR_QEMU_PROCESS_START_PAUSED); @@ -6707,7 +6709,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (header->was_running && !start_paused) { if (qemuProcessStartCPUs(driver, vm, conn, VIR_DOMAIN_RUNNING_RESTORED, - QEMU_ASYNC_JOB_NONE) < 0) { + asyncJob) < 0) { if (virGetLastError() == NULL) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); @@ -6819,15 +6821,15 @@ qemuDomainRestoreFlags(virConnectPtr conn, priv->hookRun = true; } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuProcessBeginJob(driver, vm) < 0) goto cleanup; ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path, - false); + false, QEMU_ASYNC_JOB_START); if (virFileWrapperFdClose(wrapperFd) < 0) VIR_WARN("Failed to close %s", path); - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); cleanup: virDomainDefFree(def); @@ -6970,7 +6972,8 @@ qemuDomainObjRestore(virConnectPtr conn, virDomainObjPtr vm, const char *path, bool start_paused, - bool bypass_cache) + bool bypass_cache, + qemuDomainAsyncJob asyncJob) { virDomainDefPtr def = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -7030,7 +7033,7 @@ qemuDomainObjRestore(virConnectPtr conn, def = NULL; ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path, - start_paused); + start_paused, asyncJob); if (virFileWrapperFdClose(wrapperFd) < 0) VIR_WARN("Failed to close %s", path); @@ -7325,7 +7328,8 @@ static int qemuDomainObjStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, - unsigned int flags) + unsigned int flags, + qemuDomainAsyncJob asyncJob) { int ret = -1; char *managed_save; @@ -7358,7 +7362,7 @@ qemuDomainObjStart(virConnectPtr conn, vm->hasManagedSave = false; } else { ret = qemuDomainObjRestore(conn, driver, vm, managed_save, - start_paused, bypass_cache); + start_paused, bypass_cache, asyncJob); if (ret == 0) { if (unlink(managed_save) < 0) @@ -7377,7 +7381,7 @@ qemuDomainObjStart(virConnectPtr conn, } } - ret = qemuProcessStart(conn, driver, vm, QEMU_ASYNC_JOB_NONE, + ret = qemuProcessStart(conn, driver, vm, asyncJob, NULL, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "booted", ret >= 0); @@ -7422,7 +7426,7 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuProcessBeginJob(driver, vm) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { @@ -7431,13 +7435,14 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) goto endjob; } - if (qemuDomainObjStart(dom->conn, driver, vm, flags) < 0) + if (qemuDomainObjStart(dom->conn, driver, vm, flags, + QEMU_ASYNC_JOB_START) < 0) goto endjob; ret = 0; endjob: - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); cleanup: virDomainObjEndAPI(&vm); @@ -15334,7 +15339,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto cleanup; } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuProcessBeginJob(driver, vm) < 0) goto cleanup; if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) @@ -15439,7 +15444,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, was_running = true; if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_FROM_SNAPSHOT, - QEMU_ASYNC_JOB_NONE) < 0) + QEMU_ASYNC_JOB_START) < 0) goto endjob; /* Create an event now in case the restore fails, so * that user will be alerted that they are now paused. @@ -15454,7 +15459,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto endjob; } } - qemuDomainObjEnterMonitor(driver, vm); + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_START) < 0) + goto endjob; rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; @@ -15473,7 +15481,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainObjAssignDef(vm, config, false, NULL); rc = qemuProcessStart(snapshot->domain->conn, driver, vm, - QEMU_ASYNC_JOB_NONE, NULL, -1, NULL, snap, + QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, VIR_QEMU_PROCESS_START_PAUSED); virDomainAuditStart(vm, "from-snapshot", rc >= 0); @@ -15508,7 +15516,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } rc = qemuProcessStartCPUs(driver, vm, snapshot->domain->conn, VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, - QEMU_ASYNC_JOB_NONE); + QEMU_ASYNC_JOB_START); if (rc < 0) goto endjob; virObjectUnref(event); @@ -15549,7 +15557,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } if (qemuDomainSnapshotRevertInactive(driver, vm, snap) < 0) { - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); qemuDomainRemoveInactive(driver, vm); goto cleanup; } @@ -15566,12 +15574,12 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuDomainEventQueue(driver, event); rc = qemuProcessStart(snapshot->domain->conn, driver, vm, - QEMU_ASYNC_JOB_NONE, NULL, -1, NULL, NULL, + QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) { - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); qemuDomainRemoveInactive(driver, vm); goto cleanup; } @@ -15607,7 +15615,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, ret = 0; endjob: - qemuDomainObjEndJob(driver, vm); + qemuProcessEndJob(driver, vm); cleanup: if (ret == 0) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 638ad02..f85e876 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3412,6 +3412,10 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, } break; + case QEMU_ASYNC_JOB_START: + /* Already handled in VIR_DOMAIN_PAUSED_STARTING_UP check. */ + break; + case QEMU_ASYNC_JOB_NONE: case QEMU_ASYNC_JOB_LAST: break; @@ -4197,10 +4201,34 @@ qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, } +int +qemuProcessBeginJob(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_START) < 0) + return -1; + + qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); + priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED; + + return 0; +} + + +void +qemuProcessEndJob(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjEndAsyncJob(driver, vm); +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, - int asyncJob, + qemuDomainAsyncJob asyncJob, const char *migrateFrom, int stdin_fd, const char *stdin_path, diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 7eee2b2..dcba728 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -58,6 +58,11 @@ qemuProcessIncomingDefPtr qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, const char *path); void qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc); +int qemuProcessBeginJob(virQEMUDriverPtr driver, + virDomainObjPtr vm); +void qemuProcessEndJob(virQEMUDriverPtr driver, + virDomainObjPtr vm); + typedef enum { VIR_QEMU_PROCESS_START_COLD = 1 << 0, VIR_QEMU_PROCESS_START_PAUSED = 1 << 1, @@ -67,7 +72,7 @@ typedef enum { int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, - int asyncJob, + qemuDomainAsyncJob asyncJob, const char *migrateFrom, int stdin_fd, const char *stdin_path, -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:08 +0100, Jiri Denemark wrote:
We only started an async job for incoming migration from another host. When we were starting a domain from scratch or restoring from a saved state (migration from file) we didn't set any async job. Let's introduce a new QEMU_ASYNC_JOB_START for these cases.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 72 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_process.c | 30 ++++++++++++++++++++- src/qemu/qemu_process.h | 7 ++++- 5 files changed, 79 insertions(+), 34 deletions(-)
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 638ad02..f85e876 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3412,6 +3412,10 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, } break;
+ case QEMU_ASYNC_JOB_START: + /* Already handled in VIR_DOMAIN_PAUSED_STARTING_UP check. */ + break; + case QEMU_ASYNC_JOB_NONE: case QEMU_ASYNC_JOB_LAST: break; @@ -4197,10 +4201,34 @@ qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, }
+int +qemuProcessBeginJob(virQEMUDriverPtr driver, + virDomainObjPtr vm)
At least this one deserves a comment. You should state the fact that users shall use QEMU_ASYNC_JOB_START for the corresponding functions later on.
+{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_START) < 0) + return -1; + + qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); + priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED; + + return 0; +}
ACK
On Thu, Nov 12, 2015 at 07:37:08PM +0100, Jiri Denemark wrote:
We only started an async job for incoming migration from another host. When we were starting a domain from scratch or restoring from a saved state (migration from file) we didn't set any async job. Let's introduce a new QEMU_ASYNC_JOB_START for these cases.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
After this patch was applied to get, libvirtd starting printing out warnings for me when booting guests 2015-11-23 17:55:49.878+0000: 32094: warning : qemuDomainObjEnterMonitorInternal:1750 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 12 ++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 +++++ 4 files changed, 45 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 49d4aa2..f63c4ea 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3811,3 +3811,15 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, return ret; } + + +int +qemuMonitorMigrateIncoming(qemuMonitorPtr mon, + const char *uri) +{ + VIR_DEBUG("uri=%s", uri); + + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONMigrateIncoming(mon, uri); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2ce3958..ed4bd70 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -924,6 +924,9 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, virHashTablePtr *info) ATTRIBUTE_NONNULL(2); +int qemuMonitorMigrateIncoming(qemuMonitorPtr mon, + const char *uri); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b39b29b..86b8c7b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6627,3 +6627,28 @@ qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon, VIR_FREE(linkname); return ret; } + + +int +qemuMonitorJSONMigrateIncoming(qemuMonitorPtr mon, + const char *uri) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("migrate-incoming", + "s:uri", uri, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + ret = qemuMonitorJSONCheckError(cmd, reply); + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 120bd93..374c8ea 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -480,4 +480,9 @@ int qemuMonitorJSONFindLinkPath(qemuMonitorPtr mon, const char *name, char **path) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +int qemuMonitorJSONMigrateIncoming(qemuMonitorPtr mon, + const char *uri) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:09 +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 12 ++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 +++++ 4 files changed, 45 insertions(+)
ACK
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. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 5 ++++ src/qemu/qemu_process.c | 12 ++++++++++ src/qemu/qemu_process.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + 7 files changed, 61 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7475298..2813212 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -299,6 +299,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "e1000", "virtio-net", "gic-version", + + "incoming-defer", /* 200 */ ); @@ -1458,6 +1460,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "nbd-server-start", QEMU_CAPS_NBD_SERVER }, { "change-backing-file", QEMU_CAPS_CHANGE_BACKING_FILE }, { "rtc-reset-reinjection", QEMU_CAPS_RTC_RESET_REINJECTION }, + { "migrate-incoming", QEMU_CAPS_INCOMING_DEFER }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 14541f6..e3e40e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -325,6 +325,9 @@ typedef enum { QEMU_CAPS_DEVICE_VIRTIO_NET, /* -device virtio-net-* */ QEMU_CAPS_MACH_VIRT_GIC_VERSION, /* -machine virt,gic-version */ + /* 200 */ + QEMU_CAPS_INCOMING_DEFER, /* -incoming defer and migrate_incoming */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d5b966..0c4c94a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2951,6 +2951,42 @@ qemuMigrationIncomingURI(const char *migrateFrom, } +int +qemuMigrationRunIncoming(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *uri, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + int rv; + + VIR_DEBUG("Setting up incoming migration with URI %s", uri); + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rv = qemuMonitorMigrateIncoming(priv->mon, uri); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) + goto cleanup; + + if (asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) { + /* qemuMigrationWaitForDestCompletion is called from the Finish phase */ + ret = 0; + goto cleanup; + } + + if (qemuMigrationWaitForDestCompletion(driver, vm, asyncJob) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} + + /* This is called for outgoing non-p2p migrations when a connection to the * client which initiated the migration was closed but we were waiting for it * to follow up with the next phase, that is, in between diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index ff4fe30..2445e13 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -205,4 +205,9 @@ int qemuMigrationCheckIncoming(virQEMUCapsPtr qemuCaps, char *qemuMigrationIncomingURI(const char *migrateFrom, int migrateFd); +int qemuMigrationRunIncoming(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *uri, + qemuDomainAsyncJob asyncJob); + #endif /* __QEMU_MIGRATION_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f85e876..3f236d4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4168,6 +4168,7 @@ qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc) return; VIR_FREE(inc->launchURI); + VIR_FREE(inc->deferredURI); VIR_FREE(inc); } @@ -4190,6 +4191,12 @@ qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, if (!inc->launchURI) goto error; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_INCOMING_DEFER)) { + inc->deferredURI = inc->launchURI; + if (VIR_STRDUP(inc->launchURI, "defer") < 0) + goto error; + } + inc->fd = fd; inc->path = path; @@ -4929,6 +4936,11 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) goto error; + if (incoming && + incoming->deferredURI && + qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) + goto error; + if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) { VIR_DEBUG("Starting domain CPUs"); /* Allow the CPUS to start executing */ diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index dcba728..dcb7e28 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -48,6 +48,7 @@ typedef struct _qemuProcessIncomingDef qemuProcessIncomingDef; typedef qemuProcessIncomingDef *qemuProcessIncomingDefPtr; struct _qemuProcessIncomingDef { char *launchURI; /* used as a parameter for -incoming command line option */ + char *deferredURI; /* used when calling migrate-incoming QMP command */ int fd; /* for fd:N URI */ const char *path; /* path associated with fd */ }; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps index 0d1b1c0..6694b7d 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps @@ -161,4 +161,5 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='incoming-defer'/> </qemuCaps> -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:10 +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.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 5 ++++ src/qemu/qemu_process.c | 12 ++++++++++ src/qemu/qemu_process.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + 7 files changed, 61 insertions(+)
[...]
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d5b966..0c4c94a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2951,6 +2951,42 @@ qemuMigrationIncomingURI(const char *migrateFrom, }
+int +qemuMigrationRunIncoming(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *uri, + qemuDomainAsyncJob asyncJob)
Is there a reason why this doesn't take the new fancy struct containing all the data and doing the decisions internally ...
+{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + int rv; + + VIR_DEBUG("Setting up incoming migration with URI %s", uri); + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rv = qemuMonitorMigrateIncoming(priv->mon, uri); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) + goto cleanup; + + if (asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) { + /* qemuMigrationWaitForDestCompletion is called from the Finish phase */ + ret = 0; + goto cleanup; + } + + if (qemuMigrationWaitForDestCompletion(driver, vm, asyncJob) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} + + diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f85e876..3f236d4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[...]
@@ -4929,6 +4936,11 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) goto error;
+ if (incoming && + incoming->deferredURI &&
... rather than having to check here ...
+ qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0)
And extract the params?
+ goto error; + if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) { VIR_DEBUG("Starting domain CPUs"); /* Allow the CPUS to start executing */
The rest looks reasonable.
On Fri, Nov 13, 2015 at 11:25:14 +0100, Peter Krempa wrote:
On Thu, Nov 12, 2015 at 19:37:10 +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.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 5 ++++ src/qemu/qemu_process.c | 12 ++++++++++ src/qemu/qemu_process.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + 7 files changed, 61 insertions(+)
[...]
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d5b966..0c4c94a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2951,6 +2951,42 @@ qemuMigrationIncomingURI(const char *migrateFrom, }
+int +qemuMigrationRunIncoming(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *uri, + qemuDomainAsyncJob asyncJob)
Is there a reason why this doesn't take the new fancy struct containing all the data and doing the decisions internally ...
...
@@ -4929,6 +4936,11 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) goto error;
+ if (incoming && + incoming->deferredURI &&
... rather than having to check here ...
+ qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0)
And extract the params?
Yeah, there are two reasons. First, I wrote this commit before the one which introduces qemuProcessIncomingDef :-) Second, I wanted to make the decision in the caller so that it is more obvious that qemuMigrationRunIncoming is not going to be run at this point in some cases because incoming migration was already initiated by qemuProcessLaunch (since QEMU does not support -incoming defer). None of the reasons is particularly strong though and if you think I should still change after knowing the reasons, I'll happily do that. Jirka
On Fri, Nov 13, 2015 at 19:52:20 +0100, Jiri Denemark wrote:
On Fri, Nov 13, 2015 at 11:25:14 +0100, Peter Krempa wrote:
On Thu, Nov 12, 2015 at 19:37:10 +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.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 5 ++++ src/qemu/qemu_process.c | 12 ++++++++++ src/qemu/qemu_process.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + 7 files changed, 61 insertions(+)
[...]
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d5b966..0c4c94a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2951,6 +2951,42 @@ qemuMigrationIncomingURI(const char *migrateFrom, }
+int +qemuMigrationRunIncoming(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *uri, + qemuDomainAsyncJob asyncJob)
Is there a reason why this doesn't take the new fancy struct containing all the data and doing the decisions internally ...
...
@@ -4929,6 +4936,11 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) goto error;
+ if (incoming && + incoming->deferredURI &&
... rather than having to check here ...
+ qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0)
And extract the params?
Yeah, there are two reasons. First, I wrote this commit before the one which introduces qemuProcessIncomingDef :-) Second, I wanted to make the
I thought so :)
decision in the caller so that it is more obvious that qemuMigrationRunIncoming is not going to be run at this point in some cases because incoming migration was already initiated by qemuProcessLaunch (since QEMU does not support -incoming defer).
None of the reasons is particularly strong though and if you think I should still change after knowing the reasons, I'll happily do that.
Actually I don't care that much, do whichever you like. ACK Peter
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3f236d4..009c729 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4237,8 +4237,8 @@ int qemuProcessStart(virConnectPtr conn, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, const char *migrateFrom, - int stdin_fd, - const char *stdin_path, + int migrateFd, + const char *migratePath, virDomainSnapshotObjPtr snapshot, virNetDevVPortProfileOp vmop, unsigned int flags) @@ -4263,10 +4263,10 @@ int qemuProcessStart(virConnectPtr conn, char *tmppath = NULL; qemuProcessIncomingDefPtr incoming = NULL; - VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s stdin_fd=%d " - "stdin_path=%s snapshot=%p vmop=%d flags=0x%x", + 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), - stdin_fd, NULLSTR(stdin_path), snapshot, vmop, flags); + migrateFd, NULLSTR(migratePath), snapshot, vmop, flags); /* Okay, these are just internal flags, * but doesn't hurt to check */ @@ -4591,7 +4591,7 @@ int qemuProcessStart(virConnectPtr conn, if (migrateFrom) { incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, - stdin_fd, stdin_path); + migrateFd, migratePath); if (!incoming) goto error; } @@ -4775,7 +4775,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, stdin_path) < 0) + vm->def, migratePath) < 0) goto error; /* Security manager labeled all devices, therefore @@ -4784,7 +4784,7 @@ int qemuProcessStart(virConnectPtr conn, * (hidden under qemuProcessStop) we need to restore labels. */ stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL; - if (stdin_fd != -1) { + if (migrateFd != -1) { /* if there's an fd to migrate from, and it's a pipe, put the * proper security label on it */ @@ -4792,13 +4792,13 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("setting security label on pipe used for migration"); - if (fstat(stdin_fd, &stdin_sb) < 0) { + if (fstat(migrateFd, &stdin_sb) < 0) { virReportSystemError(errno, - _("cannot stat fd %d"), stdin_fd); + _("cannot stat fd %d"), migrateFd); goto error; } if (S_ISFIFO(stdin_sb.st_mode) && - virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, stdin_fd) < 0) + virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, migrateFd) < 0) goto error; } -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:11 +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
ACK
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 80 ++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 009c729..112e561 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4232,6 +4232,29 @@ qemuProcessEndJob(virQEMUDriverPtr driver, } +static int +qemuProcessStartHook(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virHookQemuOpType op, + virHookSubopType subop) +{ + char *xml; + int ret; + + if (!virHookPresent(VIR_HOOK_DRIVER_QEMU)) + return 0; + + if (!(xml = qemuDomainDefFormatXML(driver, vm->def, 0))) + return -1; + + ret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, op, subop, + NULL, xml, NULL); + VIR_FREE(xml); + + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4327,21 +4350,10 @@ int qemuProcessStart(virConnectPtr conn, driver->inhibitCallback(true, driver->inhibitOpaque); /* Run an early hook to set-up missing devices */ - if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { - char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); - int hookret; - - hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, - VIR_HOOK_QEMU_OP_PREPARE, VIR_HOOK_SUBOP_BEGIN, - NULL, xml, NULL); - VIR_FREE(xml); - - /* - * If the script raised an error abort the launch - */ - if (hookret < 0) - goto error; - } + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_PREPARE, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto error; VIR_DEBUG("Determining emulator version"); virObjectUnref(priv->qemuCaps); @@ -4644,21 +4656,10 @@ int qemuProcessStart(virConnectPtr conn, VIR_FREE(tmppath); /* now that we know it is about to start call the hook if present */ - if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { - char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); - int hookret; - - hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, - VIR_HOOK_QEMU_OP_START, VIR_HOOK_SUBOP_BEGIN, - NULL, xml, NULL); - VIR_FREE(xml); - - /* - * If the script raised an error abort the launch - */ - if (hookret < 0) - goto error; - } + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_START, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto error; qemuLogOperation(vm, "starting up", logfile, cmd); @@ -4968,21 +4969,10 @@ int qemuProcessStart(virConnectPtr conn, goto error; /* finally we can call the 'started' hook script if any */ - if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { - char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); - int hookret; - - hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, - VIR_HOOK_QEMU_OP_STARTED, VIR_HOOK_SUBOP_BEGIN, - NULL, xml, NULL); - VIR_FREE(xml); - - /* - * If the script raised an error abort the launch - */ - if (hookret < 0) - goto error; - } + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_STARTED, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto error; /* Keep watching qemu log for errors during incoming migration, otherwise * unset reporting errors from qemu log. */ -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:12 +0100, Jiri Denemark wrote:
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 80 ++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 45 deletions(-)
ACK
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 147 ++++++++++++++++++++++++++---------------------- 1 file changed, 81 insertions(+), 66 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 112e561..4b871a8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4255,6 +4255,84 @@ qemuProcessStartHook(virQEMUDriverPtr driver, } +static int +qemuProcessSetupGraphics(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + size_t i; + int ret = -1; + + for (i = 0; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.autoport) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.vnc.port, + true) < 0) + goto cleanup; + graphics->data.vnc.portReserved = true; + + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + !graphics->data.spice.autoport) { + if (graphics->data.spice.port > 0) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.port, + true) < 0) + goto cleanup; + graphics->data.spice.portReserved = true; + } + + if (graphics->data.spice.tlsPort > 0) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.tlsPort, + true) < 0) + goto cleanup; + graphics->data.spice.tlsPortReserved = true; + } + } + } + + for (i = 0; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + if (qemuProcessVNCAllocatePorts(driver, graphics) < 0) + goto cleanup; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, true) < 0) + goto cleanup; + } + + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + if (graphics->nListens == 0) { + if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) + goto cleanup; + graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; + if (VIR_STRDUP(graphics->listens[0].address, + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ? + cfg->vncListen : cfg->spiceListen) < 0) { + VIR_SHRINK_N(graphics->listens, graphics->nListens, 1); + goto cleanup; + } + graphics->listens[0].fromConfig = true; + } else if (graphics->nListens > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU does not support multiple listen " + "addresses for one graphics device.")); + goto cleanup; + } + } + } + + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4423,72 +4501,9 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(driver, vm); - for (i = 0; i < vm->def->ngraphics; ++i) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !graphics->data.vnc.autoport) { - if (virPortAllocatorSetUsed(driver->remotePorts, - graphics->data.vnc.port, - true) < 0) { - goto error; - } - - graphics->data.vnc.portReserved = true; - - } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - !graphics->data.spice.autoport) { - - if (graphics->data.spice.port > 0) { - if (virPortAllocatorSetUsed(driver->remotePorts, - graphics->data.spice.port, - true) < 0) - goto error; - - graphics->data.spice.portReserved = true; - } - - if (graphics->data.spice.tlsPort > 0) { - if (virPortAllocatorSetUsed(driver->remotePorts, - graphics->data.spice.tlsPort, - true) < 0) - goto error; - - graphics->data.spice.tlsPortReserved = true; - } - } - } - - for (i = 0; i < vm->def->ngraphics; ++i) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - if (qemuProcessVNCAllocatePorts(driver, graphics) < 0) - goto error; - } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, true) < 0) - goto error; - } - - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - if (graphics->nListens == 0) { - if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) - goto error; - graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (VIR_STRDUP(graphics->listens[0].address, - graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ? - cfg->vncListen : cfg->spiceListen) < 0) { - VIR_SHRINK_N(graphics->listens, graphics->nListens, 1); - goto error; - } - graphics->listens[0].fromConfig = true; - } else if (graphics->nListens > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("QEMU does not support multiple listen " - "addresses for one graphics device.")); - goto error; - } - } - } + VIR_DEBUG("Setting up ports for graphics"); + if (qemuProcessSetupGraphics(driver, vm) < 0) + goto error; if (virFileMakePath(cfg->logDir) < 0) { virReportSystemError(errno, -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:13 +0100, Jiri Denemark wrote:
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 147 ++++++++++++++++++++++++++---------------------- 1 file changed, 81 insertions(+), 66 deletions(-)
ACK
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 105 ++++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4b871a8..d5f6750 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4333,6 +4333,65 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, } +static int +qemuProcessSetupRawIO(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd ATTRIBUTE_UNUSED) +{ + bool rawio = false; + size_t i; + int ret = -1; + + /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */ + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDeviceDef dev; + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (disk->rawio == VIR_TRISTATE_BOOL_YES) { + rawio = true; +#ifndef CAP_SYS_RAWIO + break; +#endif + } + + dev.type = VIR_DOMAIN_DEVICE_DISK; + dev.data.disk = disk; + if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0) + goto cleanup; + + if (qemuSetUnprivSGIO(&dev) < 0) + goto cleanup; + } + + /* If rawio not already set, check hostdevs as well */ + if (!rawio) { + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevSubsysSCSIPtr scsisrc = + &vm->def->hostdevs[i]->source.subsys.u.scsi; + if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) { + rawio = true; + break; + } + } + } + + ret = 0; + + cleanup: + if (rawio) { +#ifdef CAP_SYS_RAWIO + if (ret == 0) + virCommandAllowCap(cmd, CAP_SYS_RAWIO); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Raw I/O is not supported on this platform")); + ret = -1; +#endif + } + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4353,7 +4412,6 @@ int qemuProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; size_t i; - bool rawio_set = false; char *nodeset = NULL; unsigned int stop_flags; virQEMUDriverConfigPtr cfg; @@ -4689,48 +4747,9 @@ int qemuProcessStart(virConnectPtr conn, if (cfg->clearEmulatorCapabilities) virCommandClearCaps(cmd); - /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */ - for (i = 0; i < vm->def->ndisks; i++) { - virDomainDeviceDef dev; - virDomainDiskDefPtr disk = vm->def->disks[i]; - - if (vm->def->disks[i]->rawio == VIR_TRISTATE_BOOL_YES) { -#ifdef CAP_SYS_RAWIO - virCommandAllowCap(cmd, CAP_SYS_RAWIO); - rawio_set = true; -#else - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Raw I/O is not supported on this platform")); - goto error; -#endif - } - - dev.type = VIR_DOMAIN_DEVICE_DISK; - dev.data.disk = disk; - if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0) - goto error; - - if (qemuSetUnprivSGIO(&dev) < 0) - goto error; - } - - /* If rawio not already set, check hostdevs as well */ - if (!rawio_set) { - for (i = 0; i < vm->def->nhostdevs; i++) { - virDomainHostdevSubsysSCSIPtr scsisrc = - &vm->def->hostdevs[i]->source.subsys.u.scsi; - if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) { -#ifdef CAP_SYS_RAWIO - virCommandAllowCap(cmd, CAP_SYS_RAWIO); - break; -#else - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Raw I/O is not supported on this platform")); - goto error; -#endif - } - } - } + VIR_DEBUG("Setting up raw IO"); + if (qemuProcessSetupRawIO(driver, vm, cmd) < 0) + goto error; virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses); -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:14 +0100, Jiri Denemark wrote:
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 105 ++++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4b871a8..d5f6750 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4333,6 +4333,65 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, }
+static int +qemuProcessSetupRawIO(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCommandPtr cmd ATTRIBUTE_UNUSED) +{ + bool rawio = false; + size_t i; + int ret = -1; + + /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */ + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDeviceDef dev; + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (disk->rawio == VIR_TRISTATE_BOOL_YES) { + rawio = true; +#ifndef CAP_SYS_RAWIO
Yuck!, but pre-existing ...
+ break; +#endif + } + + dev.type = VIR_DOMAIN_DEVICE_DISK; + dev.data.disk = disk; + if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0) + goto cleanup; + + if (qemuSetUnprivSGIO(&dev) < 0) + goto cleanup; + } + + /* If rawio not already set, check hostdevs as well */ + if (!rawio) { + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevSubsysSCSIPtr scsisrc = + &vm->def->hostdevs[i]->source.subsys.u.scsi; + if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) { + rawio = true; + break; + } + } + } + + ret = 0; + + cleanup: + if (rawio) { +#ifdef CAP_SYS_RAWIO + if (ret == 0) + virCommandAllowCap(cmd, CAP_SYS_RAWIO); +#else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Raw I/O is not supported on this platform")); + ret = -1; +#endif + } + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4353,7 +4412,6 @@ int qemuProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; size_t i; - bool rawio_set = false; char *nodeset = NULL; unsigned int stop_flags; virQEMUDriverConfigPtr cfg; @@ -4689,48 +4747,9 @@ int qemuProcessStart(virConnectPtr conn, if (cfg->clearEmulatorCapabilities) virCommandClearCaps(cmd);
- /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */ - for (i = 0; i < vm->def->ndisks; i++) { - virDomainDeviceDef dev; - virDomainDiskDefPtr disk = vm->def->disks[i]; - - if (vm->def->disks[i]->rawio == VIR_TRISTATE_BOOL_YES) { -#ifdef CAP_SYS_RAWIO - virCommandAllowCap(cmd, CAP_SYS_RAWIO); - rawio_set = true; -#else - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Raw I/O is not supported on this platform")); - goto error; -#endif
... but the new code is possibly less-yuck than this.
- } - - dev.type = VIR_DOMAIN_DEVICE_DISK; - dev.data.disk = disk; - if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0) - goto error; - - if (qemuSetUnprivSGIO(&dev) < 0) - goto error; - } - - /* If rawio not already set, check hostdevs as well */ - if (!rawio_set) { - for (i = 0; i < vm->def->nhostdevs; i++) { - virDomainHostdevSubsysSCSIPtr scsisrc = - &vm->def->hostdevs[i]->source.subsys.u.scsi; - if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) { -#ifdef CAP_SYS_RAWIO - virCommandAllowCap(cmd, CAP_SYS_RAWIO); - break; -#else - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Raw I/O is not supported on this platform")); - goto error; -#endif - } - } - } + VIR_DEBUG("Setting up raw IO"); + if (qemuProcessSetupRawIO(driver, vm, cmd) < 0) + goto error;
virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
We probably should add a function that will be conditionally compiled and determinign whether rawio works or not so that the code can be cleaner. This is a improvement compared to the old code though, so ... ACK. Peter
Move {Enter,Exit}Monitor calls inside qemuProcessSetLinkStates to simplify qemuProcessStart. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d5f6750..de5c9f8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2233,19 +2233,25 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) /* set link states to down on interfaces at qemu start */ static int -qemuProcessSetLinkStates(virDomainObjPtr vm) +qemuProcessSetLinkStates(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; size_t i; - int ret = 0; + int ret = -1; + int rv; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; for (i = 0; i < def->nnets; i++) { if (def->nets[i]->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { if (!def->nets[i]->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing alias for network device")); - return -1; + goto cleanup; } VIR_DEBUG("Setting link state: %s", def->nets[i]->info.alias); @@ -2253,20 +2259,26 @@ qemuProcessSetLinkStates(virDomainObjPtr vm) if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Setting of link state is not supported by this qemu")); - return -1; + goto cleanup; } - ret = qemuMonitorSetLink(priv->mon, - def->nets[i]->info.alias, - VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN); - if (ret != 0) { + rv = qemuMonitorSetLink(priv->mon, + def->nets[i]->info.alias, + VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN); + if (rv < 0) { virReportError(VIR_ERR_OPERATION_FAILED, - _("Couldn't set link state on interface: %s"), def->nets[i]->info.alias); - break; + _("Couldn't set link state on interface: %s"), + def->nets[i]->info.alias); + goto cleanup; } } } + ret = 0; + + cleanup: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; return ret; } @@ -4926,11 +4938,7 @@ int qemuProcessStart(virConnectPtr conn, /* qemu doesn't support setting this on the command line, so * enter the monitor */ VIR_DEBUG("Setting network link states"); - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto error; - if (qemuProcessSetLinkStates(vm) < 0) - goto exit_monitor; - if (qemuDomainObjExitMonitor(driver, vm)) + if (qemuProcessSetLinkStates(driver, vm, asyncJob) < 0) goto error; VIR_DEBUG("Fetching list of active devices"); -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:15 +0100, Jiri Denemark wrote:
Move {Enter,Exit}Monitor calls inside qemuProcessSetLinkStates to simplify qemuProcessStart.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-)
ACK
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 56 ++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index de5c9f8..1f10ac9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4404,6 +4404,39 @@ qemuProcessSetupRawIO(virQEMUDriverPtr driver, } +static int +qemuProcessSetupBalloon(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + unsigned long long balloon = vm->def->mem.cur_balloon; + qemuDomainObjPrivatePtr priv = vm->privateData; + int period; + int ret = -1; + + if (!vm->def->memballoon || + vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) + return 0; + + period = vm->def->memballoon->period; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; + + if (period) + qemuMonitorSetMemoryStatsPeriod(priv->mon, period); + if (qemuMonitorSetBalloon(priv->mon, balloon) < 0) + goto cleanup; + + ret = 0; + + cleanup: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4950,23 +4983,8 @@ int qemuProcessStart(virConnectPtr conn, goto error; VIR_DEBUG("Setting initial memory amount"); - if (vm->def->memballoon && - vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) { - unsigned long long balloon = vm->def->mem.cur_balloon; - int period = vm->def->memballoon->period; - - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto error; - - if (period) - qemuMonitorSetMemoryStatsPeriod(priv->mon, period); - - if (qemuMonitorSetBalloon(priv->mon, balloon) < 0) - goto exit_monitor; - - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto error; - } + if (qemuProcessSetupBalloon(driver, vm, asyncJob) < 0) + goto error; /* 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 @@ -5042,10 +5060,6 @@ int qemuProcessStart(virConnectPtr conn, qemuMonitorSetDomainLog(priv->mon, -1); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags); goto cleanup; - - exit_monitor: - ignore_value(qemuDomainObjExitMonitor(driver, vm)); - goto error; } -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:16 +0100, Jiri Denemark wrote:
qemuProcessStart is so big that any nontrivial code should be moved to dedicated functions to make the code easier to read and maintain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 56 ++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 21 deletions(-)
ACK
qemuProcessMakeDir is used for creating a per-domain directory in a given parent directory. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 59 +++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1f10ac9..a02754d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4437,6 +4437,34 @@ qemuProcessSetupBalloon(virQEMUDriverPtr driver, } +static int +qemuProcessMakeDir(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *parentDir) +{ + char *path = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/domain-%s", parentDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePathWithMode(path, 0750) < 0) { + virReportSystemError(errno, _("Cannot create directory '%s'"), path); + goto cleanup; + } + + if (virSecurityManagerDomainSetDirLabel(driver->securityManager, + vm->def, path) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(path); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4464,7 +4492,6 @@ int qemuProcessStart(virConnectPtr conn, unsigned int hostdev_flags = 0; size_t nnicindexes = 0; int *nicindexes = NULL; - char *tmppath = NULL; qemuProcessIncomingDefPtr incoming = NULL; VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s migrateFd=%d " @@ -4744,35 +4771,10 @@ int qemuProcessStart(virConnectPtr conn, * Create all per-domain directories in order to make sure domain * with any possible seclabels can access it. */ - if (virAsprintf(&tmppath, "%s/domain-%s", cfg->libDir, vm->def->name) < 0) + if (qemuProcessMakeDir(driver, vm, cfg->libDir) < 0 || + qemuProcessMakeDir(driver, vm, cfg->channelTargetDir) < 0) goto error; - if (virFileMakePathWithMode(tmppath, 0750) < 0) { - virReportSystemError(errno, _("Cannot create directory '%s'"), tmppath); - goto error; - } - - if (virSecurityManagerDomainSetDirLabel(driver->securityManager, - vm->def, tmppath) < 0) - goto error; - - VIR_FREE(tmppath); - - if (virAsprintf(&tmppath, "%s/domain-%s", - cfg->channelTargetDir, vm->def->name) < 0) - goto error; - - if (virFileMakePathWithMode(tmppath, 0750) < 0) { - virReportSystemError(errno, _("Cannot create directory '%s'"), tmppath); - goto error; - } - - if (virSecurityManagerDomainSetDirLabel(driver->securityManager, - vm->def, tmppath) < 0) - goto error; - - VIR_FREE(tmppath); - /* now that we know it is about to start call the hook if present */ if (qemuProcessStartHook(driver, vm, VIR_HOOK_QEMU_OP_START, @@ -5048,7 +5050,6 @@ int qemuProcessStart(virConnectPtr conn, virObjectUnref(caps); VIR_FREE(nicindexes); VIR_FREE(nodeset); - VIR_FREE(tmppath); qemuProcessIncomingDefFree(incoming); return ret; -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:17 +0100, Jiri Denemark wrote:
qemuProcessMakeDir is used for creating a per-domain directory in a given parent directory.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 59 +++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 29 deletions(-)
ACK
Every caller setting migrateFrom already sets VIR_QEMU_PROCESS_START_PAUSED flag anyway. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a02754d..d424d76 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4889,9 +4889,6 @@ int qemuProcessStart(virConnectPtr conn, goto error; VIR_DEBUG("Handshake complete, child running"); - if (incoming) - flags |= VIR_QEMU_PROCESS_START_PAUSED; - if (rv == -1) /* The VM failed to start; tear filters before taps */ virDomainConfVMNWFilterTeardown(vm); -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:18 +0100, Jiri Denemark wrote:
Every caller setting migrateFrom already sets VIR_QEMU_PROCESS_START_PAUSED flag anyway.
Indeed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 3 --- 1 file changed, 3 deletions(-)
ACK
Remembering to call qemuMonitorSetDomainLog in the right paths before calling qemuProcessStop is annoying and easy to forget. And I already forget to do so in commit v1.2.8-52-g0389060: logfd may be leaked if QEMU process dies between Prepare and Finish migration phases. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 2 ++ src/qemu/qemu_process.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f63c4ea..50c6549 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -960,6 +960,8 @@ qemuMonitorClose(qemuMonitorPtr mon) PROBE(QEMU_MONITOR_CLOSE, "mon=%p refs=%d", mon, mon->parent.parent.u.s.refs); + qemuMonitorSetDomainLog(mon, -1); + if (mon->fd >= 0) { if (mon->watch) { virEventRemoveHandle(mon->watch); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d424d76..71cfa2e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5054,8 +5054,6 @@ int qemuProcessStart(virConnectPtr conn, /* 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 */ - if (priv->mon) - qemuMonitorSetDomainLog(priv->mon, -1); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags); goto cleanup; } -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:19 +0100, Jiri Denemark wrote:
Remembering to call qemuMonitorSetDomainLog in the right paths before calling qemuProcessStop is annoying and easy to forget. And I already forget to do so in commit v1.2.8-52-g0389060: logfd may be leaked if
forgot
QEMU process dies between Prepare and Finish migration phases.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 2 ++ src/qemu/qemu_process.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-)
ACK
On Fri, Nov 13, 2015 at 14:07:42 +0100, Peter Krempa wrote:
On Thu, Nov 12, 2015 at 19:37:19 +0100, Jiri Denemark wrote:
Remembering to call qemuMonitorSetDomainLog in the right paths before calling qemuProcessStop is annoying and easy to forget. And I already forget to do so in commit v1.2.8-52-g0389060: logfd may be leaked if
forgot
QEMU process dies between Prepare and Finish migration phases.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 2 ++ src/qemu/qemu_process.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-)
ACK
I made the suggested changes and pushed the first 17 patches of this series. I'll send a v2 of the rest soon. Thanks. Jirka
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> --- src/qemu/qemu_process.c | 122 ++++++++++++++++++++++++++++++------------------ src/qemu/qemu_process.h | 4 ++ 2 files changed, 81 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 71cfa2e..5735935 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4465,6 +4465,79 @@ qemuProcessMakeDir(virQEMUDriverPtr driver, } +/** + * qemuProcessInit: + * + * Prepares the domain up to the point when priv->qemuCaps is initialized. + */ +int +qemuProcessInit(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool migration) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + 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 cleanup; + + /* 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 cleanup; + + /* 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 cleanup; + + 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 cleanup; + + VIR_DEBUG("Determining emulator version"); + virObjectUnref(priv->qemuCaps); + if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, + vm->def->emulator, + vm->def->os.machine))) + goto cleanup; + + ret = 0; + + cleanup: + virObjectUnref(cfg); + virObjectUnref(caps); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4487,7 +4560,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; @@ -4505,6 +4578,9 @@ int qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, -1); + if (qemuProcessInit(driver, vm, !!migrateFrom) < 0) + goto error; + cfg = virQEMUDriverGetConfig(driver); /* From now on until domain security labeling is done: @@ -4523,53 +4599,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
On Thu, Nov 12, 2015 at 19:37:20 +0100, Jiri Denemark wrote:
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> --- src/qemu/qemu_process.c | 122 ++++++++++++++++++++++++++++++------------------ src/qemu/qemu_process.h | 4 ++ 2 files changed, 81 insertions(+), 45 deletions(-)
ACK
Once qemuProcessInit was called, qemuProcessLaunch will launch a new QEMU process with stopped virtual CPUs. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 162 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 9 +++ 2 files changed, 118 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5735935..0314c4a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4538,16 +4538,20 @@ 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. + */ +int +qemuProcessLaunch(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuProcessIncomingDefPtr incoming, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags) { int ret = -1; int rv; @@ -4560,17 +4564,22 @@ int qemuProcessStart(virConnectPtr conn, 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 */ @@ -4578,9 +4587,6 @@ int qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, -1); - if (qemuProcessInit(driver, vm, !!migrateFrom) < 0) - goto error; - cfg = virQEMUDriverGetConfig(driver); /* From now on until domain security labeling is done: @@ -4590,7 +4596,7 @@ int qemuProcessStart(virConnectPtr conn, 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) + if (incoming) stop_flags |= VIR_QEMU_PROCESS_STOP_MIGRATED; hookData.conn = conn; @@ -4614,7 +4620,7 @@ int qemuProcessStart(virConnectPtr conn, 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) @@ -4715,7 +4721,7 @@ int qemuProcessStart(virConnectPtr conn, goto error; } - if (!migrateFrom && !snapshot && + if (!incoming && !snapshot && virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) goto error; @@ -4778,13 +4784,6 @@ int qemuProcessStart(virConnectPtr conn, goto error; } - if (migrateFrom) { - incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, - migrateFd, migratePath); - if (!incoming) - goto error; - } - VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON, priv->qemuCaps, @@ -4888,8 +4887,9 @@ int qemuProcessStart(virConnectPtr conn, goto error; VIR_DEBUG("Setting domain security labels"); - if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, migratePath) < 0) + if (incoming && + virSecurityManagerSetAllLabel(driver->securityManager, + vm->def, incoming->path) < 0) goto error; /* Security manager labeled all devices, therefore @@ -4898,7 +4898,7 @@ int qemuProcessStart(virConnectPtr conn, * (hidden under qemuProcessStop) we need to restore labels. */ stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL; - 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 */ @@ -4906,13 +4906,14 @@ 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); + _("cannot stat fd %d"), incoming->fd); goto error; } if (S_ISFIFO(stdin_sb.st_mode) && - virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, migrateFd) < 0) + virSecurityManagerSetImageFDLabel(driver->securityManager, + vm->def, incoming->fd) < 0) goto error; } @@ -5028,6 +5029,73 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) goto error; + if (flags & VIR_QEMU_PROCESS_START_AUTODESTROY && + qemuProcessAutoDestroyAdd(driver, vm, conn) < 0) + goto error; + + ret = 0; + + cleanup: + virCommandFree(cmd); + VIR_FORCE_CLOSE(logfile); + virObjectUnref(cfg); + virObjectUnref(caps); + VIR_FREE(nicindexes); + VIR_FREE(nodeset); + 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); + goto cleanup; +} + + +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; + int ret = -1; + + 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 error; + + if (migrateFrom) { + incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, + migrateFd, migratePath); + if (!incoming) + goto error; + } + + if (qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, + snapshot, vmop, flags) < 0) + goto error; + if (incoming && incoming->deferredURI && qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) @@ -5051,20 +5119,15 @@ 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; - - /* 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; + VIR_DEBUG("Writing domain status to disk"); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto error; + /* Keep watching qemu log for errors during incoming migration, otherwise * unset reporting errors from qemu log. */ if (!incoming) @@ -5073,20 +5136,13 @@ 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); + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + incoming ? VIR_QEMU_PROCESS_STOP_MIGRATED : 0); 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
On Thu, Nov 12, 2015 at 19:37:21 +0100, Jiri Denemark wrote:
Once qemuProcessInit was called, qemuProcessLaunch will launch a new QEMU process with stopped virtual CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 162 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 9 +++ 2 files changed, 118 insertions(+), 53 deletions(-)
[...]
@@ -5051,20 +5119,15 @@ 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; - - /* 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;
+ VIR_DEBUG("Writing domain status to disk"); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto error;
I think that the order of calling the VIR_HOOK_QEMU_OP_STARTED hook should not be changed with status XML saving since that may still jump to the path where the VM will be killed off.
+ /* Keep watching qemu log for errors during incoming migration, otherwise * unset reporting errors from qemu log. */ if (!incoming)
ACK with ^^ fixed.
On 11/12/2015 01:37 PM, Jiri Denemark wrote:
Once qemuProcessInit was called, qemuProcessLaunch will launch a new QEMU process with stopped virtual CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 162 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 9 +++ 2 files changed, 118 insertions(+), 53 deletions(-)
Been following along with the review so far - have a question though...
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5735935..0314c4a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[..]
+int +qemuProcessLaunch(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuProcessIncomingDefPtr incoming, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags)
[...]
VIR_DEBUG("Setting domain security labels"); - if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, migratePath) < 0) + if (incoming && + virSecurityManagerSetAllLabel(driver->securityManager, + vm->def, incoming->path) < 0)
shouldn't this be if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, incoming ? incoming->path : NULL) < 0) Previously we'd call SetAllLabel with migratePath as NULL anyway...
goto error;
[...]
+ + 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);
Doesn't this happen in qemuProcessStart too? IOW would this happen twice?
+ goto cleanup; +}
On Fri, Nov 13, 2015 at 10:17:16 -0500, John Ferlan wrote:
On 11/12/2015 01:37 PM, Jiri Denemark wrote:
Once qemuProcessInit was called, qemuProcessLaunch will launch a new QEMU process with stopped virtual CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 162 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 9 +++ 2 files changed, 118 insertions(+), 53 deletions(-)
Been following along with the review so far - have a question though...
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5735935..0314c4a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[..]
+int +qemuProcessLaunch(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuProcessIncomingDefPtr incoming, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags)
[...]
VIR_DEBUG("Setting domain security labels"); - if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, migratePath) < 0) + if (incoming && + virSecurityManagerSetAllLabel(driver->securityManager, + vm->def, incoming->path) < 0)
shouldn't this be
if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, incoming ? incoming->path : NULL) < 0)
Previously we'd call SetAllLabel with migratePath as NULL anyway...
goto error;
Hmm, right you are. Thanks for catching this.
[...]
+ + 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);
Doesn't this happen in qemuProcessStart too? IOW would this happen twice?
Well, yes, although running it twice does nothing except of logging a "VM not active" debug message. However, a much bigger problem is in the previous patch, which causes qemuProcessStop in case we fail to start a domain because it is already running, not something you'd expect :-) Jirka
Finishes starting a new domain launched by qemuProcessLaunch. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 76 +++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 6 ++++ 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0314c4a..38c4eeb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5053,6 +5053,52 @@ qemuProcessLaunch(virConnectPtr conn, } +/** + * qemuProcessFinish: + * + * Finish starting a new domain. + */ +int +qemuProcessFinish(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + 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, + QEMU_ASYNC_JOB_NONE) < 0) { + if (!virGetLastError()) + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("resume operation failed")); + goto cleanup; + } + } else { + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, pausedReason); + } + + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_STARTED, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto cleanup; + + VIR_DEBUG("Writing domain status to disk"); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, @@ -5101,31 +5147,11 @@ qemuProcessStart(virConnectPtr conn, qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) goto error; - 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, - QEMU_ASYNC_JOB_NONE) < 0) { - if (virGetLastError() == NULL) - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("resume operation failed")); - goto error; - } - } else { - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, - incoming ? - VIR_DOMAIN_PAUSED_MIGRATION : - VIR_DOMAIN_PAUSED_USER); - } - - if (qemuProcessStartHook(driver, vm, - VIR_HOOK_QEMU_OP_STARTED, - VIR_HOOK_SUBOP_BEGIN) < 0) - goto error; - - VIR_DEBUG("Writing domain status to disk"); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + if (qemuProcessFinish(conn, driver, vm, + !(flags & VIR_QEMU_PROCESS_START_PAUSED), + incoming ? + VIR_DOMAIN_PAUSED_MIGRATION : + VIR_DOMAIN_PAUSED_USER) < 0) goto error; /* Keep watching qemu log for errors during incoming migration, otherwise diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 54009c5..6af36ee 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -94,6 +94,12 @@ int qemuProcessLaunch(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags); +int qemuProcessFinish(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool startCPUs, + virDomainPausedReason pausedReason); + typedef enum { VIR_QEMU_PROCESS_STOP_MIGRATED = 1 << 0, VIR_QEMU_PROCESS_STOP_NO_RELABEL = 1 << 1, -- 2.6.3
On Thu, Nov 12, 2015 at 19:37:22 +0100, Jiri Denemark wrote:
Finishes starting a new domain launched by qemuProcessLaunch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 76 +++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 6 ++++ 2 files changed, 57 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0314c4a..38c4eeb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5053,6 +5053,52 @@ qemuProcessLaunch(virConnectPtr conn, }
+/** + * qemuProcessFinish:
The name is misleading. I'd go for qemuProcessFinishStartup or something along that to emphasize that this is tied to the startup operation.
+ * + * Finish starting a new domain. + */ +int +qemuProcessFinish(virConnectPtr conn,
I'm leaned towards insisting on a name change. ACK to the code though.
On 11/12/2015 01:37 PM, Jiri Denemark wrote:
Finishes starting a new domain launched by qemuProcessLaunch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 76 +++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 6 ++++ 2 files changed, 57 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0314c4a..38c4eeb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5053,6 +5053,52 @@ qemuProcessLaunch(virConnectPtr conn, }
[...]
+ + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver,
Seems as though cfg is now unnecessary John
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- 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
On Thu, Nov 12, 2015 at 19:37:23 +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 154 +++++++++++++++++++++++++--------------------- 1 file changed, 85 insertions(+), 69 deletions(-)
ACK
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> --- 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
On Thu, Nov 12, 2015 at 19:37:24 +0100, Jiri Denemark wrote:
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> --- src/qemu/qemu_migration.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
ACK
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> --- 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
On Thu, Nov 12, 2015 at 19:37:25 +0100, Jiri Denemark wrote:
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> --- src/qemu/qemu_migration.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)
ACK
Using qemuProcess{Init,Launch,Finish} 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> --- src/qemu/qemu_migration.c | 48 +++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b6525df..b63f66e 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,7 +3398,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, char *xmlout = NULL; unsigned int cookieFlags; virCapsPtr caps = NULL; - char *migrateFrom = NULL; + qemuProcessIncomingDefPtr incoming = NULL; bool taint_hook = false; virNWFilterReadLockFilterUpdates(); @@ -3528,25 +3533,19 @@ 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; - 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) { + if (qemuProcessLaunch(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, + incoming, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, + VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) { virDomainAuditStart(vm, "migrated", false); goto stopjob; } @@ -3594,6 +3593,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 (qemuProcessFinish(dconn, driver, vm, false, + VIR_DOMAIN_PAUSED_MIGRATION) < 0) + goto stopjob; + done: if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, cookieFlags) < 0) { @@ -3625,7 +3633,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]); -- 2.6.3
participants (4)
-
Daniel P. Berrange -
Jiri Denemark -
John Ferlan -
Peter Krempa