[libvirt] [PATCH 0/2] Cleanup storage migration code a bit
Jiri Denemark (2): qemu: Keep track of what disks are being migrated qemu: Don't give up on first error in qemuMigrationCancelDriverMirror src/conf/domain_conf.h | 2 ++ src/qemu/qemu_migration.c | 36 +++++++++++++++++------------------- 2 files changed, 19 insertions(+), 19 deletions(-) -- 2.4.0
Instead of redoing the same filtering over and over everytime we need to walk through all disks which are being migrated. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_migration.c | 23 ++++++----------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..391f49a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -703,6 +703,8 @@ struct _virDomainDiskDef { int blockJobStatus; /* status of the finished block job */ bool blockJobSync; /* the block job needs synchronized termination */ + bool migrating; /* the disk is being migrated */ + struct { unsigned int cylinders; unsigned int heads; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c1af704..7448794 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1744,13 +1744,7 @@ qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; - /* skip shared, RO and source-less disks */ - if (disk->src->shared || disk->src->readonly || - !virDomainDiskGetSource(disk)) - continue; - - /* skip disks that didn't start mirroring */ - if (!disk->blockJobSync) + if (!disk->migrating || !disk->blockJobSync) continue; /* process any pending event */ @@ -1872,17 +1866,13 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; - /* skip shared, RO and source-less disks */ - if (disk->src->shared || disk->src->readonly || - !virDomainDiskGetSource(disk)) - continue; - - /* skip disks that didn't start mirroring */ - if (!disk->blockJobSync) + if (!disk->migrating || !disk->blockJobSync) continue; if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) return -1; + + disk->migrating = false; } return 0; @@ -1973,6 +1963,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, qemuBlockJobSyncEnd(driver, vm, disk, NULL); goto cleanup; } + disk->migrating = true; } /* Wait for each disk to become ready in turn, but check the status @@ -1980,9 +1971,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; - /* skip shared, RO and source-less disks */ - if (disk->src->shared || disk->src->readonly || - !virDomainDiskGetSource(disk)) + if (!disk->migrating) continue; while (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { -- 2.4.0
On Tue, May 12, 2015 at 02:37:09PM +0200, Jiri Denemark wrote:
Instead of redoing the same filtering over and over everytime we need to walk through all disks which are being migrated.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_migration.c | 23 ++++++----------------- 2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..391f49a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -703,6 +703,8 @@ struct _virDomainDiskDef { int blockJobStatus; /* status of the finished block job */ bool blockJobSync; /* the block job needs synchronized termination */
+ bool migrating; /* the disk is being migrated */
I'm not a huge fan of the fact that we're filling the generic virDomainDiskDef struct with ever more QEMU driver state fields. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, May 12, 2015 at 13:53:20 +0100, Daniel P. Berrange wrote:
On Tue, May 12, 2015 at 02:37:09PM +0200, Jiri Denemark wrote:
Instead of redoing the same filtering over and over everytime we need to walk through all disks which are being migrated.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_migration.c | 23 ++++++----------------- 2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..391f49a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -703,6 +703,8 @@ struct _virDomainDiskDef { int blockJobStatus; /* status of the finished block job */ bool blockJobSync; /* the block job needs synchronized termination */
+ bool migrating; /* the disk is being migrated */
I'm not a huge fan of the fact that we're filling the generic virDomainDiskDef struct with ever more QEMU driver state fields.
Hmm, I think we could just introduce a per-disk priv where we would hide all this stuff. Jirka
When cancelling drive mirror, always try to do that for all disks even if it fails for some of them. Report the first error we saw. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7448794..2dce44b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1861,6 +1861,8 @@ static int qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, virDomainObjPtr vm) { + virErrorPtr err = NULL; + int ret = 0; size_t i; for (i = 0; i < vm->def->ndisks; i++) { @@ -1869,13 +1871,20 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, if (!disk->migrating || !disk->blockJobSync) continue; - if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) - return -1; + if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) { + ret = -1; + if (!err) + err = virSaveLastError(); + } disk->migrating = false; } - return 0; + if (err) { + virSetError(err); + virFreeError(err); + } + return ret; } -- 2.4.0
participants (2)
-
Daniel P. Berrange -
Jiri Denemark