On 10/06/2014 12:45 PM, David Shea wrote:
On 10/06/2014 11:12 AM, Mark Hamzy wrote:
> Another reason to start vncconfig in a vnc session is to support cut
> & paste.
>
> ---
> pyanaconda/vnc.py | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py
> index a7e2072..a9ba148 100644
> --- a/pyanaconda/vnc.py
> +++ b/pyanaconda/vnc.py
> @@ -177,6 +177,21 @@ class VncServer:
> maxTries), maxTries)
> return False
> + def startVncConfig(self):
> + """Attempt to start vncconfig"""
> +
> + self.log.info(_("Attempting to start vncconfig"))
There's no need to localize the log message.
Ok, I see the other log message now, and they are printed to the console
so I guess leaving them localized is the better idea.
> +
> + vncconfigcommand = [self.root+"/usr/bin/vncconfig",
> "-display", ":%s"%self.display]
I don't think self.root is needed here. I know it's used in the client
vncconfig command, but it could be removed there too since the object
isn't created with a root argument anymore.
Also, I think it would be preferable to run vncconfig with -nowin. The
window looks kind of weird to begin with since we disable decorations,
and the clipboard sharing stops as soon as you close it because that
ends the process. We already start the spice-based clipboard things
unconditionally so this would match that behavior.
I don't care if this stays as it is for consistency with the other
vncconfig command.
> +
> + try:
> + vncconfigp = subprocess.Popen(vncconfigcommand,
> stdout=self.openlogfile(), stderr=subprocess.STDOUT)
> +
> + return True
> + except OSError:
> + self.log.info(_("Could not start vncconfig: %s", e))
> + return False
> +
Please use iutil.startProgram instead of calling subprocess.Popen
directly.
This is the only part I really care about. I would also prefer to remove
the error checking, since vncconfig failing to run and returning OSError
is most likely going to be a corrupt stage2.
> def VNCListen(self):
> """Put the server in listening mode.
> @@ -247,6 +262,7 @@ class VncServer:
> sys.exit(1)
> # Lets try to configure the vnc server to whatever the
> user specified
> + connected = False
> if self.vncconnecthost != "":
> connected = self.connectToView()
> if not connected:
> @@ -256,6 +272,9 @@ class VncServer:
> os.environ["DISPLAY"]=":%s" % self.display
> + if not connected:
> + self.startVncConfig()
> +
> def changeVNCPasswdWindow(self):
> """ Change the password to a sane parameter.
What's the purpose of the connect variable? Shouldn't vncconfig be
started any time Xvnc is started? on
Taking another look, I see what you're trying to do (only start
vncconfig if it wasn't already started), but we actually do need a
second vncconfig in the vncconnect case. vncconfig -connect exits once
the connection has been established.
I'll send out a revised patch to run vncconfig unconditionally and use
startProgram.