Re: [libvirt] [PATCH 2/3] qemu: migration: error if tunnelled + storage specified

On 28.05.2013 22:44, Cole Robinson wrote: > Since as the code indicates it doesn't work yet, so let's be > explicit about it. > --- > src/qemu/qemu_migration.c | 24 ++++++++++-------------- > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index 9ac9be4..ffc86a4 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -1900,12 +1900,13 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver, > if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && > virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { > /* TODO support NBD for TUNNELLED migration */ > - if (flags & VIR_MIGRATE_TUNNELLED) > - VIR_DEBUG("NBD in tunnelled migration is currently not supported"); > - else { > - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; > - priv->nbdPort = 0; > + if (flags & VIR_MIGRATE_TUNNELLED) { > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > + _("NBD in tunnelled migration is currently not supported")); > + goto cleanup; > } > + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; > + priv->nbdPort = 0; > } > > if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) > @@ -2200,16 +2201,11 @@ done: > if (mig->nbd && > flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && > virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { > - /* TODO support NBD for TUNNELLED migration */ > - if (flags & VIR_MIGRATE_TUNNELLED) > - VIR_DEBUG("NBD in tunnelled migration is currently not supported"); > - else { > - if (qemuMigrationStartNBDServer(driver, vm, listenAddr) < 0) { > - /* error already reported */ > - goto endjob; > - } > - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; > + if (qemuMigrationStartNBDServer(driver, vm, listenAddr) < 0) { > + /* error already reported */ > + goto endjob; > } > + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; > } > > if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, > I know you've already pushed this but I don't think this is desired. There are currently two ways to copy the storage during migration: 1) new one using nbd server 2) old one, using an 'copy_storage' attribute to 'migrate' command The first one is preferred and turned on whenever libvirt detects both sides support it. There's no way for a user to enforce one or another way of copying the storage. So if the old way works even for tunnelled migration, we should fall back to using the old way rather then erroring out. The debug message was really just a debug message :)