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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list