[master/rhel7/f21] Start vncconfig for cutNpaste

David Shea dshea at redhat.com
Wed Dec 3 15:39:29 UTC 2014


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.


More information about the anaconda-patches mailing list