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@xxxxxxxxxx>
> > ---
> > 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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list