Please use [PATCH v2] for version 2 of a patch, etc. git format-patch --subject-prefix "PATCH v2" On 08/06/2013 04:44 PM, Giuseppe Scrivano wrote: > In case of connection failure, the user can either maintain the connection > or re-edit it. > > Split up `add_connection_to_ui' into `make_conn' and `register_conn' > to handle separately the object creation and its registration in the > list of connections (ui and conf). > > Solves: https://bugzilla.redhat.com/show_bug.cgi?id=617386 > Signed-off-by: Giuseppe Scrivano <gscrivano@xxxxxxx> > --- > virtManager/connect.py | 43 ++++++++++++++++++++++++++++++++- > virtManager/engine.py | 64 ++++++++++++++++++++++++++++++++++++-------------- > 2 files changed, 88 insertions(+), 19 deletions(-) > Behavior looks good now, but a comment inline. > diff --git a/virtManager/connect.py b/virtManager/connect.py > index 3aed22f..3b50efd 100644 > --- a/virtManager/connect.py > +++ b/virtManager/connect.py > @@ -111,9 +111,11 @@ class vmmConnect(vmmGObjectUI): > self.topwin.hide() > self.stop_browse() > > - def show(self, parent): > + def show(self, parent, initial_state=None): > logging.debug("Showing open connection") > self.reset_state() > + if initial_state: > + self.set_state(initial_state) > self.topwin.set_transient_for(parent) > self.topwin.present() > > @@ -314,6 +316,45 @@ class vmmConnect(vmmGObjectUI): > default_user = default_conn_user(conn) > self.widget("username-entry").set_text(default_user) > > + def set_state(self, state): > + hv = None > + if state.is_xen(): > + hv = HV_XEN > + elif state.is_qemu(): > + hv = HV_QEMU > + elif state.is_lxc(): > + hv = HV_LXC > + is_remote = bool(state.is_remote()) > + host = state.get_uri_hostname() > + autoconnect = state.get_autoconnect() > + connection = None > + transport = state.get_transport() > + user = transport[1] > + if transport[0]: > + if "tls" in transport[0]: > + connection = CONN_TLS > + elif "ssh" in transport[0]: > + connection = CONN_SSH > + elif "tcp" in transport[0]: > + connection = CONN_TCP > + > + self.widget("connection").set_sensitive(is_remote) > + if connection: > + self.widget("connection").set_active(connection) > + self.widget("connect-remote").set_active(is_remote) > + self.widget("username-entry").set_sensitive(is_remote) > + self.widget("hostname").set_sensitive(is_remote) > + if is_remote: > + self.widget("hostname").get_child().set_text(host) > + if(hv is not None): > + self.widget("hypervisor").set_active(hv) > + if (user): > + self.widget("username-entry").set_text(user) > + > + self.widget("autoconnect").set_sensitive(True) > + self.widget("autoconnect").set_active(autoconnect) > + > + This seems overkill. Why can't we add an argument to show() which skips the reset_state step? Hmm, I guess someone could reopen the connect wizard while the first connection is opening, which would mean the state is wrong when we re-show the wizard. Thinking some more, what we really want here is: Launch 'Open Connection', set parameters, hit 'connect' 'Open Connection' window is desensitized, async progress meter dialog opens up while connection is connecting If connection fails, we drop back to the still open connection wizard with same state still listed. That's how the create/clone/delete wizards work, basically make the operation synchronous. However truth be told I don't care enough about this bug to mandate that we do that. If you want to look into it it might be simple, or it might be a pain. But the above solution is definitely not ideal since it requires duplicating update of UI elements in two places (set_state and reset_state) with the latter in a rarely tested code path. So I'll take a patch with the show() option bypassing reset_state, or skipping that entirely and user loses the previous state if we reshow the connect wizard, or a patch implementing my suggestion above. Thanks, Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
