[libvirt] [PATCH] qemu: migrate: Error if we collide with an inactive domain

[Daniel P. Berrange]

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 at 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 :|