This just keeps track whether qemu knows nbd-server-* commands so we can use it during migration or not. --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6ce2638..e331de9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -193,6 +193,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "drive-mirror", /* 115 */ "usb-redir.bootindex", "usb-host.bootindex", + "nbd-server-start", ); struct _qemuCaps { @@ -1909,6 +1910,8 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps, qemuCapsSet(caps, QEMU_CAPS_VNC); else if (STREQ(name, "drive-mirror")) qemuCapsSet(caps, QEMU_CAPS_DRIVE_MIRROR); + else if (STREQ(name, "nbd-server-start")) + qemuCapsSet(caps, 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 751b3ec..962dfdf 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -155,6 +155,7 @@ enum qemuCapsFlags { QEMU_CAPS_DRIVE_MIRROR = 115, /* drive-mirror monitor command */ QEMU_CAPS_USB_REDIR_BOOTINDEX = 116, /* usb-redir.bootindex */ QEMU_CAPS_USB_HOST_BOOTINDEX = 117, /* usb-host.bootindex */ + QEMU_CAPS_NBD_SERVER = 118, /* nbd-server-start QMP command */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.7.8.6

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 is NBD server running. --- src/qemu/qemu_migration.c | 108 ++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 97 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d52ec59..cd59eda 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -72,6 +72,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 }; @@ -79,13 +80,14 @@ 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; @@ -119,6 +121,17 @@ 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. + Zero value has special meaning - it is there just to let + destination know we (the source) do support NBD. + Negative one is meant to be sent when translating from + perform to finish phase to let destination know it's + safe to stop NBD server.*/ +}; + typedef struct _qemuMigrationCookie qemuMigrationCookie; typedef qemuMigrationCookie *qemuMigrationCookiePtr; struct _qemuMigrationCookie { @@ -147,6 +160,9 @@ struct _qemuMigrationCookie { /* If (flags & QEMU_MIGRATION_COOKIE_NETWORK) */ qemuMigrationCookieNetworkPtr network; + + /* If (flags & QEMU_MIGRATION_COOKIE_NBD) */ + qemuMigrationCookieNBDPtr nbd; }; static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) @@ -192,6 +208,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); } @@ -492,6 +509,24 @@ qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig, } +static int +qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, + int nbdPort) +{ + /* 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 = nbdPort; + mig->flags |= QEMU_MIGRATION_COOKIE_NBD; + + return 0; +} + + static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf, qemuMigrationCookieGraphicsPtr grap) { @@ -594,6 +629,13 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *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; } @@ -821,6 +863,12 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, goto error; } + /* nbd is optional */ + if (val == QEMU_MIGRATION_COOKIE_FLAG_NBD) { + VIR_FREE(str); + continue; + } + if ((flags & (1 << val)) == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unsupported migration cookie feature %s"), @@ -873,6 +921,25 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt)))) goto error; + if (flags & QEMU_MIGRATION_COOKIE_NBD && + virXPathBoolean("count(./nbd) > 0", 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); + goto error; + } + } + return 0; error: @@ -911,6 +978,7 @@ static int qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, struct qemud_driver *driver, virDomainObjPtr dom, + int nbdPort, char **cookieout, int *cookieoutlen, unsigned int flags) @@ -937,6 +1005,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, return -1; } + if (flags & QEMU_MIGRATION_COOKIE_NBD && + qemuMigrationCookieAddNBD(mig, nbdPort) < 0) + return -1; + if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) return -1; @@ -1415,6 +1487,7 @@ char *qemuMigrationBegin(struct qemud_driver *driver, qemuMigrationCookiePtr mig = NULL; virDomainDefPtr def = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + unsigned int cookie_flags = QEMU_MIGRATION_COOKIE_LOCKSTATE; VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s," " cookieout=%p, cookieoutlen=%p, flags=%lx", @@ -1434,12 +1507,15 @@ char *qemuMigrationBegin(struct qemud_driver *driver, if (!(flags & VIR_MIGRATE_UNSAFE) && !qemuMigrationIsSafe(vm->def)) goto cleanup; + if (qemuCapsGet(priv->caps, QEMU_CAPS_NBD_SERVER)) + cookie_flags |= QEMU_MIGRATION_COOKIE_NBD; + if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) goto cleanup; - if (qemuMigrationBakeCookie(mig, driver, vm, + if (qemuMigrationBakeCookie(mig, driver, vm, 0, cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0) + cookie_flags) < 0) goto cleanup; if (xmlin) { @@ -1512,6 +1588,8 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, bool tunnel = !!st; char *origname = NULL; char *xmlout = NULL; + int nbdPort = 0; + unsigned int cookie_flags = QEMU_MIGRATION_COOKIE_GRAPHICS; if (virTimeMillisNow(&now) < 0) return -1; @@ -1589,7 +1667,8 @@ qemuMigrationPrepareAny(struct qemud_driver *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) @@ -1640,8 +1719,12 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, VIR_DEBUG("Received no lockstate"); } - if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_GRAPHICS) < 0) { + /* dummy place holder for real work */ + nbdPort = 0; + cookie_flags |= QEMU_MIGRATION_COOKIE_NBD; + + if (qemuMigrationBakeCookie(mig, driver, vm, nbdPort, + cookieout, cookieoutlen, cookie_flags) < 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. @@ -2150,7 +2233,8 @@ qemuMigrationRun(struct qemud_driver *driver, } if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, - QEMU_MIGRATION_COOKIE_GRAPHICS))) + QEMU_MIGRATION_COOKIE_GRAPHICS | + QEMU_MIGRATION_COOKIE_NBD))) goto cleanup; if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig) < 0) @@ -2296,9 +2380,10 @@ cleanup: } if (ret == 0 && - qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, + qemuMigrationBakeCookie(mig, driver, vm, -1, cookieout, cookieoutlen, QEMU_MIGRATION_COOKIE_PERSISTENT | - QEMU_MIGRATION_COOKIE_NETWORK) < 0) { + QEMU_MIGRATION_COOKIE_NETWORK | + QEMU_MIGRATION_COOKIE_NBD) < 0) { VIR_WARN("Unable to encode migration cookie"); } @@ -3233,7 +3318,7 @@ qemuMigrationFinish(struct qemud_driver *driver, qemuDomainCleanupRemove(vm, qemuMigrationPrepareCleanup); - cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK; + cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK | QEMU_MIGRATION_COOKIE_NBD; if (flags & VIR_MIGRATE_PERSIST_DEST) cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT; @@ -3377,7 +3462,8 @@ qemuMigrationFinish(struct qemud_driver *driver, VIR_DOMAIN_EVENT_STOPPED_FAILED); } - if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0) + if (qemuMigrationBakeCookie(mig, driver, vm, 0, + cookieout, cookieoutlen, 0) < 0) VIR_WARN("Unable to encode migration cookie"); endjob: -- 1.7.8.6

On 27.11.2012 23:18, Eric Blake wrote: The destination would not parse that part (without any error of course) and doesn't reply to the destination neither. So in Perform phase, when the source gets a migration cookie from dst without any NBD data it is obvious dst is running an older libvirt without NBD support and hence source will fall back to previous implementation. However, we can't switch to VIR_DRV_FEATURE approach as you suggest, because usage of new implementation is not just matter of qemu driver itself, but depends on current qemu instance that is to be migrated (it's pointless to ask cached capabilities as running qemu can has a different ones). It would be nice if we could do qemuSupportsFeature() but I am afraid we can't. Okay, I'll add it into comment somewhere in code. Not really. It will overwrite info about the port NBD server is listening to. However, this will in place when we need it, the proper value is passed. Otherwise, in all other places 0 is being passed - which is what we want, since in that phases the port info is not relevant at all. So I think it is okay as is. It does play nice. I've tested it. Michal

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 | 21 +++++++++++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 23 +++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 48 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 228a993..b84b961 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3344,3 +3344,24 @@ int qemuMonitorNBDServerStart(qemuMonitorPtr mon, return qemuMonitorJSONNBDServerStart(mon, host, port); } + +int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID) +{ + 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_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONNBDServerAdd(mon, deviceID); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index eb069ed..09b7866 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -634,6 +634,8 @@ char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); int qemuMonitorNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port); +int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID); /** * 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 2bf03e8..c740837 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4589,3 +4589,26 @@ cleanup: virJSONValueFree(data); return ret; } + +int +qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-add", + "s:device", deviceID, + 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 506e6f7..1dc2fbe 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -339,4 +339,6 @@ char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port); +int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, + const char *deviceID); #endif /* QEMU_MONITOR_JSON_H */ -- 1.7.8.6

No change to this patch, but I'm getting rather tired of this copy and paste pattern - every new monitor command has to touch two .h files. I wonder if we should instead switch to more of a callback pattern, where we only have to touch one .h file (the public entry point, and the signature of a callback function), then have qemu_monitor.c do: if (mon->driver.func) mon->driver.func(args) else report error about unsupported then qemu_monitor_json.c would register a table of static callback functions, rather than having to also declare every single function almost verbatim like qemu_monitor.h. But enough of that side-track thought about a potential future reorganization of the code. ACK to this patch; fine as-is.

This is a stub internal API just for now. Its purpose in life is to start NBD server and feed it with all domain disks. When adding a disk to NBD server, it is addressed via its alias (id= param on qemu command line). --- src/qemu/qemu_driver.c | 8 +++--- src/qemu/qemu_migration.c | 59 +++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 6 +++- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d4cafcc..493fbb9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9625,7 +9625,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, ret = qemuMigrationPrepareTunnel(driver, dconn, NULL, 0, NULL, NULL, /* No cookies in v2 */ - st, dname, dom_xml); + st, dname, dom_xml, flags); cleanup: qemuDriverUnlock(driver); @@ -9685,7 +9685,7 @@ qemudDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - dname, dom_xml); + dname, dom_xml, flags); cleanup: qemuDriverUnlock(driver); @@ -9922,7 +9922,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - dname, dom_xml); + dname, dom_xml, flags); cleanup: qemuDriverUnlock(driver); @@ -9967,7 +9967,7 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, ret = qemuMigrationPrepareTunnel(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, - st, dname, dom_xml); + st, dname, dom_xml, flags); qemuDriverUnlock(driver); cleanup: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index cd59eda..7e86c33 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1074,6 +1074,29 @@ error: return NULL; } +/** + * qemuMigrationStartNBDServer: + * @driver: qemu driver + * @vm: domain + * @nbdPort: which port is NBD server listening to + * + * 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(struct qemud_driver *driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + int *nbdPort ATTRIBUTE_UNUSED) +{ + /* do nothing for now */ + return 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 @@ -1575,7 +1598,8 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, const char *dname, const char *dom_xml, const char *migrateFrom, - virStreamPtr st) + virStreamPtr st, + unsigned long flags) { virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; @@ -1719,9 +1743,17 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, VIR_DEBUG("Received no lockstate"); } - /* dummy place holder for real work */ - nbdPort = 0; - cookie_flags |= QEMU_MIGRATION_COOKIE_NBD; + if ((flags & VIR_MIGRATE_NON_SHARED_INC || + flags & VIR_MIGRATE_NON_SHARED_DISK) && + mig->nbd && qemuCapsGet(priv->caps, QEMU_CAPS_NBD_SERVER)) { + /* both source and destination qemus support nbd-server-* + * commands and user requested disk copy. Use the new ones */ + if (qemuMigrationStartNBDServer(driver, vm, &nbdPort) < 0) { + /* error already reported */ + goto endjob; + } + cookie_flags |= QEMU_MIGRATION_COOKIE_NBD; + } if (qemuMigrationBakeCookie(mig, driver, vm, nbdPort, cookieout, cookieoutlen, cookie_flags) < 0) { @@ -1791,21 +1823,23 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, int *cookieoutlen, virStreamPtr st, const char *dname, - const char *dom_xml) + const char *dom_xml, + unsigned long flags) { int ret; VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " - "cookieout=%p, cookieoutlen=%p, st=%p, dname=%s, dom_xml=%s", + "cookieout=%p, cookieoutlen=%p, st=%p, dname=%s, dom_xml=%s " + "flags=%lx", driver, dconn, NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, st, NULLSTR(dname), dom_xml); + cookieout, cookieoutlen, st, NULLSTR(dname), dom_xml, flags); /* QEMU will be started with -incoming stdio (which qemu_command might * convert to exec:cat or fd:n) */ ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, dname, dom_xml, - "stdio", st); + "stdio", st, flags); return ret; } @@ -1820,7 +1854,8 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, const char *uri_in, char **uri_out, const char *dname, - const char *dom_xml) + const char *dom_xml, + unsigned long flags) { static int port = 0; int this_port; @@ -1831,10 +1866,10 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " - "dname=%s, dom_xml=%s", + "dname=%s, dom_xml=%s flags=%lx", driver, dconn, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, NULLSTR(uri_in), uri_out, - NULLSTR(dname), dom_xml); + NULLSTR(dname), dom_xml, flags); /* The URI passed in may be NULL or a string "tcp://somehostname:port". * @@ -1916,7 +1951,7 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, dname, dom_xml, - migrateFrom, NULL); + migrateFrom, NULL, flags); cleanup: VIR_FREE(hostname); if (ret != 0) diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 7a2269a..8e411ef 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -97,7 +97,8 @@ int qemuMigrationPrepareTunnel(struct qemud_driver *driver, int *cookieoutlen, virStreamPtr st, const char *dname, - const char *dom_xml); + const char *dom_xml, + unsigned long flags); int qemuMigrationPrepareDirect(struct qemud_driver *driver, virConnectPtr dconn, @@ -108,7 +109,8 @@ int qemuMigrationPrepareDirect(struct qemud_driver *driver, const char *uri_in, char **uri_out, const char *dname, - const char *dom_xml); + const char *dom_xml, + unsigned long flags); int qemuMigrationPerform(struct qemud_driver *driver, virConnectPtr conn, -- 1.7.8.6

On 28.11.2012 04:03, li guang wrote: Yes, that's correct. nbdPort is generated by that function. However, in this patch, I wanted to point out it qemuMigrationStartNBDServer which decides which port the NBD server will listen to. The flow is exactly the opposite - qemuMigrationStartNBDServer starts the server on a port it decides. The port is returned to callee which will store it into the cookie and send back to the source. Source needs to know which port the data should be sent to but it has no info which ports are free on dst. Long story short, This patch is mainly here to show this flow: A NBDServerStart function is called (under certain circumstances). This function allocates a port which is then injected into migration cookie. The cookie is then sent from dst to src. How the function work inside is not important for this patch. Michal

We need to start NBD server and feed it with all non-<shared/> disks. However, after qemuDomainObjEnterMonitorAsync the domain object is unlocked so we cannot touch its disk definitions. Therefore, we must prepare the list of disk IDs prior entering monitor. --- src/qemu/qemu_migration.c | 65 +++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7a2d0a2..3f35a8c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -33,6 +33,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" @@ -1099,12 +1100,66 @@ qemuMigrationNextPort(void) { * Returns 0 on success, -1 otherwise. */ static int -qemuMigrationStartNBDServer(struct qemud_driver *driver ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - int *nbdPort ATTRIBUTE_UNUSED) +qemuMigrationStartNBDServer(struct qemud_driver *driver, + virDomainObjPtr vm, + int *nbdPort) { - /* do nothing for now */ - return 0; + int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + int port = qemuMigrationNextPort(); + const char *listen = "0.0.0.0"; + char **disks = NULL; + size_t i, ndisks = 0; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + /* skip shared disks */ + if (disk->shared) + continue; + + if (VIR_REALLOC_N(disks, ndisks + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&disks[ndisks++], "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) { + virReportOOMError(); + goto cleanup; + } + } + + if (!ndisks) { + /* Hooray! Nothing to care about */ + ret = 0; + goto cleanup; + } + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + goto cleanup; + + if (qemuMonitorNBDServerStart(priv->mon, listen, port) < 0) + goto endjob; + + for (i = 0; i < ndisks; i++) { + if (qemuMonitorNBDServerAdd(priv->mon, disks[i]) < 0) { + VIR_WARN("Unable to add '%s' to NDB server", disks[i]); + goto endjob; + } + } + + *nbdPort = port; + ret = 0; + +endjob: + qemuDomainObjExitMonitorWithDriver(driver, vm); +cleanup: + for (i = 0; i < ndisks; i++) + VIR_FREE(disks[i]); + VIR_FREE(disks); + return ret; } -- 1.7.8.6

With new NBD storage migration approach there are several requirements that need to be meet for successful use of the feature. One of them is - the file representing a disk, needs to have at least same size as on the source. Hence, we must transfer a list of pairs [disk source, size] and check on destination that this requirement is met and/or take actions to meet it. --- src/qemu/qemu_migration.c | 162 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 159 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2c66d8c..f8e52fb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -27,6 +27,9 @@ #include <gnutls/x509.h> #include <fcntl.h> #include <poll.h> +#include <unistd.h> +#include <sys/types.h> +#include <sys/stat.h> #include "qemu_migration.h" #include "qemu_monitor.h" @@ -131,6 +134,15 @@ struct _qemuMigrationCookieNBD { Negative one is meant to be sent when translating from perform to finish phase to let destination know it's safe to stop NBD server.*/ + + /* The list of pairs [disk-alias,disk-size] (in Bytes). + * This is needed because the same disk size is one of + * prerequisites for NBD storage migration. */ + size_t ndisks; + struct { + char *src; /* disk alias */ + size_t bytes; + } *disk; }; typedef struct _qemuMigrationCookie qemuMigrationCookie; @@ -193,6 +205,21 @@ qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network) } +static void +qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd) +{ + size_t i; + + if (!nbd) + return; + + for (i = 0; i < nbd->ndisks; i++) + VIR_FREE(nbd->disk[i].src); + VIR_FREE(nbd->disk); + VIR_FREE(nbd); +} + + static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) { if (!mig) @@ -204,12 +231,13 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) if (mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) qemuMigrationCookieNetworkFree(mig->network); + qemuMigrationCookieNBDFree(mig->nbd); + VIR_FREE(mig->localHostname); VIR_FREE(mig->remoteHostname); VIR_FREE(mig->name); VIR_FREE(mig->lockState); VIR_FREE(mig->lockDriver); - VIR_FREE(mig->nbd); VIR_FREE(mig); } @@ -512,8 +540,12 @@ qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig, static int qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, + virDomainObjPtr vm, int nbdPort) { + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + /* It is not a bug if there already is a NBD data */ if (!mig->nbd && VIR_ALLOC(mig->nbd) < 0) { @@ -521,6 +553,38 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, return -1; } + /* in Begin phase add info about disks */ + if (priv->job.phase == QEMU_MIGRATION_PHASE_BEGIN3 && + vm->def->ndisks) { + if (VIR_ALLOC_N(mig->nbd->disk, vm->def->ndisks) < 0) { + virReportOOMError(); + return -1; + } + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + struct stat sb; + + if (!disk->src) + continue; + + if (virAsprintf(&mig->nbd->disk[mig->nbd->ndisks].src, + "%s", disk->src) < 0) { + virReportOOMError(); + return -1; + } + + if (stat(disk->src, &sb) < 0) { + virReportSystemError(errno, + _("Unable to stat '%s'"), + disk->src); + return -1; + } + + mig->nbd->disk[mig->nbd->ndisks++].bytes = sb.st_size; + } + } + mig->nbd->port = nbdPort; mig->flags |= QEMU_MIGRATION_COOKIE_NBD; @@ -634,7 +698,16 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver, virBufferAddLit(buf, " <nbd"); if (mig->nbd->port) virBufferAsprintf(buf, " port='%d'", mig->nbd->port); - virBufferAddLit(buf, "/>\n"); + if (mig->nbd->ndisks) { + virBufferAddLit(buf, ">\n"); + for (i = 0; i < mig->nbd->ndisks; i++) + virBufferAsprintf(buf, " <disk src='%s' size='%zu'/>\n", + mig->nbd->disk[i].src, + mig->nbd->disk[i].bytes); + virBufferAddLit(buf, " </nbd>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } } virBufferAddLit(buf, "</qemu-migration>\n"); @@ -939,6 +1012,34 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, port); goto error; } + + if ((n = virXPathNodeSet("./nbd/disk", ctxt, &nodes)) > 0) { + xmlNodePtr oldNode = ctxt->node; + if (VIR_ALLOC_N(mig->nbd->disk, n) < 0) { + virReportOOMError(); + goto error; + } + mig->nbd->ndisks = n; + + for (i = 0; i < n; i++) { + ctxt->node = nodes[i]; + + mig->nbd->disk[i].src = virXPathString("string(./@src)", ctxt); + + tmp = virXPathString("string(./@size)", ctxt); + if (virStrToLong_ull(tmp, NULL, 10, (unsigned long long *) + &mig->nbd->disk[i].bytes) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed size attribute '%s'"), + tmp); + VIR_FREE(tmp); + goto error; + } + VIR_FREE(tmp); + } + VIR_FREE(nodes); + ctxt->node = oldNode; + } } return 0; @@ -1007,7 +1108,7 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, } if (flags & QEMU_MIGRATION_COOKIE_NBD && - qemuMigrationCookieAddNBD(mig, nbdPort) < 0) + qemuMigrationCookieAddNBD(mig, dom, nbdPort) < 0) return -1; if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) @@ -1334,6 +1435,57 @@ error: goto cleanup; } +static int +qemuMigrationPreCreateStorage(qemuMigrationCookiePtr mig) +{ + int ret = -1; + size_t i; + struct stat sb; + int fd = -1; + + if (!mig->nbd || !mig->nbd->ndisks) { + /* nothing to do here */ + return 0; + } + + for (i = 0; i < mig->nbd->ndisks; i++) { + const char *src = mig->nbd->disk[i].src; + size_t bytes = mig->nbd->disk[i].bytes; + VIR_DEBUG("Checking '%s' for its size (requested %zuB)", src, bytes); + + if ((fd = virFileOpenAs(src, O_RDWR | O_CREAT, 0660, + -1, -1, VIR_FILE_OPEN_NOFORK)) < 0) { + virReportSystemError(errno, _("Unable to create '%s'"), src); + goto cleanup; + } + + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, _("Unable to stat '%s'"), src); + goto cleanup; + } + + VIR_DEBUG("File '%s' is %zuB big", src, sb.st_size); + if (sb.st_size < bytes && + ftruncate(fd, bytes) < 0) { + virReportSystemError(errno, _("Unable to ftruncate '%s'"), src); + goto cleanup; + } + + VIR_FORCE_CLOSE(fd); + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(fd); + /* free from migration data to prevent + * infinite sending from src to dst and back */ + for (i = 0; i < mig->nbd->ndisks; i++) + VIR_FREE(mig->nbd->disk[i].src); + VIR_FREE(mig->nbd->disk); + mig->nbd->ndisks = 0; + 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 @@ -1932,6 +2084,10 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, QEMU_MIGRATION_COOKIE_NBD))) goto cleanup; + /* pre-create all storage */ + if (qemuMigrationPreCreateStorage(mig) < 0) + goto cleanup; + if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto cleanup; qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PREPARE); -- 1.7.8.6