[libvirt] [PATCH v4 0/9] Rework storage migration
Since there are many hidden problems with auto-creating storage for the domain in here, as previous rounds of reviewing the series has shown, I've decided to not do anything about it for now and the auto allocation is dropped completely. So we are back to the assumption we already have - users need to make sure files already exists and have the right size. Hawk. diff to v1: -Eric's and Daniel's suggestions worked in. To point out the bigger ones: don't do NBD style when TUNNELLED requested, added 'b:writable' to 'nbd-server-add' -drop '/qemu-migration/nbd/disk/@src' attribute from migration cookie. As pointed out by Jirka, disk->src can be changed during migration (e.g. by migration hook or by passed xml). So I've tried (as suggested on the list) passing disk alias. However, since qemu hasn't been started on destination yet, the aliases hasn't been generated yet. So we have to rely on ordering completely. diff to v2: -rebase to reflect changes made by offline migration patch -send initial nbd cookie only if needed diff to v2.1: -nbd cookie reworked -don't rely on disk ordering in the cookie, but use disk target for that -adapt to virPortAllocator -unlink pre-created storage on migration fail -other of Jirka's suggestions worked in "diff" to v3: -just rebase & adapt to new qemu code after dropping QDL (Qemu Driver Lock) diff to v4: -drop storage auto-allocation -include John's and Jirka's reviews Note, that most of the patches has been ACKed already. Michal Privoznik (9): qemu: Introduce NBD_SERVER capability Introduce NBD migration cookie qemu: Introduce nbd-server-start command qemu: Introduce nbd-server-add command qemu: Introduce nbd-server-stop command qemu_migration: Introduce qemuMigrationStartNBDServer() qemu_migration: Introduce qemuMigrationDriveMirror qemu_migration: Stop NBD server at Finish phase qemu_migration: Cancel running jobs on failed migration src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_migration.c | 458 +++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 63 ++++++ src/qemu/qemu_monitor.h | 7 + src/qemu/qemu_monitor_json.c | 102 ++++++++++ src/qemu/qemu_monitor_json.h | 7 + src/qemu/qemu_process.c | 5 + 9 files changed, 631 insertions(+), 17 deletions(-) -- 1.8.1.2
This just keeps track whether qemu knows nbd-server-* commands so we can use it during migration or not. --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index af52bbf..5e8f26a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -205,7 +205,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "usb-serial", /* 125 */ "usb-net", "add-fd", - + "nbd-server-start", ); struct _virQEMUCaps { @@ -1940,6 +1940,8 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, virQEMUCapsSet(qemuCaps, QEMU_CAPS_DISK_SNAPSHOT); else if (STREQ(name, "add-fd")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ADD_FD); + else if (STREQ(name, "nbd-server-start")) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NBD_SERVER); VIR_FREE(name); } VIR_FREE(commands); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e69d558..7c55dc8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -166,6 +166,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_SERIAL = 125, /* -device usb-serial */ QEMU_CAPS_DEVICE_USB_NET = 126, /* -device usb-net */ QEMU_CAPS_ADD_FD = 127, /* -add-fd */ + QEMU_CAPS_NBD_SERVER = 128, /* nbd-server-start QMP command */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:18 +0100, Michal Privoznik wrote:
This just keeps track whether qemu knows nbd-server-* commands so we can use it during migration or not. --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index af52bbf..5e8f26a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -205,7 +205,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "usb-serial", /* 125 */ "usb-net", "add-fd", - + "nbd-server-start", );
struct _virQEMUCaps { @@ -1940,6 +1940,8 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, virQEMUCapsSet(qemuCaps, QEMU_CAPS_DISK_SNAPSHOT); else if (STREQ(name, "add-fd")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ADD_FD); + else if (STREQ(name, "nbd-server-start")) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NBD_SERVER);
Hmm, I just noticed the capability is called NBD_SERVER while it's string representation is nbd-server-start. Would it be better to change the string representation to "nbd-server" to make it consistent? It's a minor issue, so ACK with or without the change. Jirka
This migration cookie is meant for two purposes. The first is to be sent in begin phase from source to destination to let it know we support new implementation of VIR_MIGRATE_NON_SHARED_{DISK,INC} so destination can start NBD server. Then, the second purpose is, destination can let us know, on which port the NBD server is running. --- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_migration.c | 131 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 905b099..e4df668 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -154,6 +154,7 @@ struct _qemuDomainObjPrivate { unsigned long migMaxBandwidth; char *origname; + int nbdPort; /* Port used for migration with NBD */ virChrdevsPtr devs; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2f79358..0acc1ac 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -74,6 +74,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT, QEMU_MIGRATION_COOKIE_FLAG_NETWORK, + QEMU_MIGRATION_COOKIE_FLAG_NBD, QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -81,13 +82,18 @@ enum qemuMigrationCookieFlags { VIR_ENUM_DECL(qemuMigrationCookieFlag); VIR_ENUM_IMPL(qemuMigrationCookieFlag, QEMU_MIGRATION_COOKIE_FLAG_LAST, - "graphics", "lockstate", "persistent", "network"); + "graphics", + "lockstate", + "persistent", + "network", + "nbd"); enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE), QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT), QEMU_MIGRATION_COOKIE_NETWORK = (1 << QEMU_MIGRATION_COOKIE_FLAG_NETWORK), + QEMU_MIGRATION_COOKIE_NBD = (1 << QEMU_MIGRATION_COOKIE_FLAG_NBD), }; typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -121,6 +127,12 @@ struct _qemuMigrationCookieNetwork { qemuMigrationCookieNetDataPtr net; }; +typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD; +typedef qemuMigrationCookieNBD *qemuMigrationCookieNBDPtr; +struct _qemuMigrationCookieNBD { + int port; /* on which port does NBD server listen for incoming data */ +}; + typedef struct _qemuMigrationCookie qemuMigrationCookie; typedef qemuMigrationCookie *qemuMigrationCookiePtr; struct _qemuMigrationCookie { @@ -149,6 +161,9 @@ struct _qemuMigrationCookie { /* If (flags & QEMU_MIGRATION_COOKIE_NETWORK) */ qemuMigrationCookieNetworkPtr network; + + /* If (flags & QEMU_MIGRATION_COOKIE_NBD) */ + qemuMigrationCookieNBDPtr nbd; }; static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) @@ -194,6 +209,7 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) VIR_FREE(mig->name); VIR_FREE(mig->lockState); VIR_FREE(mig->lockDriver); + VIR_FREE(mig->nbd); VIR_FREE(mig); } @@ -504,6 +520,27 @@ qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig, } +static int +qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, + virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + /* It is not a bug if there already is a NBD data */ + if (!mig->nbd && + VIR_ALLOC(mig->nbd) < 0) { + virReportOOMError(); + return -1; + } + + mig->nbd->port = priv->nbdPort; + mig->flags |= QEMU_MIGRATION_COOKIE_NBD; + + return 0; +} + + static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf, qemuMigrationCookieGraphicsPtr grap) { @@ -606,6 +643,13 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network) qemuMigrationCookieNetworkXMLFormat(buf, mig->network); + if ((mig->flags & QEMU_MIGRATION_COOKIE_NBD) && mig->nbd) { + virBufferAddLit(buf, " <nbd"); + if (mig->nbd->port) + virBufferAsprintf(buf, " port='%d'", mig->nbd->port); + virBufferAddLit(buf, "/>\n"); + } + virBufferAddLit(buf, "</qemu-migration>\n"); return 0; } @@ -889,6 +933,26 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt)))) goto error; + if (flags & QEMU_MIGRATION_COOKIE_NBD && + virXPathBoolean("boolean(./nbd)", ctxt)) { + char *port; + + if (VIR_ALLOC(mig->nbd) < 0) { + virReportOOMError(); + goto error; + } + + port = virXPathString("string(./nbd/@port)", ctxt); + if (port && virStrToLong_i(port, NULL, 10, &mig->nbd->port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed nbd port '%s'"), + port); + VIR_FREE(port); + goto error; + } + VIR_FREE(port); + } + virObjectUnref(caps); return 0; @@ -955,6 +1019,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, return -1; } + if ((flags & QEMU_MIGRATION_COOKIE_NBD) && + qemuMigrationCookieAddNBD(mig, driver, dom) < 0) + return -1; + if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) return -1; @@ -1439,6 +1507,7 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver, virDomainDefPtr def = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCapsPtr caps = NULL; + unsigned int cookieFlags = QEMU_MIGRATION_COOKIE_LOCKSTATE; VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s," " cookieout=%p, cookieoutlen=%p, flags=%lx", @@ -1461,12 +1530,23 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver, if (!(flags & VIR_MIGRATE_UNSAFE) && !qemuMigrationIsSafe(vm->def)) goto cleanup; + if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { + /* TODO support NBD for TUNNELLED migration */ + if (flags & VIR_MIGRATE_TUNNELLED) + VIR_DEBUG("NBD in tunnelled migration is currently not supported"); + else { + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + priv->nbdPort = 0; + } + } + if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) goto cleanup; if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0) + cookieFlags) < 0) goto cleanup; if (flags & VIR_MIGRATE_OFFLINE) { @@ -1666,7 +1746,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, origname = NULL; if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, - QEMU_MIGRATION_COOKIE_LOCKSTATE))) + QEMU_MIGRATION_COOKIE_LOCKSTATE | + QEMU_MIGRATION_COOKIE_NBD))) goto cleanup; if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) @@ -1726,8 +1807,20 @@ done: else cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS; - if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, - cookieFlags) < 0) { + if (mig->nbd && + flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { + /* TODO support NBD for TUNNELLED migration */ + if (flags & VIR_MIGRATE_TUNNELLED) + VIR_DEBUG("NBD in tunnelled migration is currently not supported"); + else { + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + priv->nbdPort = 0; + } + } + + if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, + cookieoutlen, cookieFlags) < 0) { /* We could tear down the whole guest here, but * cookie data is (so far) non-critical, so that * seems a little harsh. We'll just warn for now. @@ -2224,6 +2317,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, int fd = -1; unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; virErrorPtr orig_err = NULL; + unsigned int cookieFlags = 0; VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, resource=%lu, " @@ -2232,6 +2326,16 @@ qemuMigrationRun(virQEMUDriverPtr driver, cookieout, cookieoutlen, flags, resource, spec, spec->destType, spec->fwdType); + if (flags & VIR_MIGRATE_NON_SHARED_DISK) { + migrate_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + } + + if (flags & VIR_MIGRATE_NON_SHARED_INC) { + migrate_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC; + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + } + if (virLockManagerPluginUsesState(driver->lockManager) && !cookieout) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2241,8 +2345,9 @@ qemuMigrationRun(virQEMUDriverPtr driver, return -1; } - if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, - QEMU_MIGRATION_COOKIE_GRAPHICS))) + mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, + cookieFlags | QEMU_MIGRATION_COOKIE_GRAPHICS); + if (!mig) goto cleanup; if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig) < 0) @@ -2275,11 +2380,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, goto cleanup; } - if (flags & VIR_MIGRATE_NON_SHARED_DISK) - migrate_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; - - if (flags & VIR_MIGRATE_NON_SHARED_INC) - migrate_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC; /* connect to the destination qemu if needed */ if (spec->destType == MIGRATION_DEST_CONNECT_HOST && @@ -2387,10 +2487,11 @@ cleanup: VIR_FORCE_CLOSE(fd); } + cookieFlags |= (QEMU_MIGRATION_COOKIE_PERSISTENT | + QEMU_MIGRATION_COOKIE_NETWORK); if (ret == 0 && - qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_PERSISTENT | - QEMU_MIGRATION_COOKIE_NETWORK) < 0) { + qemuMigrationBakeCookie(mig, driver, vm, cookieout, + cookieoutlen, cookieFlags) < 0) { VIR_WARN("Unable to encode migration cookie"); } -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:19 +0100, Michal Privoznik wrote:
This migration cookie is meant for two purposes. The first is to be sent in begin phase from source to destination to let it know we support new implementation of VIR_MIGRATE_NON_SHARED_{DISK,INC} so destination can start NBD server. Then, the second purpose is, destination can let us know, on which port the NBD server is running. --- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_migration.c | 131 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 15 deletions(-)
ACK Jirka
This will be used with new migration scheme. This patch creates basically just monitor stub functions. Wiring them into something useful is done in later patches. --- src/qemu/qemu_monitor.c | 22 +++++++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 84 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7af571d..c7aa6d0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3383,3 +3383,25 @@ char *qemuMonitorGetTargetArch(qemuMonitorPtr mon) return qemuMonitorJSONGetTargetArch(mon); } + +int qemuMonitorNBDServerStart(qemuMonitorPtr mon, + const char *host, + unsigned int port) +{ + VIR_DEBUG("mon=%p host=%s port=%u", + mon, host, port); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONNBDServerStart(mon, host, port); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index ac77158..230f920 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -632,6 +632,9 @@ int qemuMonitorGetObjectProps(qemuMonitorPtr mon, char ***props); char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); +int qemuMonitorNBDServerStart(qemuMonitorPtr mon, + const char *host, + unsigned int port); /** * 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 a86d90c..e03688d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4359,3 +4359,59 @@ cleanup: virJSONValueFree(reply); return ret; } + +int +qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, + const char *host, + unsigned int port) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + virJSONValuePtr addr = NULL; + char *port_str = NULL; + + if (!(data = virJSONValueNewObject()) || + !(addr = virJSONValueNewObject()) || + (virAsprintf(&port_str, "%u", port) < 0)) { + virReportOOMError(); + goto cleanup; + } + + /* port is really expected as a string here by qemu */ + if (virJSONValueObjectAppendString(data, "host", host) < 0 || + virJSONValueObjectAppendString(data, "port", port_str) < 0 || + virJSONValueObjectAppendString(addr, "type", "inet") < 0 || + virJSONValueObjectAppend(addr, "data", data) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* From now on, @data is part of @addr */ + data = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-start", + "a:addr", addr, + NULL))) + goto cleanup; + + /* From now on, @addr is part of @cmd */ + addr = NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + +cleanup: + VIR_FREE(port_str); + virJSONValueFree(reply); + virJSONValueFree(cmd); + virJSONValueFree(addr); + virJSONValueFree(data); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 925d937..a6ab68b 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -327,4 +327,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); +int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, + const char *host, + unsigned int port); #endif /* QEMU_MONITOR_JSON_H */ -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:20 +0100, Michal Privoznik wrote:
This will be used with new migration scheme. This patch creates basically just monitor stub functions. Wiring them into something useful is done in later patches. --- src/qemu/qemu_monitor.c | 22 +++++++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 84 insertions(+)
ACK Jirka
This will be used with new migration scheme. This patch creates basically just monitor stub functions. Wiring them into something useful is done in later patches. --- src/qemu/qemu_monitor.c | 22 ++++++++++++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 53 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c7aa6d0..5abd9d0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3405,3 +3405,25 @@ int qemuMonitorNBDServerStart(qemuMonitorPtr mon, return qemuMonitorJSONNBDServerStart(mon, host, port); } + +int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID, + bool writable) +{ + VIR_DEBUG("mon=%p deviceID=%s", + mon, deviceID); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONNBDServerAdd(mon, deviceID, writable); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 230f920..d54d16d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -635,6 +635,9 @@ char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); int qemuMonitorNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port); +int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID, + bool writable); /** * 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 e03688d..f14b597 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4415,3 +4415,28 @@ cleanup: virJSONValueFree(data); return ret; } + +int +qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID, + bool writable) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-add", + "s:device", deviceID, + "b:writable", writable, + NULL))) + return ret; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index a6ab68b..c58aa7d 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -330,4 +330,7 @@ char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port); +int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID, + bool writable); #endif /* QEMU_MONITOR_JSON_H */ -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:21 +0100, Michal Privoznik wrote:
This will be used with new migration scheme. This patch creates basically just monitor stub functions. Wiring them into something useful is done in later patches. --- src/qemu/qemu_monitor.c | 22 ++++++++++++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 53 insertions(+)
ACK Jirka
This will be used after all migration work is done to stop NBD server running on destination. It doesn't take any arguments, just issues a command. --- src/qemu/qemu_monitor.c | 19 +++++++++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 21 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 42 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 5abd9d0..1166d8d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3427,3 +3427,22 @@ int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, return qemuMonitorJSONNBDServerAdd(mon, deviceID, writable); } + +int qemuMonitorNBDServerStop(qemuMonitorPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONNBDServerStop(mon); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d54d16d..10145e5 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -638,6 +638,7 @@ int qemuMonitorNBDServerStart(qemuMonitorPtr mon, int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); +int qemuMonitorNBDServerStop(qemuMonitorPtr); /** * 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 f14b597..856dd6a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4440,3 +4440,24 @@ qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + +int +qemuMonitorJSONNBDServerStop(qemuMonitorPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-stop", + NULL))) + return ret; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c58aa7d..fd04a92 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -333,4 +333,5 @@ int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); +int qemuMonitorJSONNBDServerStop(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:22 +0100, Michal Privoznik wrote:
This will be used after all migration work is done to stop NBD server running on destination. It doesn't take any arguments, just issues a command. --- src/qemu/qemu_monitor.c | 19 +++++++++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 21 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 42 insertions(+)
ACK Jirka
We need to start NBD server and feed it with all non-<shared/>, RW and source-full disks. Moreover, with new virPortAllocator we must ensure the borrowed port for NBD server will be returned if either migration completes or qemu process is teared down. --- src/qemu/qemu_migration.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_process.c | 5 ++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0acc1ac..f572034 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -35,6 +35,7 @@ #include "qemu_domain.h" #include "qemu_process.h" #include "qemu_capabilities.h" +#include "qemu_command.h" #include "qemu_cgroup.h" #include "domain_audit.h" @@ -1088,6 +1089,72 @@ error: return NULL; } +/** + * qemuMigrationStartNBDServer: + * @driver: qemu driver + * @vm: domain + * + * Starts NBD server. This is a newer method to copy + * storage during migration than using 'blk' and 'inc' + * arguments in 'migrate' monitor command. + * Error is reported here. + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuMigrationStartNBDServer(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + unsigned short port = 0; + const char *listenAddr = "0.0.0.0"; + char *diskAlias = NULL; + size_t i; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + /* skip shared, RO and source-less disks */ + if (disk->shared || disk->readonly || !disk->src) + continue; + + VIR_FREE(diskAlias); + if (virAsprintf(&diskAlias, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + goto cleanup; + + if (!port && + ((virPortAllocatorAcquire(driver->remotePorts, &port) < 0) || + (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) { + qemuDomainObjExitMonitor(driver, vm); + goto cleanup; + } + + if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0) { + qemuDomainObjExitMonitor(driver, vm); + goto cleanup; + } + qemuDomainObjExitMonitor(driver, vm); + } + + priv->nbdPort = port; + ret = 0; + +cleanup: + VIR_FREE(diskAlias); + if ((ret < 0) && port) + virPortAllocatorRelease(driver->remotePorts, port); + return ret; +} + + /* Validate whether the domain is safe to migrate. If vm is NULL, * then this is being run in the v2 Prepare stage on the destination * (where we only have the target xml); if vm is provided, then this @@ -1814,8 +1881,11 @@ done: if (flags & VIR_MIGRATE_TUNNELLED) VIR_DEBUG("NBD in tunnelled migration is currently not supported"); else { + if (qemuMigrationStartNBDServer(driver, vm) < 0) { + /* error already reported */ + goto endjob; + } cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; - priv->nbdPort = 0; } } @@ -1862,6 +1932,10 @@ cleanup: virObjectUnlock(vm); else qemuDomainRemoveInactive(driver, vm); + if (ret < 0 && priv->nbdPort) { + virPortAllocatorRelease(driver->remotePorts, priv->nbdPort); + priv->nbdPort = 0; + } } if (event) qemuDomainEventQueue(driver, event); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 476e3ed..030c143 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4197,6 +4197,11 @@ void qemuProcessStop(virQEMUDriverPtr driver, } } + if (priv->nbdPort) { + virPortAllocatorRelease(driver->remotePorts, priv->nbdPort); + priv->nbdPort = 0; + } + if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL; -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:23 +0100, Michal Privoznik wrote:
We need to start NBD server and feed it with all non-<shared/>, RW and source-full disks. Moreover, with new virPortAllocator we must ensure the borrowed port for NBD server will be returned if either migration completes or qemu process is teared down. --- src/qemu/qemu_migration.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_process.c | 5 ++++ 2 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0acc1ac..f572034 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -35,6 +35,7 @@ #include "qemu_domain.h" #include "qemu_process.h" #include "qemu_capabilities.h" +#include "qemu_command.h" #include "qemu_cgroup.h"
#include "domain_audit.h" @@ -1088,6 +1089,72 @@ error: return NULL; }
+/** + * qemuMigrationStartNBDServer: + * @driver: qemu driver + * @vm: domain + * + * Starts NBD server. This is a newer method to copy + * storage during migration than using 'blk' and 'inc' + * arguments in 'migrate' monitor command. + * Error is reported here. + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuMigrationStartNBDServer(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + unsigned short port = 0; + const char *listenAddr = "0.0.0.0";
This will need a follow-up patch to use whatever address (0.0.0.0 or ::) we use for qemu's -incoming. However, I think it's better to wait for the patch that implements IPv6 support for -incoming. Or the patch could solve both issues at the same time, what do you think Jan? ;-) ACK Jirka
On 02/21/2013 06:39 AM, Michal Privoznik wrote:
We need to start NBD server and feed it with all non-<shared/>, RW and source-full disks. Moreover, with new virPortAllocator we must ensure the borrowed port for NBD server will be returned if either migration completes or qemu process is teared down.
s/teared/torn/ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This function does the source part of NBD magic. It invokes drive-mirror on each non shared and RW disk with a source and wait till the mirroring process completes. When it does we can proceed with migration. Currently, an active waiting is done: every 500ms libvirt asks qemu if block-job is finished or not. However, once the job finishes, qemu doesn't report its progress so we can only assume if the job finished successfully or not. The better solution would be to listen to the event which is sent as soon as the job finishes. The event does contain the result of job. --- src/qemu/qemu_migration.c | 181 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 180 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f572034..081a72b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1154,6 +1154,179 @@ cleanup: return ret; } +/** + * qemuMigrationDriveMirror: + * @driver: qemu driver + * @vm: domain + * @mig: migration cookie + * @host: where are we migrating to + * @speed: how much should the copying be limited + * @migrate_flags: migrate monitor command flags + * + * Run drive-mirror to feed NBD server running on dst and wait + * till the process switches into another phase where writes go + * simultaneously to both source and destination. And this switch + * is what we are waiting for before proceeding with the next + * disk. On success, update @migrate_flags so we don't tell + * 'migrate' command to do the very same operation. + * + * Returns 0 on success (@migrate_flags updated), + * -1 otherwise. + */ +static int +qemuMigrationDriveMirror(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMigrationCookiePtr mig, + const char *host, + unsigned long speed, + unsigned int *migrate_flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + int mon_ret; + int port; + size_t i, lastGood = 0; + char *diskAlias = NULL; + char *nbd_dest = NULL; + unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; + virErrorPtr err = NULL; + + if (!(*migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | + QEMU_MONITOR_MIGRATE_NON_SHARED_INC))) + return 0; + + if (!mig->nbd) { + /* Destination doesn't support NBD server. + * Fall back to previous implementation. */ + VIR_DEBUG("Destination doesn't support NBD server " + "Falling back to previous implementation."); + return 0; + } + + /* steal NBD port and thus prevent its propagation back to destination */ + port = mig->nbd->port; + mig->nbd->port = 0; + + if (*migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC) + mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + virDomainBlockJobInfo info; + + /* skip shared, RO and source-less disks */ + if (disk->shared || disk->readonly || !disk->src) + continue; + + VIR_FREE(diskAlias); + VIR_FREE(nbd_dest); + if ((virAsprintf(&diskAlias, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) || + (virAsprintf(&nbd_dest, "nbd:%s:%u:exportname=%s", + host, port, diskAlias) < 0)) { + virReportOOMError(); + goto error; + } + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto error; + mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest, + NULL, speed, mirror_flags); + qemuDomainObjExitMonitor(driver, vm); + + if (mon_ret < 0) + goto error; + + lastGood = i; + + /* wait for completion */ + while (true) { + /* Poll every 500ms for progress & to allow cancellation */ + struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000 * 1000ull }; + + memset(&info, 0, sizeof(info)); + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto error; + if (priv->job.asyncAbort) { + /* explicitly do this *after* we entered the monitor, + * as this is a critical section so we are guaranteed + * priv->job.asyncAbort will not change */ + qemuDomainObjExitMonitor(driver, vm); + virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"), + qemuDomainAsyncJobTypeToString(priv->job.asyncJob), + _("canceled by client")); + goto error; + } + mon_ret = qemuMonitorBlockJob(priv->mon, diskAlias, NULL, 0, + &info, BLOCK_JOB_INFO, true); + qemuDomainObjExitMonitor(driver, vm); + + if (mon_ret < 0) + goto error; + + if (info.cur == info.end) { + VIR_DEBUG("Drive mirroring of '%s' completed", diskAlias); + break; + } + + /* XXX Frankly speaking, we should listen to the events, + * instead of doing this. But this works for now and we + * are doing something similar in migration itself anyway */ + + virObjectUnlock(vm); + + nanosleep(&ts, NULL); + + virObjectLock(vm); + } + } + + /* Okay, copied. Modify migrate_flags */ + *migrate_flags &= ~(QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | + QEMU_MONITOR_MIGRATE_NON_SHARED_INC); + ret = 0; + +cleanup: + VIR_FREE(diskAlias); + VIR_FREE(nbd_dest); + return ret; + +error: + /* don't overwrite any errors */ + err = virSaveLastError(); + /* cancel any outstanding jobs */ + while (lastGood) { + virDomainDiskDefPtr disk = vm->def->disks[--lastGood]; + + /* skip shared, RO disks */ + if (disk->shared || disk->readonly || !disk->src) + continue; + + VIR_FREE(diskAlias); + if (virAsprintf(&diskAlias, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) { + virReportOOMError(); + continue; + } + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) { + if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, 0, + NULL, BLOCK_JOB_ABORT, true) < 0) { + VIR_WARN("Unable to cancel block-job on '%s'", diskAlias); + } + qemuDomainObjExitMonitor(driver, vm); + } else { + VIR_WARN("Unable to enter monitor. No block job cancelled"); + } + } + if (err) + virSetError(err); + virFreeError(err); + goto cleanup; +} /* Validate whether the domain is safe to migrate. If vm is NULL, * then this is being run in the v2 Prepare stage on the destination @@ -2427,6 +2600,13 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig) < 0) VIR_WARN("unable to provide data for graphics client relocation"); + /* this will update migrate_flags on success */ + if (qemuMigrationDriveMirror(driver, vm, mig, spec->dest.host.name, + migrate_speed, &migrate_flags) < 0) { + /* error reported by helper func */ + goto cleanup; + } + /* Before EnterMonitor, since qemuMigrationSetOffline already does that */ if (!(flags & VIR_MIGRATE_LIVE) && virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -2454,7 +2634,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, goto cleanup; } - /* connect to the destination qemu if needed */ if (spec->destType == MIGRATION_DEST_CONNECT_HOST && qemuMigrationConnect(driver, vm, spec) < 0) { -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:24 +0100, Michal Privoznik wrote:
This function does the source part of NBD magic. It invokes drive-mirror on each non shared and RW disk with a source and wait till the mirroring process completes. When it does we can proceed with migration.
Currently, an active waiting is done: every 500ms libvirt asks qemu if block-job is finished or not. However, once the job finishes, qemu doesn't report its progress so we can only assume if the job finished successfully or not. The better solution would be to listen to the event which is sent as soon as the job finishes. The event does contain the result of job. ... +static int +qemuMigrationDriveMirror(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMigrationCookiePtr mig, + const char *host, + unsigned long speed, + unsigned int *migrate_flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + int mon_ret; + int port; + size_t i, lastGood = 0; + char *diskAlias = NULL; + char *nbd_dest = NULL; + unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; + virErrorPtr err = NULL; + + if (!(*migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | + QEMU_MONITOR_MIGRATE_NON_SHARED_INC))) + return 0; + + if (!mig->nbd) { + /* Destination doesn't support NBD server. + * Fall back to previous implementation. */ + VIR_DEBUG("Destination doesn't support NBD server " + "Falling back to previous implementation."); + return 0; + } + + /* steal NBD port and thus prevent its propagation back to destination */ + port = mig->nbd->port; + mig->nbd->port = 0; + + if (*migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC) + mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + virDomainBlockJobInfo info; + + /* skip shared, RO and source-less disks */ + if (disk->shared || disk->readonly || !disk->src) + continue; + + VIR_FREE(diskAlias); + VIR_FREE(nbd_dest); + if ((virAsprintf(&diskAlias, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) || + (virAsprintf(&nbd_dest, "nbd:%s:%u:exportname=%s", + host, port, diskAlias) < 0)) {
port is signed => s/%u/%d/ ACK Jirka
At the end of migration, it is important to stop NBD server and thus release all allocated resources. --- src/qemu/qemu_migration.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 081a72b..8350f5d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1328,6 +1328,30 @@ error: goto cleanup; } + +static void +qemuMigrationStopNBDServer(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMigrationCookiePtr mig) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!mig->nbd) + return; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + return; + + if (qemuMonitorNBDServerStop(priv->mon) < 0) + VIR_WARN("Unable to stop NBD server"); + + qemuDomainObjExitMonitor(driver, vm); + + virPortAllocatorRelease(driver->remotePorts, priv->nbdPort); + priv->nbdPort = 0; +} + /* Validate whether the domain is safe to migrate. If vm is NULL, * then this is being run in the v2 Prepare stage on the destination * (where we only have the target xml); if vm is provided, then this @@ -3743,6 +3767,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver, VIR_WARN("unable to provide network data for relocation"); } + qemuMigrationStopNBDServer(driver, vm, mig); + if (flags & VIR_MIGRATE_PERSIST_DEST) { virDomainDefPtr vmdef; if (vm->persistent) -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:25 +0100, Michal Privoznik wrote:
At the end of migration, it is important to stop NBD server and thus release all allocated resources. --- src/qemu/qemu_migration.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
ACK Jirka
If a migration fails, we need to stop all block jobs running so qemu doesn't try to send data to destination over and over again. --- src/qemu/qemu_migration.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8350f5d..f48c59f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1352,6 +1352,46 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver, priv->nbdPort = 0; } +static void +qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, + virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + char *diskAlias = NULL; + + VIR_DEBUG("mig=%p nbdPort=%d", mig->nbd, priv->nbdPort); + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + /* skip shared, RO and source-less disks */ + if (disk->shared || disk->readonly || !disk->src) + continue; + + VIR_FREE(diskAlias); + if (virAsprintf(&diskAlias, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto cleanup; + + if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, 0, + NULL, BLOCK_JOB_ABORT, true) < 0) + VIR_WARN("Unable to stop block job on %s", diskAlias); + qemuDomainObjExitMonitor(driver, vm); + } + +cleanup: + VIR_FREE(diskAlias); + return; +} + /* Validate whether the domain is safe to migrate. If vm is NULL, * then this is being run in the v2 Prepare stage on the destination * (where we only have the target xml); if vm is provided, then this @@ -2758,6 +2798,9 @@ cleanup: if (ret < 0 && !orig_err) orig_err = virSaveLastError(); + /* cancel any outstanding NBD jobs */ + qemuMigrationCancelDriveMirror(mig, driver, vm); + if (spec->fwdType != MIGRATION_FWD_DIRECT) { if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0) ret = -1; @@ -3965,6 +4008,9 @@ int qemuMigrationConfirm(virQEMUDriverPtr driver, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); } else { + /* cancel any outstanding NBD jobs */ + qemuMigrationCancelDriveMirror(mig, driver, vm); + /* run 'cont' on the destination, which allows migration on qemu * >= 0.10.6 to work properly. This isn't strictly necessary on * older qemu's, but it also doesn't hurt anything there -- 1.8.1.2
On Thu, Feb 21, 2013 at 14:39:26 +0100, Michal Privoznik wrote:
If a migration fails, we need to stop all block jobs running so qemu doesn't try to send data to destination over and over again. --- src/qemu/qemu_migration.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
This will need a follow-up patch to cancel drive mirror jobs in qemuProcessRecoverMigration when libvirtd is restarted during an outgoing migration. ACK Jirka
On 22.02.2013 23:23, Jiri Denemark wrote:
On Thu, Feb 21, 2013 at 14:39:26 +0100, Michal Privoznik wrote:
If a migration fails, we need to stop all block jobs running so qemu doesn't try to send data to destination over and over again. --- src/qemu/qemu_migration.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
This will need a follow-up patch to cancel drive mirror jobs in qemuProcessRecoverMigration when libvirtd is restarted during an outgoing migration.
ACK
Jirka
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
So I've pushed the patches. Will send follow-up patches soon. Thanks. Michal
On Thu, Feb 21, 2013 at 14:39:17 +0100, Michal Privoznik wrote:
Since there are many hidden problems with auto-creating storage for the domain in here, as previous rounds of reviewing the series has shown, I've decided to not do anything about it for now and the auto allocation is dropped completely. So we are back to the assumption we already have - users need to make sure files already exists and have the right size. Hawk.
OK, I think that's wise considering how things are getting complicated on the storage pre-creation side. I guess you'll be working on implementing that in a follow-up series.
diff to v4: -drop storage auto-allocation -include John's and Jirka's reviews
Note, that most of the patches has been ACKed already.
Could you consider putting the changelog into individual patches next time? Especially in case the previous version of a particular patch was acked and no changes were made in it since then. You can't expect reviewers will remember what patches they already acked :-) Jirka
participants (3)
-
Eric Blake -
Jiri Denemark -
Michal Privoznik