On 05/26/2015 09:01 AM, Michal Privoznik wrote:
From: Pavel Boldin <pboldin@mirantis.com>
Implement a `migrate_disks' parameters for the QEMU driver. This multi- value parameter can be used to explicitly specify what block devices are to be migrated using the NBD server. Tunnelled migration using NBD is to be done.
Signed-off-by: Pavel Boldin <pboldin@mirantis.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 9 ++ src/qemu/qemu_driver.c | 72 +++++++++--- src/qemu/qemu_migration.c | 245 ++++++++++++++++++++++++++++----------- src/qemu/qemu_migration.h | 24 ++-- 4 files changed, 258 insertions(+), 92 deletions(-)
Didn't see much major - just a NIT or two... Also noted sometimes the order of arguments is nmigrate_disks, migrate_disks and others it's reversed (qemuMigrationBegin and virTypedParamsPickStrings). It's not a big deal to me personally, but there are those that do ;-) ...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 279b43f..5a8057e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1579,12 +1579,34 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, return ret; }
+static bool +qemuMigrateDisk(virDomainDiskDef const *disk, + size_t nmigrate_disks, const char **migrate_disks)
NIT: 3rd arg on own line
+{ + size_t i; + /* Check if the disk alias is in the list */ + if (nmigrate_disks) { + for (i = 0; i < nmigrate_disks; i++) { + if (STREQ(disk->dst, migrate_disks[i])) + return true; + } + return false; + } + + /* Default is to migrate only non-shared non-readonly disks + * with source */ + return !disk->src->shared && !disk->src->readonly && + virDomainDiskGetSource(disk); +} +
...
@@ -2710,6 +2734,8 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, const char *dname, char **cookieout, int *cookieoutlen, + size_t nmigrate_disks, + const char **migrate_disks, unsigned long flags) { char *rv = NULL; @@ -2721,9 +2747,11 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR);
VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s," - " cookieout=%p, cookieoutlen=%p, flags=%lx", + " cookieout=%p, cookieoutlen=%p," + " nmigrate_disks=%zu, migrate_disks=%p, flags=%lx", driver, vm, NULLSTR(xmlin), NULLSTR(dname), - cookieout, cookieoutlen, flags); + cookieout, cookieoutlen, nmigrate_disks, + migrate_disks, flags);
if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -2738,17 +2766,54 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (!qemuMigrationIsAllowed(driver, vm, NULL, true, abort_on_error)) goto cleanup;
- if (!(flags & VIR_MIGRATE_UNSAFE) && !qemuMigrationIsSafe(vm->def)) + if (!(flags & VIR_MIGRATE_UNSAFE) && + !qemuMigrationIsSafe(vm->def, nmigrate_disks, migrate_disks)) goto cleanup;
- if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR)) { - /* TODO support NBD for TUNNELLED migration */ - if (flags & VIR_MIGRATE_TUNNELLED) { - VIR_WARN("NBD in tunnelled migration is currently not supported"); - } else { - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; - priv->nbdPort = 0; + if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { + bool has_drive_mirror = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_DRIVE_MIRROR); + + if (nmigrate_disks) { + if (has_drive_mirror) { + size_t i, j; + + /* Check user requested only known disk targets. */ + for (i = 0; i < nmigrate_disks; i++) { + for (j = 0; j < vm->def->ndisks; j++) { + if (STREQ(vm->def->disks[j]->dst, migrate_disks[i])) + break; + } + + if (j == vm->def->ndisks) { + virReportError(VIR_ERR_INVALID_ARG, + _("disk target %s not found"), + migrate_disks[i]); + goto cleanup; + } + } + + if (flags & VIR_MIGRATE_TUNNELLED) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Selecting disks to migrate is not " + "implemented for tunnelled migration")); + goto cleanup; + }
NIT: this check could be done prior to loops...
+ } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("qemu does not support drive-mirror command"));
NIT: s/qemu/this qemu binary
+ goto cleanup;
A level of indention could be removed by if (nmigrate_disks) { size_t i, j; if (!has_drive_mirror) { virReportError() goto cleanup; } if (flags & VIR_MIGRATE_TUNNELLED) { virReportError() goto cleanup; } for (...) {} } Rest seemed OK to me, too. John