Daniel Erez
2018-Jul-05 07:46 UTC
[Libguestfs] [PATCH] v2v: rhv plugin - find suitable host
From: root <root@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) + 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). + return None + + datacenter = data_centers[0] + debug("hw_id = %r, datacenter = %s" % (vdsm_id, datacenter.name)) - 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: -- 2.17.1
Richard W.M. Jones
2018-Jul-05 07:56 UTC
[Libguestfs] [PATCH] v2v: rhv plugin - find suitable host
On Thu, Jul 05, 2018 at 10:46:17AM +0300, Daniel Erez 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) > + 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). > + return None > + > + datacenter = data_centers[0] > + debug("hw_id = %r, datacenter = %s" % (vdsm_id, datacenter.name)) > > - 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),I'm assuming these don't have to be quoted in some way?> case_sensitive=False, > ) > if len(hosts) == 0:I've added this to my queue, it should go upstream later today. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Daniel Erez
2018-Jul-05 08:38 UTC
Re: [Libguestfs] [PATCH] v2v: rhv plugin - find suitable host
On Thu, Jul 5, 2018 at 10:56 AM Richard W.M. Jones <rjones@redhat.com> wrote:> On Thu, Jul 05, 2018 at 10:46:17AM +0300, Daniel Erez wrote: > > From: root <root@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) > > + 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). > > + return None > > + > > + datacenter = data_centers[0] > > + debug("hw_id = %r, datacenter = %s" % (vdsm_id, datacenter.name)) > > > > - 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), > > I'm assuming these don't have to be quoted in some way? >we don't allow any special characters or whitespaces in datacenter's name, so it should be fine.> > > case_sensitive=False, > > ) > > if len(hosts) == 0: > > I've added this to my queue, it should go upstream later today. > > Thanks, > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat > http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > Fedora Windows cross-compiler. Compile Windows programs, test, and > build Windows installers. Over 100 libraries supported. > http://fedoraproject.org/wiki/MinGW >
Nir Soffer
2018-Jul-05 10:46 UTC
[Libguestfs] [PATCH] v2v: rhv plugin - find suitable host
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. Nir -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://listman.redhat.com/archives/libguestfs/attachments/20180705/5050aee0/attachment.htm>
Richard W.M. Jones
2018-Jul-05 12:04 UTC
Re: [Libguestfs] [PATCH] v2v: rhv plugin - find suitable host
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@redhat.com> wrote: > > > From: root <root@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@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