<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Jun 26, 2018 at 4:25 PM Richard W.M. Jones <<a href="mailto:rjones@redhat.com">rjones@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">In the case where virt-v2v runs on the same server as the imageio<br>
daemon that we are talking to, it may be possible to optimize access<br>
using a Unix domain socket.<br>
<br>
This is only an optimization. If it fails or if we're not running on<br>
the same server it will fall back to the usual HTTPS over TCP<br>
connection.<br>
<br>
Thanks: Nir Soffer, Daniel Erez.<br>
---<br>
v2v/rhv-upload-plugin.py | 61 +++++++++++++++++++++++++++++++++++++++++++++---<br>
1 file changed, 58 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py<br>
index f215eaecf..354cc524c 100644<br>
--- a/v2v/rhv-upload-plugin.py<br>
+++ b/v2v/rhv-upload-plugin.py<br>
@@ -19,11 +19,12 @@<br>
import builtins<br>
import json<br>
import logging<br>
+import socket<br>
import ssl<br>
import sys<br>
import time<br>
<br>
-from http.client import HTTPSConnection<br>
+from http.client import HTTPSConnection, HTTPConnection<br>
from urllib.parse import urlparse<br>
<br>
import ovirtsdk4 as sdk<br>
@@ -56,6 +57,28 @@ def debug(s):<br>
print(s, file=sys.stderr)<br>
sys.stderr.flush()<br>
<br>
+def find_host(connection):<br>
+ """Return the current host object or None."""<br></blockquote><div><br></div><div>I think we need a comment for this, something like:</div><div><br></div><div># If we are running on a oVirt host, it must have a hardware id</div><div># file.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+ try:<br>
+ with builtin_open("/etc/vdsm/<a href="http://vdsm.id" rel="noreferrer" target="_blank">vdsm.id</a>") as f:<br>
+ vdsm_id = f.readline().strip()<br>
+ except Exception as e:<br>
+ return None<br></blockquote><div><br></div><div>A debug message about no hardware id file would be nice to the person</div><div>looking at the logs later. Without this you will have to detect this case</div><div>by not seeing the next debug line. Possible but require more effort.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ debug("hw_id = %r" % vdsm_id)<br>
+<br></blockquote><div><br></div><div>A comment abut looking up the host id would be nice. The code is not</div><div>explaining it self very well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ hosts_service = connection.system_service().hosts_service()<br>
+ hosts = hosts_service.list(<br>
+ search="hw_id=%s" % vdsm_id,<br>
+ case_sensitive=False,<br>
+ )<br>
+ if len(hosts) == 0:<br>
+ return None<br></blockquote><div><br></div><div>Debug message about unknown host would be nice.</div><div><br></div><div>Did you try to feed non existing hardware id? do we actually get empty</div><div>list?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ host = hosts[0]<br>
+ debug("<a href="http://host.id" rel="noreferrer" target="_blank">host.id</a> = %r" % <a href="http://host.id" rel="noreferrer" target="_blank">host.id</a>)<br>
+<br>
+ return types.Host(id = <a href="http://host.id" rel="noreferrer" target="_blank">host.id</a>)<br></blockquote><div><br></div><div>I like this, simple and easy to follow.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
def open(readonly):<br>
# Parse out the username from the output_conn URL.<br>
parsed = urlparse(params['output_conn'])<br>
@@ -121,9 +144,11 @@ def open(readonly):<br>
transfers_service = system_service.image_transfers_service()<br>
<br>
# Create a new image transfer.<br>
+ host = find_host(connection)<br>
transfer = transfers_service.add(<br>
types.ImageTransfer(<br>
disk = types.Disk(id = <a href="http://disk.id" rel="noreferrer" target="_blank">disk.id</a>),<br>
+ host = host,<br>
inactivity_timeout = 3600,<br>
)<br></blockquote><div><br></div><div>Nice!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
)<br>
@@ -170,6 +195,7 @@ def open(readonly):<br>
can_flush = False<br>
can_trim = False<br>
can_zero = False<br>
+ unix_socket = None<br>
<br>
http.putrequest("OPTIONS", destination_url.path)<br>
http.putheader("Authorization", transfer.signed_ticket)<br>
@@ -186,6 +212,7 @@ def open(readonly):<br>
can_flush = "flush" in j['features']<br>
can_trim = "trim" in j['features']<br>
can_zero = "zero" in j['features']<br>
+ unix_socket = j.get('unix_socket')<br>
<br>
# Old imageio servers returned either 405 Method Not Allowed or<br>
# 204 No Content (with an empty body). If we see that we leave<br>
@@ -197,8 +224,17 @@ def open(readonly):<br>
raise RuntimeError("could not use OPTIONS request: %d: %s" %<br>
(r.status, r.reason))<br>
<br>
- debug("imageio features: flush=%r trim=%r zero=%r" %<br>
- (can_flush, can_trim, can_zero))<br>
+ debug("imageio features: flush=%r trim=%r zero=%r unix_socket=%r" %<br>
+ (can_flush, can_trim, can_zero, unix_socket))<br>
+<br>
+ # If we are connected to imageio on the local host and the<br>
+ # transfer features a unix_socket then we can reconnect to that.<br>
+ if host is not None and unix_socket is not None:<br>
+ try:<br>
+ http = UnixHTTPConnection(unix_socket)<br>
+ debug("optimizing connection using unix socket %r" % unix_socket)<br>
+ except:<br></blockquote><div><br></div><div>I think this should be:</div><div><br></div><div>except Exception as e:</div><div> debug("error using unix socket connection, using https connection: %s" % e)</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ pass<br>
<br>
# Save everything we need to make requests in the handle.<br>
return {<br>
@@ -463,3 +499,22 @@ def close(h):<br>
raise<br>
<br>
connection.close()<br>
+<br>
+# Modify http.client.HTTPConnection to work over a Unix domain socket.<br>
+# Derived from uhttplib written by Erik van Zijst under an MIT license.<br>
+# (<a href="https://pypi.org/project/uhttplib/" rel="noreferrer" target="_blank">https://pypi.org/project/uhttplib/</a>)<br>
+# Ported to Python 3 by Irit Goihman.<br>
+<br>
+class UnsupportedError(Exception):<br>
+ pass<br></blockquote><div><br></div><div>Yes this is unneeded now.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+class UnixHTTPConnection(HTTPConnection):<br>
+ def __init__(self, path, timeout=socket._GLOBAL_DEFAULT_TIMEOUT):<br>
+ self.path = path<br>
+ HTTPConnection.__init__(self, "localhost", timeout=timeout)<br>
+<br>
+ def connect(self):<br>
+ self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)<br>
+ if self.timeout is not socket._GLOBAL_DEFAULT_TIMEOUT:<br>
+ self.sock.settimeout(timeout)<br>
+ self.sock.connect(self.path)<br>
-- <br>
2.16.2<br></blockquote><div> </div></div></div>