Re: [Kimchi-devel] [v2 1/1] Virtual machine migration

On 11-11-2014 02:37, simonjin(a)linux.vnet.ibm.com wrote: > +from kimchi.utils import add_task There's already an import "kimchi.utils" below. Add the "add_task" first to that list.

> from kimchi.exception import InvalidOperation, InvalidParameter > +from kimchi.model.libvirtconnection import LibvirtConnection > from kimchi.exception import NotFoundError, OperationFailed Add that line in alphabetical order.

> from kimchi.model.templates import TemplateModel > +from kimchi.model.templates import TemplateModel Isn't that class already imported on the line above?

> from kimchi.model.utils import get_vm_name > from kimchi.model.utils import get_metadata_node > from kimchi.model.utils import set_metadata_node > from kimchi.screenshot import VMScreenshot > +#from kimchi.model.vmhostdevs import VMHostDevsModel Unless that line is here due to debug status of this patch, there's no need to keep it commented.

> + if remoteHost in ['localhost', '127.0.0.1']: > + kimchi_log.debug('vm %s is not in running, only running > vm can be migrated' % name) > + raise OperationFailed("KCHVM00033E", {'name': name, > + 'remoteHost': > remoteHost}) I don't think that is the reason why the VM cannot be migrated... By looking at the code and at the message KCHVM00033E, the reason is that the user is trying to migrate to localhost.

> + if self.conn.get().getInfo()[0] != _destConn.getInfo()[0]: > + kimchi_log.debug('vm %s can not migrate to different > arch server.' % (name, _destConn.getInfo()[0])) > + raise OperationFailed("KCHVM00034E", {'name': name, > + 'srcarch': > self.conn.get().getType(), > + 'destarch': > _destConn.getType()}) 'srcarch' and 'destarch' don't seem to contain the servers' architectures, but their hypervisor type.

Also, only run "getInfo" (and other connection functions) once and store its value on a local variable, especially because we're dealing with remote connections and it might take a little more time to execute them.

> + > + if self.conn.get().getType() != _destConn.getType(): > + kimchi_log.debug('vm %s can not migrate to a different > hypervisor' % name) > + raise OperationFailed("KCHVM00035E", {'name': self.name, > + 'srchyp':self.conn.get().getType(), > + 'desthyp': > _destConn.getType()}) As I commented earlier, only run "getType" once and store the values on local variables. > + def _getRemoteConn(self, remoteHost, user): > + #open destination connect to migrate > + transport = 'ssh' > + duri = 'qemu+%s://%s@%s/system' % (transport, > + user,remoteHost) > + > + conn = LibvirtConnection(duri) > + return conn That function can be static, it doesn't use "self".

> + def _do_migrate(self, name, destConn, remoteHost, cb): > + _destConn = destConn.get() > + cb('starting a migration') > + kimchi_log.debug('migrate %s start' % name) > + #import pdb > + #pdb.set_trace() > + dom = self.get_vm(name, self.conn) > + flag = 0 > + flag |= (libvirt.VIR_MIGRATE_PEER2PEER | > + libvirt.VIR_MIGRATE_TUNNELLED) > + > + state = DOM_STATE_MAP[dom.info()[0]] > + if state == 'running': > + flag |= libvirt.VIR_MIGRATE_LIVE > + try: > + dom.migrateSetMaxDowntime(0) Isn't "migrateSetMaxDowntime" called in "_preMigrate", which is called before "_do_migrate"?

> + # Record vm migrate task for future querying > + try: > + with self.objstore as session: > + session.store('migrate', name, task_id) > + return task_id If I'm not mistaken, the AsyncTask constructor (which is called inside the function "add_task" above) already registers the task in the local object store.