Re: [Libguestfs] [PATCH v2 2/2] v2v: -o rhv-upload: Use Unix domain socket to access imageio (RHBZ#1588088).

On Fri, Jun 22, 2018 at 2:59 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > In the case where virt-v2v runs on the same server as the imageio > daemon that we are talking to, it may be possible to optimize access > using a Unix domain socket. > > This is only an optimization. If it fails or if we're not running on > the same server it will fall back to the usual HTTPS over TCP > connection. > I wonder how this can happen. If we do: - get current host hardware id - ask engine to start transfer on this host Then there is no way that the transfer will not run on the current host, and we can use unix socket if imageio supports it.

> + # Get the current host. If it fails, don't worry. > + host = None > + try: > + with builtin_open("/etc/vdsm/vdsm.id") as f: > Why use builtin_open() instead of open() or io.open()?

> + vdsm_id = f.readline().strip() > debug log with the host hardware id would be nice here.

> + > + hosts_service = connection.system_service().hosts_service() > + hosts = hosts_service.list( > + search="hw_id=%s" % vdsm_id, > + case_sensitive=False, > + ) > + if len(hosts) > 0: > + host = hosts[0] > + debug("host.id = %r" % host.id) > + else: > + debug("could not retrieve host with hw_id=%s" % vdsm_id) > + except: > + pass > This will make it vey hard to debug slow uploads. I agree that optimization can be skipped on errors, but we must log a detailed error message before we fallback to https. Also bare except should be used only in one case, when you do: try: ... except: cleanup raise If you don't raise, you should use Exception. Otherwise you will swallow KeyboardInterrupt, SystemExit, and GeneratorExit and any other user defined exceptions inheriting from BaseException. Also it will be easier to maintain this if this will be in a separate function like: def find_host(): read hardware id or return None... get hosts from engine or return None... return types.Host(host.id)