[virt-tools-list] [RFC Patch v2] Support job cancellation for migration, save functions
Cole Robinson
crobinso at redhat.com
Wed Dec 1 21:21:59 UTC 2010
On 11/30/2010 10:35 PM, Wen Congyang wrote:
> # HG changeset patch
> # User Wen Congyang <wency at cn.fujitsu.com>
> # Date 1291173826 -28800
> # Node ID da5771e13f5a395c3fff8bcbdf2769d308034744
> # Parent dedbf8d0d5a34399a2baa8422b0f154a1c243bb0
> Support job cancellation for migration, save functions
>
> I do not use the API libvirt.jobinfo() to check whether the job is cancelled,
> because the jobinfo is cleared when job ends...
>
Thanks for the patch. A couple suggestions. Again I added a virtinst
support check for jobInfo(), we can roughly use this to guess if
libvirt/the hypervisor supports cancel jobs as well:
http://hg.fedorahosted.org/hg/python-virtinst/rev/871813b2dda1
So I would recommend actually hiding the cancel button if the HV doesn't
support jobInfo, otherwise we will eventually get bug reports about the
cancel button not being clickable for other actions.
You should be able to use the test driver (virt-manager --connect
test:///default) to simulate a connection that doesn't support jobInfo,
to make sure that everything is working correctly.
Also, any plans to implement save/migrate progress reporting? If not, no
worries.
> diff -r dedbf8d0d5a3 -r da5771e13f5a src/virtManager/asyncjob.py
> --- a/src/virtManager/asyncjob.py Tue Nov 30 10:52:11 2010 -0500
> +++ b/src/virtManager/asyncjob.py Wed Dec 01 11:23:46 2010 +0800
> @@ -40,6 +40,7 @@
> class vmmAsyncJob(gobject.GObject):
>
> def __init__(self, config, callback, args=None,
> + cancel_back=None, cancel_args=None,
Probably safer to move these options to the end of the opt list, incase
any AsyncJob callers are relying on the text/title option order (I
didn't check, but it's better to be safe anyways)
> text=_("Please wait a few moments..."),
> title=_("Operation in progress"),
> run_main=True):
> @@ -50,6 +51,16 @@
> self.window = gtk.glade.XML(config.get_glade_dir() + \
> "/vmm-progress.glade",
> "vmm-progress", domain="virt-manager")
> + self.window.signal_autoconnect({
> + "on_async_job_delete_event" : self.cancel,
> + "on_async_job_cancel_clicked" : self.cancel,
> + })
Hmm, not sure if we want to overwrite the window delete event.
Previously that wouldn't cancel a migration, so we shouldn't change this
behavior silently. Maybe launch a confirm dialog or something?
> + self.cancel_job = cancel_back
> + self.cancel_args = cancel_args
> + if self.cancel_job:
> + self.window.get_widget("cancel-async-job").set_sensitive(True)
> + else:
> + self.window.get_widget("cancel-async-job").set_sensitive(False)
> self.window.get_widget("pbar-text").set_text(text)
>
> self.topwin = self.window.get_widget("vmm-progress")
> @@ -91,6 +102,10 @@
>
> self.topwin.destroy()
>
> + def cancel(self, ignore1=None, ignore2=None):
> + if self.cancel_job:
> + self.cancel_job(*self.cancel_args)
> +
> def pulse_pbar(self, progress="", stage=None):
> gtk.gdk.threads_enter()
> try:
> diff -r dedbf8d0d5a3 -r da5771e13f5a src/virtManager/domain.py
> --- a/src/virtManager/domain.py Tue Nov 30 10:52:11 2010 -0500
> +++ b/src/virtManager/domain.py Wed Dec 01 11:23:46 2010 +0800
> @@ -1001,6 +1001,9 @@
> if self.get_autostart() != val:
> self._backend.setAutostart(val)
>
> + def abort_job(self):
> + self._backend.abortJob()
> +
> def migrate_set_max_downtime(self, max_downtime, flag=0):
> self._backend.migrateSetMaxDowntime(max_downtime, flag)
>
> diff -r dedbf8d0d5a3 -r da5771e13f5a src/virtManager/engine.py
> --- a/src/virtManager/engine.py Tue Nov 30 10:52:11 2010 -0500
> +++ b/src/virtManager/engine.py Wed Dec 01 11:23:46 2010 +0800
> @@ -828,8 +828,10 @@
> if not path:
> return
>
> + job_info = {"is_cancelled" : False}
Hmm, rather than use this dictionary hack, I'd just track it in the
Async class with a job_canceled variable
> progWin = vmmAsyncJob(self.config, self._save_callback,
> - [vm, path],
> + [vm, path, job_info],
> + self._save_cancel, [vm, job_info],
> _("Saving Virtual Machine"))
> progWin.run()
> error, details = progWin.get_error()
> @@ -837,7 +839,12 @@
> if error is not None:
> src.err.show_err(_("Error saving domain: %s") % error, details)
>
> - def _save_callback(self, vm, file_to_save, asyncjob):
> + def _save_cancel(self, vm, job_info):
> + vm.abort_job()
> + job_info["is_cancelled"] = True
> + return
> +
What if this throws an error, will the user ever see it? Maybe if we
could find a way to update the progress dialog with a little warning
that cancellation failed.
Thanks,
Cole
> + def _save_callback(self, vm, file_to_save, job_info, asyncjob):
> try:
> conn = util.dup_conn(self.config, vm.connection,
> return_conn_class=True)
> @@ -845,7 +852,11 @@
>
> newvm.save(file_to_save)
> except Exception, e:
> - asyncjob.set_error(str(e), "".join(traceback.format_exc()))
> + if not (isinstance(e, libvirt.libvirtError) and
> + job_info["is_cancelled"]):
> + # If job is cancelled, we should not report the error
> + # to user.
> + asyncjob.set_error(str(e), "".join(traceback.format_exc()))
>
> def _do_restore_domain(self, src, uri):
> conn = self._lookup_connection(uri)
> diff -r dedbf8d0d5a3 -r da5771e13f5a src/virtManager/migrate.py
> --- a/src/virtManager/migrate.py Tue Nov 30 10:52:11 2010 -0500
> +++ b/src/virtManager/migrate.py Wed Dec 01 11:23:46 2010 +0800
> @@ -448,9 +448,11 @@
> self.topwin.set_sensitive(False)
> self.topwin.window.set_cursor(gtk.gdk.Cursor(gtk.gdk.WATCH))
>
> + job_info = {"is_cancelled" : False}
> progWin = vmmAsyncJob(self.config, self._async_migrate,
> [self.vm, destconn, uri, rate, live, secure,
> - max_downtime],
> + max_downtime, job_info],
> + self.cancel_migration, [self.vm, job_info],
> title=_("Migrating VM '%s'" % self.vm.get_name()),
> text=(_("Migrating VM '%s' from %s to %s. "
> "This may take awhile.") %
> @@ -484,8 +486,13 @@
> logging.warning("Error setting migrate downtime: %s" % e)
> return False
>
> + def cancel_migration(self, vm, job_info):
> + vm.abort_job()
> + job_info["is_cancelled"] = True
> + return
> +
> def _async_migrate(self, origvm, origdconn, migrate_uri, rate, live,
> - secure, max_downtime, asyncjob):
> + secure, max_downtime, job_info, asyncjob):
> errinfo = None
> try:
> try:
> @@ -514,8 +521,12 @@
> if timer:
> gobject.source_remove(timer)
> except Exception, e:
> - errinfo = (str(e), ("Unable to migrate guest:\n %s" %
> - "".join(traceback.format_exc())))
> + if not (isinstance(e, libvirt.libvirtError) and
> + job_info["is_cancelled"]):
> + # If job is cancelled, we should not report the error
> + # to user.
> + errinfo = (str(e), ("Unable to migrate guest:\n %s" %
> + "".join(traceback.format_exc())))
> finally:
> if errinfo:
> asyncjob.set_error(errinfo[0], errinfo[1])
> diff -r dedbf8d0d5a3 -r da5771e13f5a src/vmm-progress.glade
> --- a/src/vmm-progress.glade Tue Nov 30 10:52:11 2010 -0500
> +++ b/src/vmm-progress.glade Wed Dec 01 11:23:46 2010 +0800
> @@ -12,6 +12,7 @@
> <property name="type_hint">dialog</property>
> <property name="skip_taskbar_hint">True</property>
> <property name="urgency_hint">True</property>
> + <signal name="delete_event" handler="on_async_job_delete_event"/>
> <child>
> <widget class="GtkAlignment" id="alignment134">
> <property name="visible">True</property>
> @@ -82,6 +83,40 @@
> <property name="position">2</property>
> </packing>
> </child>
> + <child>
> + <widget class="GtkHBox" id="hbox1">
> + <property name="visible">True</property>
> + <property name="spacing">12</property>
> + <child>
> + <widget class="GtkLabel" id="label1">
> + <property name="visible">True</property>
> + </widget>
> + <packing>
> + <property name="position">0</property>
> + </packing>
> + </child>
> + <child>
> + <widget class="GtkButton" id="cancel-async-job">
> + <property name="label">gtk-cancel</property>
> + <property name="visible">True</property>
> + <property name="can_focus">True</property>
> + <property name="receives_default">True</property>
> + <property name="use_stock">True</property>
> + <signal name="clicked" handler="on_async_job_cancel_clicked"/>
> + </widget>
> + <packing>
> + <property name="expand">False</property>
> + <property name="fill">False</property>
> + <property name="position">1</property>
> + </packing>
> + </child>
> + </widget>
> + <packing>
> + <property name="expand">False</property>
> + <property name="fill">False</property>
> + <property name="position">3</property>
> + </packing>
> + </child>
> </widget>
> </child>
> </widget>
More information about the virt-tools-list
mailing list