[Libguestfs] [PATCH] v2v: rhv plugin - find suitable host

[Richard W.M. Jones]

On Thu, Jul 05, 2018 at 01:46:48PM +0300, Nir Soffer wrote:
> On Thu, Jul 5, 2018 at 10:46 AM Daniel Erez <derez at redhat.com> wrote:
> 
> > From: root <root at localhost.localdomain>
> >
> > For direct upload, a suitable host must be in status 'Up'
> > and belong to the same datacenter as the created disk.
> > Added these criteria to the host search query.
> > ---
> >  v2v/rhv-upload-plugin.py | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> > index da309e288..c72f5e181 100644
> > --- a/v2v/rhv-upload-plugin.py
> > +++ b/v2v/rhv-upload-plugin.py
> > @@ -67,11 +67,23 @@ def find_host(connection):
> >          debug("cannot read /etc/vdsm/vdsm.id, using any host: %s" % e)
> >          return None
> >
> > -    debug("hw_id = %r" % vdsm_id)
> >
> 
> I would leave this as is...
> 
> 
> > +    system_service = connection.system_service()
> > +    storage_name = params['output_storage']
> > +    data_centers = system_service.data_centers_service().list(
> > +        search='storage=%s' % storage_name,
> > +        case_sensitive=False,
> > +    )
> > +    if len(data_centers) == 0:
> > +        # The storage domain is not attached to a datacenter
> > +        # (shouldn't happen, would fail on disk creation).
> >
> 
> A debug message here would be helpful.
> 
> 
> > +        return None
> > +
> > +    datacenter = data_centers[0]
> > +    debug("hw_id = %r, datacenter = %s" % (vdsm_id, datacenter.name))
> >
> 
> And log only the datacenter here, to match other logs in the plugin.
> 
> >
> > -    hosts_service = connection.system_service().hosts_service()
> > +    hosts_service = system_service.hosts_service()
> >      hosts = hosts_service.list(
> > -        search="hw_id=%s" % vdsm_id,
> > +        search="hw_id=%s and datacenter=%s and status=Up" % (vdsm_id,
> > datacenter.name),
> >          case_sensitive=False,
> >      )
> >      if len(hosts) == 0:
> >
> 
> We need to change the comments and debug message here. The current code is:
> 
>  77     if len(hosts) == 0:
>  78         # This oVirt host is not registered with engine.
>  79         debug("cannot find host with hw_id=%r, using any host" %
> vdsm_id)
>  80         return None
> 
> We are handling 3 cases:
> 1. host not found
> 2. host no in the datacenter
> 3. host not up
> 
> The comment and the debug message should make it clear.

OK, I dropped this patch again.

Can someone post an updated version which fixes this.

Another thing to fix is the Author field in the original patch was
wrong (root at localhost IIRC - Daniel needs to set up his .gitconfig
file properly).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html