On Mon, Oct 03, 2011 at 04:07:24PM +0200, Michal Privoznik wrote:
This patch implements previous extension in qemu driver. That is, during prepare phase check for every source to be accessible. If not, but marked as optional, simply VIR_FREE the source. Moreover, if migration is persistent, we must check inactive xml, which is transfered at the finish phase, as well. --- src/qemu/qemu_migration.c | 51 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1122dab..4aa2532 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1039,6 +1039,48 @@ cleanup: return rv; }
+/* qemuCheckDisksPresence: + * @def domain definition + * + * Iterate over domain disks and check if source exists. + * If not and: + * - it's marked as optional, free() it. + * - it's marked as required, throw an error. + * + * Returns 0 on success (all remaining disks/sources exist + * or have been dropped), + * -1 on failure. + */ +static int +qemuCheckDisksPresence(virDomainDefPtr def) { + int ret = -1; + int i; + virDomainDiskDefPtr disk; + + for (i = 0; i < def->ndisks; i++) { + disk = def->disks[i]; + + if (virFileExists(disk->src)) { + if (disk->migration.optional == VIR_DOMAIN_DEVICE_MIGRATION_OPT_DROP) + VIR_FREE(disk->src); + + continue; + }
This is not going to play nice with RHEVM, where images are stored on a root squashing NFS server, where libvirtd has no visibility, but QEMU can access. At the very least you need to run this check in a separate process which is running as the QEMU user/group ID.
+ + if (disk->migration.optional == VIR_DOMAIN_DEVICE_MIGRATION_OPT_REQ) { + qemuReportError(VIR_ERR_NO_SOURCE, + _("no such file %s"), + disk->src); + goto cleanup; + } + + VIR_FREE(disk->src); + } + + ret = 0; +cleanup: + return ret; +}
/* Prepare is the first step, and it runs on the destination host. */ @@ -1087,6 +1129,9 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup;
+ if (qemuCheckDisksPresence(def) < 0) + goto cleanup; + if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { @@ -2579,9 +2624,11 @@ qemuMigrationFinish(struct qemud_driver *driver, if (vm->persistent) newVM = 0; vm->persistent = 1; - if (mig->persistent) + if (mig->persistent) { + if (qemuCheckDisksPresence(mig->persistent) < 0) + goto endjob;
What's the point in checking here ? You've already checked at te start of migration, and if it has gone away since then, it is too late for this check todo anything useful 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 :|