On Wed, Oct 28, 2009 at 03:12:45PM +0100, Chris Lalancette wrote:

Cole Robinson wrote:

...

Currently the check for a VM name/uuid collision on the migrate destination only errors for active domains. Not sure why this wouldn't apply for non running VMs as well, so drop the check.

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86546e5..fb952d8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6341,13 +6341,10 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,

if (!vm) vm = virDomainFindByName(&driver->domains, dname); if (vm) { - if (virDomainIsActive(vm)) { - qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); + qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain with the same name or UUID already exists as '%s'"), + vm->def->name); + goto cleanup; }

if (!(vm = virDomainAssignDef(dconn,

No, I don't think we should drop this check. This is useful for a workaround where you want to make a guest "persistent" on all sides of a migration. True, we now have migration flags that do this for you, but that's only as of 0.7.2. I think there is still a use-case for defining a guest on a destination, and then migrating over there.

That being said, maybe we could tighten this check up. In particular, if the XML on the destination doesn't equal the incoming XML, then we should probably fail the migration. However, if they are the same, we should allow the migration.

It needs to match the logic in qemudDomainCreate() I believe. /* See if a VM with matching UUID already exists */ vm = virDomainFindByUUID(&driver->domains, def->uuid); if (vm) { /* UUID matches, but if names don't match, refuse it */ if (STRNEQ(vm->def->name, def->name)) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(vm->def->uuid, uuidstr); qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain '%s' is already defined with uuid %s"), vm->def->name, uuidstr); goto cleanup; } /* UUID & name match, but if VM is already active, refuse it */ if (virDomainIsActive(vm)) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain is already active as '%s'"), vm->def->name); goto cleanup; } virDomainObjUnlock(vm); } else { /* UUID does not match, but if a name matches, refuse it */ vm = virDomainFindByName(&driver->domains, def->name); if (vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(vm->def->uuid, uuidstr); qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain '%s' is already defined with uuid %s"), def->name, uuidstr); goto cleanup; } } ie, this is allowing a completely new incoming domain, or a matching name + UUID for an existing domain only if it is inactive. If matching existing domain is running, or if partial name/uuid match then it must be refused Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|