7:01 p.m.
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(a)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