Hi
Thanks for reviewing.
> I think the nice way to check the port would be to have a function that
> actually attempts to bind the port, to test that it is empty. You would
> understandably have to release it if you succeeded so the install can use
> it in the future. I'm not sure if this would carry any residual effects,
> maybe someone else has a better idea?
>
This idea looks better.
I remake the patch.
This patch checks the port by using socket.
Thanks,
Masayuki Sunou
-------------------------------------------------------------------------------
diff -r f40212ea1fd6 virtinst/cli.py
--- a/virtinst/cli.py Wed Nov 07 16:31:59 2007 -0500
+++ b/virtinst/cli.py Fri Nov 09 11:55:28 2007 +0900
@@ -15,6 +15,7 @@ import os, sys
import os, sys
import logging
import logging.handlers
+import socket
import libvirt
import util
@@ -207,6 +208,8 @@ def get_graphics(vnc, vncport, nographic
guest.graphics = False
return
if vnc is not None:
+ if vncport is not None and vncport >= 5900:
+ vncport = port_search(vncport)
guest.graphics = (True, "vnc", vncport, keymap)
return
if sdl is not None:
@@ -216,6 +219,8 @@ def get_graphics(vnc, vncport, nographic
res = prompt_for_input(_("Would you like to enable graphics support? (yes or no)"))
try:
vnc = yes_or_no(res)
+ if vncport is not None and vncport >= 5900:
+ vncport = port_search(vncport)
except ValueError, e:
print _("ERROR: "), e
continue
@@ -224,6 +229,26 @@ def get_graphics(vnc, vncport, nographic
else:
guest.graphics = False
break
+
+def port_search(vncport):
+ while 1:
+ try:
+ s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ s.bind(('', vncport))
+ except socket.error, e:
+ s.close()
+ s = None
+ if s is not None:
+ s.close()
+ return vncport
+ port = prompt_for_input(_("Installation cannot be continued, because the specified port number is already used.\n Please set another port number or just push enter to allocate a port number automatically."))
+ try:
+ if port is "":
+ return None
+ else:
+ vncport = int(port)
+ except ValueError, e:
+ print _("ERROR: "), e
### Option parsing
def check_before_store(option, opt_str, value, parser):
-------------------------------------------------------------------------------
In message <473231BD.7010101@xxxxxxxxxx>
"Re: [PATCH] Strengthen port number validation"
"Cole Robinson <crobinso@xxxxxxxxxx>" wrote:
> Masayuki Sunou wrote:
> > Hi
> >
> > Installation fails when port number used by other processes is set
> > to --vncport of virt-install, because graphical console is not displayed.
> > The same problem occurs when port number exceeds upper bound.
> >
> > One of patches fixes to request re-input when port number used is set.
> > --> check_vncport_used.patch
> > Other fixes to output error message when port number exceeds upper bound.
> > --> check_vncport_upperbound.patch
> >
> > Signed-off-by: Masayuki Sunou <fj1826dm@xxxxxxxxxxxxxxxxx>
> >
> > Thanks,
> > Masayuki Sunou.
>
>
> Hi,
>
> The upperbound check looks good, I just applied it.
>
> The vncport collision detection though I'm a bit worried about. Parsing
> 'netstat' doesn't seem like a nice solution: its a lot of output to parse
> for little gain and requires an external utility to do it.
>
> I think the nice way to check the port would be to have a function that
> actually attempts to bind the port, to test that it is empty. You would
> understandably have to release it if you succeeded so the install can use
> it in the future. I'm not sure if this would carry any residual effects,
> maybe someone else has a better idea?
>
> - Cole
>
> --
> Cole Robinson
> crobinso@xxxxxxxxxx
>
> _______________________________________________
> et-mgmt-tools mailing list
> et-mgmt-tools@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/et-mgmt-tools
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
