On 11/27/14 14:55, Michal Privoznik wrote:
Up 'til now, users need to precreate non-shared storage on migration themselves. This is not very friendly requirement and we should do something about it. In this patch, the migration cookie is extended, so that <nbd/> section does not only contain NBD port, but info on disks being migrated. This patch sends a list of pairs of:
<disk target; disk size>
to the destination. The actual storage allocation is left for next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_migration.c | 180 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 161 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a1b1458..d4b3acf 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -141,6 +141,10 @@ typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD; typedef qemuMigrationCookieNBD *qemuMigrationCookieNBDPtr; struct _qemuMigrationCookieNBD { int port; /* on which port does NBD server listen for incoming data */ + + size_t ndisks; /* Number of items in @target and @alloc arrays */ + char **target; /* Disk target */ + size_t *alloc; /* And its allocation */
I think this already warrants usage of a struct to group them together.
};
typedef struct _qemuMigrationCookie qemuMigrationCookie;
@@ -523,18 +540,86 @@ qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig, }
+/** + * qemuMigrationGetDiskSize: + * @disk: disk to query + * @alloc: disk size in bytes
Allocation and capacity are two different stats of a disk volume. You should stick with capacity here to avoid confusion
+ * + * For given disk get its image size. + * + * Returns: 0 on success, + * -1 on failure, + * 1 if disk size doesn't matter + */ +static int +qemuMigrationGetDiskSize(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + size_t *alloc) +{ + int ret = -1; + virDomainBlockInfo info; + + if (!virStorageSourceIsLocalStorage(disk->src) || + !disk->src->path) + return 1; + + if (qemuDomainGetBlockInfoImpl(driver, vm, disk, + &info, disk->src->path) < 0) + goto cleanup;
Since you are requesting this only for a live domain and the only stat you care about is capacity of the guest visible portion of the disk it would be better to use a monitor call to determine the data from the disk rather than use the weird function that parses the information from the file on the disk.
+ + *alloc = info.capacity; + ret = 0; + cleanup: + return ret; +} + + static int qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, - virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virQEMUDriverPtr driver, virDomainObjPtr vm) { 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) return -1;
+ if (vm->def->ndisks && + (VIR_ALLOC_N(mig->nbd->target, vm->def->ndisks) < 0 || + VIR_ALLOC_N(mig->nbd->alloc, vm->def->ndisks) < 0))
Allocing an array of strutcts would look nicer.
+ return -1; + mig->nbd->ndisks = 0; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + int rc; + size_t alloc; + + /* skip shared, RO and source-less disks */ + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk))
I think also networked disks don't make sense here. We also have a function (virStorageSourceIsEmpty or similar) to check whether the source doesn't denote an empty drive. One additional thing that might break is if some of the storage is on a shared storage system, while other is not.
+ continue; + + if ((rc = qemuMigrationGetDiskSize(driver, vm, disk, &alloc)) < 0) { + /* error reported */
We report errors by default. No need to note it explicitly.
+ return -1; + } else if (rc > 0) { + /* Don't add this disk */ + continue; + } + + if (VIR_STRDUP(mig->nbd->target[mig->nbd->ndisks], + disk->dst) < 0) + return -1; + + mig->nbd->alloc[mig->nbd->ndisks] = alloc; + mig->nbd->ndisks++; + } + mig->nbd->port = priv->nbdPort; mig->flags |= QEMU_MIGRATION_COOKIE_NBD;
@@ -763,7 +848,18 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr 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"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < mig->nbd->ndisks; i++) + virBufferAsprintf(buf, "<disk target='%s' alloc='%zu'/>\n",
Again ... capacity is a better word for this IMO.
+ mig->nbd->target[i], + mig->nbd->alloc[i]); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</nbd>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } }
if (mig->flags & QEMU_MIGRATION_COOKIE_STATS && mig->jobInfo) @@ -891,6 +987,65 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) }
+static qemuMigrationCookieNBDPtr +qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) +{ + qemuMigrationCookieNBDPtr ret = NULL; + char *port = NULL, *alloc = NULL; + size_t i; + int n; + xmlNodePtr *disks = NULL; + xmlNodePtr save_ctxt = ctxt->node; + + if (VIR_ALLOC(ret) < 0) + goto error; + + port = virXPathString("string(./nbd/@port)", ctxt); + if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed nbd port '%s'"), + port); + goto error; + } + + /* Now check if source sent a list of disks to prealloc. We might be + * talking to an older server, so it's not an error if the list is + * missing. */ + if ((n = virXPathNodeSet("./nbd/disk", ctxt, &disks)) > 0) { + if (VIR_ALLOC_N(ret->target, n) < 0 || + VIR_ALLOC_N(ret->alloc, n) < 0)
capacity ...
+ goto error; + ret->ndisks = n; + + for (i = 0; i < n; i++) { + ctxt->node = disks[i]; + VIR_FREE(alloc); + + ret->target[i] = virXPathString("string(./@target)", ctxt); + alloc = virXPathString("string(./@alloc)", ctxt); + if (virStrToLong_ull(alloc, NULL, 10, (unsigned long long *) + &ret->alloc[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed disk allocation: '%s'"), + alloc); + goto error; + } + } + } + + cleanup: + VIR_FREE(port); + VIR_FREE(alloc); + VIR_FREE(disks); + ctxt->node = save_ctxt; + return ret; + error: + qemuMigrationCookieNBDFree(ret); + ret = NULL; + goto cleanup; +} + + static qemuDomainJobInfoPtr qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) { @@ -1123,22 +1278,9 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, goto error;
if (flags & QEMU_MIGRATION_COOKIE_NBD && - virXPathBoolean("boolean(./nbd)", ctxt)) { - char *port; - - if (VIR_ALLOC(mig->nbd) < 0) - 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); - } + virXPathBoolean("boolean(./nbd)", ctxt) && + (!(mig->nbd = qemuMigrationCookieNBDXMLParse(ctxt)))) + goto error;
if (flags & QEMU_MIGRATION_COOKIE_STATS && virXPathBoolean("boolean(./statistics)", ctxt) &&
Peter