Re: [ovirt-devel] planned Jenkins restart

[Evgheni Dereveanchin]

Maintenance completed, Jenkins back up and running.
The most important change this time was a fix
to the artifact download corruption issue:
https://ovirt-jira.atlassian.net/browse/OVIRT-1150
As always - if you see any issues please report them to Jira.

Regards,
Evgheni Dereveanchin

On Tue, Mar 28, 2017 at 2:00 AM, Evgheni Dereveanchin 
wrote:

> Hi everyone,
>
> I'll be performing a planned Jenkins restart within the next hour.
> No new builds will be scheduled during this maintenance period.
> I will inform you once it is over.
>
> Regards,
> Evgheni Dereveanchin
>



-- 
Regards,
Evgheni Dereveanchin
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

[ovirt-devel] planned Jenkins restart

[Evgheni Dereveanchin]

Hi everyone,

I'll be performing a planned Jenkins restart within the next hour.
No new builds will be scheduled during this maintenance period.
I will inform you once it is over.

Regards,
Evgheni Dereveanchin
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] oVirt Engine checkstyle upgrade

[Allon Mureinik]

On Mon, Mar 27, 2017 at 8:48 PM, Vojtech Szocs  wrote:

>
>
> On Mon, Mar 27, 2017 at 2:10 PM, Allon Mureinik 
> wrote:
>
>>
>>
>> On Mon, Mar 27, 2017 at 3:09 PM, Allon Mureinik 
>> wrote:
>>
>>> Indeed, Greg.thanks
>>>
>>> As a temporary solution, you could cherry-pick https://gerrit.ovi
>>> rt.org/#/c/74611 and work above it.
>>> It should solve the issue (even though IMHO it isn't ready for merging
>>> to master).
>>>
>>> Vojta - are you updating https://gerrit.ovirt.org/#/c/74611 as per my
>>> comments there, or should I take it over?
>>>
>> I literally have no idea who I did made this typo.
>> I meant "Vojtech", of course.​
>>
>
> ​In Czech language, Vojta actually means Vojtech, in a less formal way =)
>
​In that case, I totally meant to type that :-)
​

>
> I've just updated https://gerrit.ovirt.org/#/c/74611/ according to your
> comments.
>
​I cleaned up some white-spacing noise there that seemed unrelated to the
patch and merged.
>From my local testing, using -Pgwtdev now works just find.

Votech (or Vojta, if you prefer ^_^) - many thanks for your efforts here.

Marek, and everyone else - please let me know if this still causes you any
grief.​




>
> The reason why GWT debugger fails to start is because one of the
> non-localized strings (which comes from `gwt-extension` and therefore
> shouldn't be checked in the first place..) contains a {} placeholder,
> NlsCheck logs it, and eventually crashes because it thinks it's a log
> message placeholder, but it's just something we want printed out.
>
>
>>
>>
>>
>>>
>>> On Mon, Mar 27, 2017 at 3:03 PM, Greg Sheremeta 
>>> wrote:
>>>
 From the comments in 74619

 "" "
 So, tl;dr - it /won't/ work with this patch but without 74611. This
 patch should be applied before 74611
 "" "

 On Mon, Mar 27, 2017 at 7:42 AM, Marek Libra  wrote:

> With
>   https://gerrit.ovirt.org/#/c/74619/
>
> applied, following is still failing:
>   make gwt-debug DEBUG_MODULE=webadmin DEV_EXTRA_BUILD_FLAGS_GWT_DEFA
> ULTS="-Dgwt.cssResourceStyle=pretty -Dgwt.userAgent=gecko1_8"
>
> with message:
> [WARNING] The requested profile "gwt-user" could not be activated
> because it does not exist.
> [ERROR] Failed to execute goal 
> org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check
> (checkstyle) on project webadmin: Failed during checkstyle configuration:
> Exception was thrown while processing /home/mlibra/IdeaProjects/ovir
> t-engine/frontend/webadmin/modules/gwt-extension/src/main/ja
> va/org/ovirt/engine/ui/uioverrides/org/slf4j/Logger.java: can't parse
> argument number: For input string: "" -> [Help 1]
>
>
> Any hint, please?
> Marek
>
>
>
> On Fri, Mar 24, 2017 at 4:14 PM, Vojtech Szocs 
> wrote:
>
>>
>>
>> On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs 
>> wrote:
>>
>>> Found the problem after debugging NlsCheck.
>>>
>>> First of all, it checks all kinds of Java sources, including the
>>> generated ones. That's silly and one of the reasons why Checkstyle
>>> execution takes a rather long time. I'll fix that.
>>>
>>> Next, when checking a Java source that contains string "{}" (without
>>> quotes) it will log the problem, but Checkstyle message logging infra
>>> things that "{}" is a placeholder to resolve, but it's not, and it 
>>> fails on
>>> NumberFormatException. I'll fix that too.
>>>
>>
>> ​https://gerrit.ovirt.org/#/c/74611/​
>>
>>
>>> Vojtech
>>>
>>>
>>> On Fri, Mar 24, 2017 at 3:19 PM, Vojtech Szocs 
>>> wrote:
>>>
 Hi Allon,

 I think I found some strange Checkstyle related problems on master.

 Engine build with (GWT compilation enabled) works OK.

 Next, trying to start GWT debugger:

 $ make gwt-debug DEBUG_MODULE=webadmin \
   DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.userAgent=gecko1_8,safari"
 \
   DEV_EXTRA_BUILD_FLAGS="-Dgwt.logLevel=INFO -Dgwt.locale=en_US
 -Dgwt.compiler.localWorkers=1" \
   DEV_BUILD_GWT_SUPER_DEV_MODE=1

 maven-checkstyle-plugin:check execution fails on

   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
 irt/engine/ui/uioverrides/org/ovirt/engine/core/compat/Forma
 tterDotnet.java
   can't parse argument number: (\\d)\\: For input string: "(\\d)\\"

 the class isn't used, removed it, retry. Now it fails on:

   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
 irt/engine/ui/uioverrides/org/slf4j/Logger.java
   can't parse argument number: For input string: ""

 I guess 

[ovirt-devel] Live demo session on how to debug oVirt GWT applications

[Vojtech Szocs]

Hello UI devs,

with oVirt master UI using the latest GWT SDK [1], I'd like to host a live
demo session with two goals in mind:

a, explain how the (Java IDE based) Classic Dev Mode works and mention its
limitations
b, explain how the (browser based) Super Dev Mode works and encourage
people to start using it to boost their productivity

[1] http://www.mail-archive.com/devel@ovirt.org/msg08558.html

The Classic vs. Super Dev Mode are two possible ways to debug GWT
applications. This session will give you the knowledge to decide which one
to use in a specific situation.

Proposed time: Mon, Apr 3 @ 5pm CET / 6pm TLV / 11am US EST. (This can be
changed as needed.)

Let me know if this kind of session interests you or if the above time
doesn't fit you but you'd still like to join.

Thanks,
Vojtech
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] oVirt Engine checkstyle upgrade

[Vojtech Szocs]

On Mon, Mar 27, 2017 at 2:10 PM, Allon Mureinik  wrote:

>
>
> On Mon, Mar 27, 2017 at 3:09 PM, Allon Mureinik 
> wrote:
>
>> Indeed, Greg.thanks
>>
>> As a temporary solution, you could cherry-pick https://gerrit.ovi
>> rt.org/#/c/74611 and work above it.
>> It should solve the issue (even though IMHO it isn't ready for merging to
>> master).
>>
>> Vojta - are you updating https://gerrit.ovirt.org/#/c/74611 as per my
>> comments there, or should I take it over?
>>
> I literally have no idea who I did made this typo.
> I meant "Vojtech", of course.​
>

​In Czech language, Vojta actually means Vojtech, in a less formal way =)

I've just updated https://gerrit.ovirt.org/#/c/74611/ according to your
comments.

The reason why GWT debugger fails to start is because one of the
non-localized strings (which comes from `gwt-extension` and therefore
shouldn't be checked in the first place..) contains a {} placeholder,
NlsCheck logs it, and eventually crashes because it thinks it's a log
message placeholder, but it's just something we want printed out.


>
>
>
>>
>> On Mon, Mar 27, 2017 at 3:03 PM, Greg Sheremeta 
>> wrote:
>>
>>> From the comments in 74619
>>>
>>> "" "
>>> So, tl;dr - it /won't/ work with this patch but without 74611. This
>>> patch should be applied before 74611
>>> "" "
>>>
>>> On Mon, Mar 27, 2017 at 7:42 AM, Marek Libra  wrote:
>>>
 With
   https://gerrit.ovirt.org/#/c/74619/

 applied, following is still failing:
   make gwt-debug DEBUG_MODULE=webadmin DEV_EXTRA_BUILD_FLAGS_GWT_DEFA
 ULTS="-Dgwt.cssResourceStyle=pretty -Dgwt.userAgent=gecko1_8"

 with message:
 [WARNING] The requested profile "gwt-user" could not be activated
 because it does not exist.
 [ERROR] Failed to execute goal 
 org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check
 (checkstyle) on project webadmin: Failed during checkstyle configuration:
 Exception was thrown while processing /home/mlibra/IdeaProjects/ovir
 t-engine/frontend/webadmin/modules/gwt-extension/src/main/ja
 va/org/ovirt/engine/ui/uioverrides/org/slf4j/Logger.java: can't parse
 argument number: For input string: "" -> [Help 1]


 Any hint, please?
 Marek



 On Fri, Mar 24, 2017 at 4:14 PM, Vojtech Szocs 
 wrote:

>
>
> On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs 
> wrote:
>
>> Found the problem after debugging NlsCheck.
>>
>> First of all, it checks all kinds of Java sources, including the
>> generated ones. That's silly and one of the reasons why Checkstyle
>> execution takes a rather long time. I'll fix that.
>>
>> Next, when checking a Java source that contains string "{}" (without
>> quotes) it will log the problem, but Checkstyle message logging infra
>> things that "{}" is a placeholder to resolve, but it's not, and it fails 
>> on
>> NumberFormatException. I'll fix that too.
>>
>
> ​https://gerrit.ovirt.org/#/c/74611/​
>
>
>> Vojtech
>>
>>
>> On Fri, Mar 24, 2017 at 3:19 PM, Vojtech Szocs 
>> wrote:
>>
>>> Hi Allon,
>>>
>>> I think I found some strange Checkstyle related problems on master.
>>>
>>> Engine build with (GWT compilation enabled) works OK.
>>>
>>> Next, trying to start GWT debugger:
>>>
>>> $ make gwt-debug DEBUG_MODULE=webadmin \
>>>   DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.userAgent=gecko1_8,safari"
>>> \
>>>   DEV_EXTRA_BUILD_FLAGS="-Dgwt.logLevel=INFO -Dgwt.locale=en_US
>>> -Dgwt.compiler.localWorkers=1" \
>>>   DEV_BUILD_GWT_SUPER_DEV_MODE=1
>>>
>>> maven-checkstyle-plugin:check execution fails on
>>>
>>>   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
>>> irt/engine/ui/uioverrides/org/ovirt/engine/core/compat/Forma
>>> tterDotnet.java
>>>   can't parse argument number: (\\d)\\: For input string: "(\\d)\\"
>>>
>>> the class isn't used, removed it, retry. Now it fails on:
>>>
>>>   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
>>> irt/engine/ui/uioverrides/org/slf4j/Logger.java
>>>   can't parse argument number: For input string: ""
>>>
>>> I guess it's a bug in our NON-NLS check? But why doesn't the problem
>>> occur during Engine build?
>>>
>>> I'm thinking about disabling Checkstyle for gwt-extension module, as
>>> it contains custom GWT RPC serializers and GWT class overrides, and 
>>> maybe
>>> the file path src/main/java/org/ovirt/engine
>>> /ui/uioverrides/here/goes/actual/pkg is confusing the Checkstyle
>>> now.
>>>
>>> Thanks,
>>> Vojtech
>>>
>>>
>>> On Wed, Mar 22, 2017 at 10:33 PM, Allon Mureinik <
>>> amure...@redhat.com> wrote:
>>>
 Hi all,

Re: [ovirt-devel] Firewalld migration.

[Martin Sivak]

> Clients should be made aware their custom rules are going to be obsolete and
> that they should reapply them once they reinstall.

Would you want to manually fix every reinstalled host? I would
consider that very annoying. This has to be somewhat automatic if we
want to support custom firewall rules. And although I agree the engine
is not the right place for that, it is the only central place we have
and from which we are starting the reinstall task.

Martin

On Mon, Mar 27, 2017 at 4:16 PM, Leon Goldberg  wrote:
> You're right, but I don't think it matters; hosts will remain unaffected
> until they're reinstalled via an upgraded Engine.
>
> Clients should be made aware their custom rules are going to be obsolete and
> that they should reapply them once they reinstall.
>
> On Mon, Mar 27, 2017 at 9:01 AM, Yedidyah Bar David  wrote:
>>
>> On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 
>> wrote:
>> > Effectively, upgrading will leave lingering (but nonetheless
>> > operational)
>> > iptables rules on the hosts. I'm not even sure there needs to be special
>> > upgrade treatment?
>>
>> Please describe the expected flow.
>>
>> Please note that at least when I tried, 'systemctl start firewalld' stops
>> iptables.
>>
>> Thanks,
>>
>> >
>> > On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David 
>> > wrote:
>> >>
>> >> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
>> >> wrote:
>> >> > 1) Do we actually need iptables for any reason that isn't a legacy
>> >> > consideration?
>> >>
>> >> No idea personally.
>> >>
>> >> Perhaps some users prefer that, and/or need that for integration with
>> >> other
>> >> systems/solutions/whatever.
>> >>
>> >> If we drop iptables, how do you suggest to treat upgrades?
>> >>
>> >> >
>> >> > 2 & 3) I am in favor of treating custom services as a requirement and
>> >> > plan
>> >> > accordingly. Many (most, even) of the services are already provided
>> >> > by
>> >> > either firewalld itself (e.g. vdsm, libvirt) or the 3rd party
>> >> > packages
>> >> > (e.g.
>> >> > gluster). Some are missing (I've recently created a pull request for
>> >> > ovirt-imageio to firewalld, for example) and I hope we'll be able to
>> >> > get
>> >> > all
>> >> > the services to be statically provided (by either firewalld or the
>> >> > relevant
>> >> > 3rd party packages).
>> >> >
>> >> > Ideally I think we'd like use statically provided services, and
>> >> > provide
>> >> > the
>> >> > capability to provide additional services (I'm not a fan of the
>> >> > current
>> >> > methodology of converting strings into xmls). I don't think we'd want
>> >> > to
>> >> > limit usage to just statically provided services. (2)
>> >> >
>> >> > As previously stated, I don't see a technical reason to keep iptables
>> >> > under
>> >> > consideration. (3)
>> >> >
>> >> >
>> >> > On Sun, Mar 26, 2017 at 2:57 PM, Yedidyah Bar David 
>> >> > wrote:
>> >> >>
>> >> >>
>> >> >> 1. Do we want to support in some version X both iptables and
>> >> >> firewalld,
>> >> >> or
>> >> >> is it ok to stop support for iptables and support only firewalld
>> >> >> without
>> >> >> overlap? If so, do we handle upgrades, and how?
>> >> >>
>> >> >> 2. Do we want to support custom firewalld xml to be configured on
>> >> >> the
>> >> >> host by us? Or is it ok to only support choosing among existing
>> >> >> services,
>> >> >> which will need to be added to the host using other means (packaged
>> >> >> by
>> >> >> firewalld, packaged by 3rd parties, added manually by users)?
>> >> >>
>> >> >> 3. Opposite of (2.): Do we want to support firewalld services that
>> >> >> are
>> >> >> added to the host using other means (see there)? Obviously we do,
>> >> >> but:
>> >> >> If we do, do we still want to support also iptables (see (1.))? And
>> >> >> if
>> >> >> so, what do we want to then happen?
>> >> >>
>> >> >> (2.) and (3.) are not conflicting, each needs its own answer.
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Didi
>> >> >
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Didi
>> >
>> >
>>
>>
>>
>> --
>> Didi
>
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] REST API data aggregation

[Martin Betak]

On Mon, Mar 27, 2017 at 4:49 PM, Greg Sheremeta  wrote:

> On Mon, Mar 27, 2017 at 8:59 AM, Tomas Jelinek 
> wrote:
>
>>
>>
>> On Mon, Mar 27, 2017 at 1:32 PM, Juan Hernández 
>> wrote:
>>
>>> On 03/27/2017 01:03 PM, Tomas Jelinek wrote:
>>> >
>>> >
>>> > On Mon, Mar 27, 2017 at 11:21 AM, Juan Hernández >> > > wrote:
>>> >
>>> > Top posting, sorry.
>>> >
>>> > There are a few things I'd like to clarify, regarding this subject:
>>> >
>>> > 1. Data aggregation, as requested now by Tomas, and by other
>>> people in
>>> > the past.
>>> >
>>> > We used to have that 'detail' parameter, to aggregate certain very
>>> > specific types of data, in particular to aggregate VM disks and
>>> NICs. We
>>> > removed that in version 4 of the API because the implementation was
>>> > extremely inefficient, from the engine point of view. An innocent
>>> > request like this:
>>> >
>>> >   GET /ovirt-engine/api/vms?detail=+disks,+nics
>>> >
>>> > Would generate, with the implementation we used to have, 1 query
>>> for the
>>> > VMs and then as many queries for disks and NICs as VMs in the
>>> system. In
>>> > our scale test environments, for example, with approx 4000 VMs and
>>> 1
>>> > disks, that would take more than 20 hours to execute.
>>> >
>>> > In addition, we didn't have in the past any mechanism to make this
>>> > available in a generic one, because there was no knowledge in the
>>> API of
>>> > what are 'details'.
>>> >
>>> > In version 4 of the API we introduced a formal (kind of)
>>> specification
>>> > of the API (a.k.a. the model), and int includes knowledge about
>>> what are
>>> > 'links'. For example, the specification of the VM type contains
>>> this:
>>> >
>>> >   @Link DiskAttachment[] diskAttachments();
>>> >   @Link Nic[] nics();
>>> >
>>> > With this information we are now in a position where we can
>>> implement
>>> > this in a generic way.
>>> >
>>> > We intend to implement this using a mechanism similar to the
>>> existing
>>> > 'detail' parameter:
>>> >
>>> >   GET /ovirt-engine/api/vms/123?follow=disk_attachments,nics
>>> >
>>> > The naive implementation of this is to let the API call itself. For
>>> > example, when the user requests to follow the 'disk_attachments'
>>> detail
>>> > the API can just call itself to get that:
>>> >
>>> >   GET /ovirt-engine/api/vms/123/disk_attachments
>>> >
>>> > However, we can't use that naive approach, if we do we end with the
>>> > 1+C*N query problem described before. We need to use specific
>>> > implementations for certain frequent use cases, like
>>> VMs+disks+nics, and
>>> > that needs work in the API and in the backend.
>>> >
>>> > Tomas, if you want to help moving this forward, please open a RFE
>>> and
>>> > makes sure it gets attention.
>>>
>>
>> ok, opened: https://bugzilla.redhat.com/show_bug.cgi?id=1436206
>> Will try to get it done soon.
>>
>
> Forgive me if this is radical, but has anyone thought of / discussed using
> a NoSQL alternative to our very normalized SQL db as a way to avoid the
> problem of aggregating details? Using mongodb as an example, embed some of
> the smaller objects, and there's no cost of aggregation there. IIRC, Doctor
> REST uses mongo under the hood.
>
> https://docs.mongodb.com/manual/tutorial/model-embedded-one-to-many-
> relationships-between-documents/
>

Greg, I really appreciate your enthusiasm for NoSQL technologies but we
have to distinguish between the functional requirements for cache of
schemaless frontend-optimized projections (as in the Dr. Rest case) and the
main application data store, supporting transactional business logic. I
don't believe the current architecture could survive the weak
eventual-consistency guarantees of not fully transactional DB (such as
Postgres is) :-)

Best regards,

Martin


>
>
> Greg
>
>
>>
>>
>>> >
>>> >
>>> > This sounds pretty good! I will open, but since we are talking already
>>> > here I'll just use the opportunity to clarify the topic more and than
>>> > I'll open the BZ.
>>> >
>>> > What I can imagine is the GetAllVmsQuery will accept in params also the
>>> > list of details it should provide. Than, the GetAllVmsQuery will
>>> > implement the efficient way of retrieving this info as well.
>>> >
>>> > So, from the API perspective, it will be about taking the
>>> > ?follow= part and passing it to the backend query params.
>>> >
>>> > What you think?
>>> >
>>>
>>> Exactly, that is the point! The API by itself can't optimize database
>>> queries, all it can do is call the backend. It is the backend that has
>>> the opportunity and possibility to send optimized queries to the
>>> database.
>>>
>>> For other less common things we can use the naive approach, and
>>> implement the aggregation in the API itself. But for 

Re: [ovirt-devel] REST API data aggregation

[Shmuel Melamud]

Hi!

> Forgive me if this is radical, but has anyone thought of / discussed using a
> NoSQL alternative to our very normalized SQL db as a way to avoid the
> problem of aggregating details? Using mongodb as an example, embed some of
> the smaller objects, and there's no cost of aggregation there. IIRC, Doctor
> REST uses mongo under the hood.
>
> https://docs.mongodb.com/manual/tutorial/model-embedded-one-to-many-relationships-between-documents/

What about data integrity that is critical to us?

Shmuel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Writing SQL queries in DAO code

[Yevgeny Zaspitsky]

On Mon, Mar 27, 2017 at 5:30 PM, Yevgeny Zaspitsky 
wrote:

>
>
> On Mon, Mar 27, 2017 at 12:45 PM, Eli Mesika  wrote:
>
>>
>>
>> On Mon, Mar 27, 2017 at 12:26 PM, Yevgeny Zaspitsky 
>> wrote:
>>
>>> > I don't think that looking in SQL in Java code is ​clear than looking
>>> in a SP code
>>>
>>> Looking in SQL isn't the problem I'm trying to solve, but finding that.
>>>
>>> > 1) You will pass more data on the wire instead of calling a SP with
>>> parameters
>>>
>>> Passing the SQL string shouldn't be a problem unless that is very long
>>> one (several K) and then we probably do something wrong.
>>>
>>> > 2) Your data that is passed on the wire is exposed to attacks since
>>> you will have to implement DB security in the engine level (for example
>>> hidden columns).
>>>
>>> IIUC currently querying tables/views with hidden columns is implemented
>>> in a SP that consist of at least 2 SQL's, so those aren't good candidates
>>> for my proposal and will stay as is.
>>> BTW how other projects resolve the security problem? AFAIK usually
>>> hidden columns are hidden by defining a proper view that do not include
>>> those.
>>>
>>
>> ​That's a bad solution as long as your data is passed unmasked on the
>> wire ​
>>
>
> In some cases we do send a sensitive info in the current solution that
> uses SP's.
> Should we use (maybe already) a kind of secured connection to DB?
>
> Database security should be done in the database level and you will not be
>> able to do that without using SPs
>>
>
> I'm not a DB security expert, but from my experience from my previous jobs
> none of them used "SP's only" approach, in fact they didn't use SP's at all.
>
>>
>>
>>
>>>
>>> > 3) Changes in the SQL code done in patches may be more complicated to
>>> track
>>>
>>> Most of SQL code changes involve changes in a DAO too, so this shouldn't
>>> be an issue.
>>>
>> ​It is !!! , changes in DAO are easy to track since it is a Java code ,
>> you are suggesting to write the SQL itself ​
>>
>
> If you need to update a Java code together with SQL, then how much
> difference is in seeing the change in a single file or in two separate ones?
> Frankly speaking I do not get what do you mean by "track". If you're about
> following what a DAO method does, then my approach simplifies the job.
>
>>
>>
>>>
>>> > 4) SQL Injection
>>>
>>> Again, how other projects resolve the security problem? Internet is full
>>> of articles/blogs of why stored procedures should be avoided ("avoid
>>> ​​
>>> stored procedures" Google query returned ~6.6M results), so I guess
>>> there are some other approaches for the security issue if that exists.
>>>
>>
>> ​"use stored procedures" Google query returned About 4,840,000 results ​
>>
>> How many of those add "do not" to the search term? Anyhow avoid gives
> more results...
>

Again, I'm not a DB security expert, but IIUC using prepared statements
with parameters (what my patches do) doesn't allow a SQL injection. On the
other side the existing SearchDao.getAllWithQuery implementors are exposed
to SQL injection attempts. Am I right?


>>
>>>
>>> > 5) I think that SP performs better than SQL inside Java
>>>
>>> Do we have a measurement of how much better?
>>> One of my intentions for this proposal is that people evidently avoid
>>> creating neat SQL queries and re-use the existing ones, which has much
>>> bigger performance impact. I guess that the biggest limit here is how
>>> complicated that procedure is in the engine code.
>>>
>>
>> ​That's why we have code review process and we should nack such ​patches
>> , so you think that if people are not familiar with Java8 syntax for
>> example we should move this code to be performed by a "easier" mechanism ?
>> If people are not doing the right thing , we have gerrit for that, we can
>> comment , nack , whatever to make the code good
>>
>
> In a perfect word you're right, proper reviews should've stop that. But in
> the hard reality of oVirt code base it doesn't happen somehow... My
> proposal bases on the current situation of oVirt code that was created by
> using all Gerrit features.
>
>>
>>
>>
>>>
>>>
>>> On Mon, Mar 27, 2017 at 11:09 AM, Eli Mesika  wrote:
>>>


 On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag 
 wrote:

>
>
> On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky <
> yzasp...@redhat.com> wrote:
>
>> Hi All,
>>
>> Recently I had a task of performance improvement in one of our
>> network related flows and had some hard time following our DAL code and 
>> one
>> of the outcomes of the task was defining a couple of new quite simple, 
>> but
>> neat queries.
>> When I came to coding those new queries I remembered how hard was
>> following the existing DAL code, I decided to make my own new methods
>> clearer. So I created [1] and [2] patches.
>>
>> Everything is quite 

Re: [ovirt-devel] REST API data aggregation

[Greg Sheremeta]

On Mon, Mar 27, 2017 at 8:59 AM, Tomas Jelinek  wrote:

>
>
> On Mon, Mar 27, 2017 at 1:32 PM, Juan Hernández 
> wrote:
>
>> On 03/27/2017 01:03 PM, Tomas Jelinek wrote:
>> >
>> >
>> > On Mon, Mar 27, 2017 at 11:21 AM, Juan Hernández > > > wrote:
>> >
>> > Top posting, sorry.
>> >
>> > There are a few things I'd like to clarify, regarding this subject:
>> >
>> > 1. Data aggregation, as requested now by Tomas, and by other people
>> in
>> > the past.
>> >
>> > We used to have that 'detail' parameter, to aggregate certain very
>> > specific types of data, in particular to aggregate VM disks and
>> NICs. We
>> > removed that in version 4 of the API because the implementation was
>> > extremely inefficient, from the engine point of view. An innocent
>> > request like this:
>> >
>> >   GET /ovirt-engine/api/vms?detail=+disks,+nics
>> >
>> > Would generate, with the implementation we used to have, 1 query
>> for the
>> > VMs and then as many queries for disks and NICs as VMs in the
>> system. In
>> > our scale test environments, for example, with approx 4000 VMs and
>> 1
>> > disks, that would take more than 20 hours to execute.
>> >
>> > In addition, we didn't have in the past any mechanism to make this
>> > available in a generic one, because there was no knowledge in the
>> API of
>> > what are 'details'.
>> >
>> > In version 4 of the API we introduced a formal (kind of)
>> specification
>> > of the API (a.k.a. the model), and int includes knowledge about
>> what are
>> > 'links'. For example, the specification of the VM type contains
>> this:
>> >
>> >   @Link DiskAttachment[] diskAttachments();
>> >   @Link Nic[] nics();
>> >
>> > With this information we are now in a position where we can
>> implement
>> > this in a generic way.
>> >
>> > We intend to implement this using a mechanism similar to the
>> existing
>> > 'detail' parameter:
>> >
>> >   GET /ovirt-engine/api/vms/123?follow=disk_attachments,nics
>> >
>> > The naive implementation of this is to let the API call itself. For
>> > example, when the user requests to follow the 'disk_attachments'
>> detail
>> > the API can just call itself to get that:
>> >
>> >   GET /ovirt-engine/api/vms/123/disk_attachments
>> >
>> > However, we can't use that naive approach, if we do we end with the
>> > 1+C*N query problem described before. We need to use specific
>> > implementations for certain frequent use cases, like
>> VMs+disks+nics, and
>> > that needs work in the API and in the backend.
>> >
>> > Tomas, if you want to help moving this forward, please open a RFE
>> and
>> > makes sure it gets attention.
>>
>
> ok, opened: https://bugzilla.redhat.com/show_bug.cgi?id=1436206
> Will try to get it done soon.
>

Forgive me if this is radical, but has anyone thought of / discussed using
a NoSQL alternative to our very normalized SQL db as a way to avoid the
problem of aggregating details? Using mongodb as an example, embed some of
the smaller objects, and there's no cost of aggregation there. IIRC, Doctor
REST uses mongo under the hood.

https://docs.mongodb.com/manual/tutorial/model-embedded-one-to-many-relationships-between-documents/

Greg


>
>
>> >
>> >
>> > This sounds pretty good! I will open, but since we are talking already
>> > here I'll just use the opportunity to clarify the topic more and than
>> > I'll open the BZ.
>> >
>> > What I can imagine is the GetAllVmsQuery will accept in params also the
>> > list of details it should provide. Than, the GetAllVmsQuery will
>> > implement the efficient way of retrieving this info as well.
>> >
>> > So, from the API perspective, it will be about taking the
>> > ?follow= part and passing it to the backend query params.
>> >
>> > What you think?
>> >
>>
>> Exactly, that is the point! The API by itself can't optimize database
>> queries, all it can do is call the backend. It is the backend that has
>> the opportunity and possibility to send optimized queries to the database.
>>
>> For other less common things we can use the naive approach, and
>> implement the aggregation in the API itself. But for common use cases,
>> like VM+disks+nics, we need to do it in an efficient way.
>>
>> >
>> >
>> > 2. Reuse of TLS sessions.
>> >
>> > The part of creating TLS sessions that is expensive is the
>> generation of
>> > the shared session key. That can be avoided if both the server and
>> the
>> > client are careful and reuse the session, using the session cache
>> > mechanism built-in into TLS itself. The web servers that we use
>> (Apache
>> > and Undertow) do implement this mechanism, and so do most of our
>> > clients. Make sure that your client uses it as well. In Java this is
>> > achieved re-using the SSLContext. We already do that for the engine

Re: [ovirt-devel] Writing SQL queries in DAO code

[Yevgeny Zaspitsky]

On Mon, Mar 27, 2017 at 12:45 PM, Eli Mesika  wrote:

>
>
> On Mon, Mar 27, 2017 at 12:26 PM, Yevgeny Zaspitsky 
> wrote:
>
>> > I don't think that looking in SQL in Java code is ​clear than looking
>> in a SP code
>>
>> Looking in SQL isn't the problem I'm trying to solve, but finding that.
>>
>> > 1) You will pass more data on the wire instead of calling a SP with
>> parameters
>>
>> Passing the SQL string shouldn't be a problem unless that is very long
>> one (several K) and then we probably do something wrong.
>>
>> > 2) Your data that is passed on the wire is exposed to attacks since you
>> will have to implement DB security in the engine level (for example hidden
>> columns).
>>
>> IIUC currently querying tables/views with hidden columns is implemented
>> in a SP that consist of at least 2 SQL's, so those aren't good candidates
>> for my proposal and will stay as is.
>> BTW how other projects resolve the security problem? AFAIK usually hidden
>> columns are hidden by defining a proper view that do not include those.
>>
>
> ​That's a bad solution as long as your data is passed unmasked on the wire
> ​
>

In some cases we do send a sensitive info in the current solution that uses
SP's.
Should we use (maybe already) a kind of secured connection to DB?

Database security should be done in the database level and you will not be
> able to do that without using SPs
>

I'm not a DB security expert, but from my experience from my previous jobs
none of them used "SP's only" approach, in fact they didn't use SP's at all.

>
>
>
>>
>> > 3) Changes in the SQL code done in patches may be more complicated to
>> track
>>
>> Most of SQL code changes involve changes in a DAO too, so this shouldn't
>> be an issue.
>>
> ​It is !!! , changes in DAO are easy to track since it is a Java code ,
> you are suggesting to write the SQL itself ​
>

If you need to update a Java code together with SQL, then how much
difference is in seeing the change in a single file or in two separate ones?
Frankly speaking I do not get what do you mean by "track". If you're about
following what a DAO method does, then my approach simplifies the job.

>
>
>>
>> > 4) SQL Injection
>>
>> Again, how other projects resolve the security problem? Internet is full
>> of articles/blogs of why stored procedures should be avoided ("avoid
>> ​​
>> stored procedures" Google query returned ~6.6M results), so I guess there
>> are some other approaches for the security issue if that exists.
>>
>
> ​"use stored procedures" Google query returned About 4,840,000 results ​
>
> How many of those add "do not" to the search term? Anyhow avoid gives more
results...

>
>
>>
>> > 5) I think that SP performs better than SQL inside Java
>>
>> Do we have a measurement of how much better?
>> One of my intentions for this proposal is that people evidently avoid
>> creating neat SQL queries and re-use the existing ones, which has much
>> bigger performance impact. I guess that the biggest limit here is how
>> complicated that procedure is in the engine code.
>>
>
> ​That's why we have code review process and we should nack such ​patches ,
> so you think that if people are not familiar with Java8 syntax for example
> we should move this code to be performed by a "easier" mechanism ?
> If people are not doing the right thing , we have gerrit for that, we can
> comment , nack , whatever to make the code good
>

In a perfect word you're right, proper reviews should've stop that. But in
the hard reality of oVirt code base it doesn't happen somehow... My
proposal bases on the current situation of oVirt code that was created by
using all Gerrit features.

>
>
>
>>
>>
>> On Mon, Mar 27, 2017 at 11:09 AM, Eli Mesika  wrote:
>>
>>>
>>>
>>> On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag  wrote:
>>>


 On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky  wrote:

> Hi All,
>
> Recently I had a task of performance improvement in one of our network
> related flows and had some hard time following our DAL code and one of the
> outcomes of the task was defining a couple of new quite simple, but neat
> queries.
> When I came to coding those new queries I remembered how hard was
> following the existing DAL code, I decided to make my own new methods
> clearer. So I created [1] and [2] patches.
>
> Everything is quite standard there beside the fact that they do not
> use any stored procedure, but use SQL directly, IMHO by that they save 
> time
> that I spent in trying to follow what a DAO method does. Looking into the
> method code you get the understanding of what this method is all about:
>
>- no looking for a stored procedure name that is buried in the DAO
>class hierarchy
>- no looking for the SP definition
>
>
 There are additional pros and cons for the suggestion to 

Re: [ovirt-devel] Firewalld migration.

[Leon Goldberg]

You're right, but I don't think it matters; hosts will remain unaffected
until they're reinstalled via an upgraded Engine.

Clients should be made aware their custom rules are going to be obsolete
and that they should reapply them once they reinstall.

On Mon, Mar 27, 2017 at 9:01 AM, Yedidyah Bar David  wrote:

> On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 
> wrote:
> > Effectively, upgrading will leave lingering (but nonetheless operational)
> > iptables rules on the hosts. I'm not even sure there needs to be special
> > upgrade treatment?
>
> Please describe the expected flow.
>
> Please note that at least when I tried, 'systemctl start firewalld' stops
> iptables.
>
> Thanks,
>
> >
> > On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David 
> wrote:
> >>
> >> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
> >> wrote:
> >> > 1) Do we actually need iptables for any reason that isn't a legacy
> >> > consideration?
> >>
> >> No idea personally.
> >>
> >> Perhaps some users prefer that, and/or need that for integration with
> >> other
> >> systems/solutions/whatever.
> >>
> >> If we drop iptables, how do you suggest to treat upgrades?
> >>
> >> >
> >> > 2 & 3) I am in favor of treating custom services as a requirement and
> >> > plan
> >> > accordingly. Many (most, even) of the services are already provided by
> >> > either firewalld itself (e.g. vdsm, libvirt) or the 3rd party packages
> >> > (e.g.
> >> > gluster). Some are missing (I've recently created a pull request for
> >> > ovirt-imageio to firewalld, for example) and I hope we'll be able to
> get
> >> > all
> >> > the services to be statically provided (by either firewalld or the
> >> > relevant
> >> > 3rd party packages).
> >> >
> >> > Ideally I think we'd like use statically provided services, and
> provide
> >> > the
> >> > capability to provide additional services (I'm not a fan of the
> current
> >> > methodology of converting strings into xmls). I don't think we'd want
> to
> >> > limit usage to just statically provided services. (2)
> >> >
> >> > As previously stated, I don't see a technical reason to keep iptables
> >> > under
> >> > consideration. (3)
> >> >
> >> >
> >> > On Sun, Mar 26, 2017 at 2:57 PM, Yedidyah Bar David 
> >> > wrote:
> >> >>
> >> >>
> >> >> 1. Do we want to support in some version X both iptables and
> firewalld,
> >> >> or
> >> >> is it ok to stop support for iptables and support only firewalld
> >> >> without
> >> >> overlap? If so, do we handle upgrades, and how?
> >> >>
> >> >> 2. Do we want to support custom firewalld xml to be configured on the
> >> >> host by us? Or is it ok to only support choosing among existing
> >> >> services,
> >> >> which will need to be added to the host using other means (packaged
> by
> >> >> firewalld, packaged by 3rd parties, added manually by users)?
> >> >>
> >> >> 3. Opposite of (2.): Do we want to support firewalld services that
> are
> >> >> added to the host using other means (see there)? Obviously we do,
> but:
> >> >> If we do, do we still want to support also iptables (see (1.))? And
> if
> >> >> so, what do we want to then happen?
> >> >>
> >> >> (2.) and (3.) are not conflicting, each needs its own answer.
> >> >>
> >> >>
> >> >> --
> >> >> Didi
> >> >
> >> >
> >>
> >>
> >>
> >> --
> >> Didi
> >
> >
>
>
>
> --
> Didi
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] REST API data aggregation

[Tomas Jelinek]

On Mon, Mar 27, 2017 at 1:32 PM, Juan Hernández  wrote:

> On 03/27/2017 01:03 PM, Tomas Jelinek wrote:
> >
> >
> > On Mon, Mar 27, 2017 at 11:21 AM, Juan Hernández  > > wrote:
> >
> > Top posting, sorry.
> >
> > There are a few things I'd like to clarify, regarding this subject:
> >
> > 1. Data aggregation, as requested now by Tomas, and by other people
> in
> > the past.
> >
> > We used to have that 'detail' parameter, to aggregate certain very
> > specific types of data, in particular to aggregate VM disks and
> NICs. We
> > removed that in version 4 of the API because the implementation was
> > extremely inefficient, from the engine point of view. An innocent
> > request like this:
> >
> >   GET /ovirt-engine/api/vms?detail=+disks,+nics
> >
> > Would generate, with the implementation we used to have, 1 query for
> the
> > VMs and then as many queries for disks and NICs as VMs in the
> system. In
> > our scale test environments, for example, with approx 4000 VMs and
> 1
> > disks, that would take more than 20 hours to execute.
> >
> > In addition, we didn't have in the past any mechanism to make this
> > available in a generic one, because there was no knowledge in the
> API of
> > what are 'details'.
> >
> > In version 4 of the API we introduced a formal (kind of)
> specification
> > of the API (a.k.a. the model), and int includes knowledge about what
> are
> > 'links'. For example, the specification of the VM type contains this:
> >
> >   @Link DiskAttachment[] diskAttachments();
> >   @Link Nic[] nics();
> >
> > With this information we are now in a position where we can implement
> > this in a generic way.
> >
> > We intend to implement this using a mechanism similar to the existing
> > 'detail' parameter:
> >
> >   GET /ovirt-engine/api/vms/123?follow=disk_attachments,nics
> >
> > The naive implementation of this is to let the API call itself. For
> > example, when the user requests to follow the 'disk_attachments'
> detail
> > the API can just call itself to get that:
> >
> >   GET /ovirt-engine/api/vms/123/disk_attachments
> >
> > However, we can't use that naive approach, if we do we end with the
> > 1+C*N query problem described before. We need to use specific
> > implementations for certain frequent use cases, like VMs+disks+nics,
> and
> > that needs work in the API and in the backend.
> >
> > Tomas, if you want to help moving this forward, please open a RFE and
> > makes sure it gets attention.
>

ok, opened: https://bugzilla.redhat.com/show_bug.cgi?id=1436206
Will try to get it done soon.


> >
> >
> > This sounds pretty good! I will open, but since we are talking already
> > here I'll just use the opportunity to clarify the topic more and than
> > I'll open the BZ.
> >
> > What I can imagine is the GetAllVmsQuery will accept in params also the
> > list of details it should provide. Than, the GetAllVmsQuery will
> > implement the efficient way of retrieving this info as well.
> >
> > So, from the API perspective, it will be about taking the
> > ?follow= part and passing it to the backend query params.
> >
> > What you think?
> >
>
> Exactly, that is the point! The API by itself can't optimize database
> queries, all it can do is call the backend. It is the backend that has
> the opportunity and possibility to send optimized queries to the database.
>
> For other less common things we can use the naive approach, and
> implement the aggregation in the API itself. But for common use cases,
> like VM+disks+nics, we need to do it in an efficient way.
>
> >
> >
> > 2. Reuse of TLS sessions.
> >
> > The part of creating TLS sessions that is expensive is the
> generation of
> > the shared session key. That can be avoided if both the server and
> the
> > client are careful and reuse the session, using the session cache
> > mechanism built-in into TLS itself. The web servers that we use
> (Apache
> > and Undertow) do implement this mechanism, and so do most of our
> > clients. Make sure that your client uses it as well. In Java this is
> > achieved re-using the SSLContext. We already do that for the engine
> to
> > VDSM communciation for example. In JavaScript the browser already
> takes
> > care of this.
> >
> > 3. Parallelism and latency.
> >
> > A typical problem that we have is that we send many request to the
> > server. For example, to retrieve user sessions for a set of VMs we
> tend
> > to send many requests like this:
> >
> >   GET /ovirt-engine/api/vms/1/sessions
> >   GET /ovirt/engine/api/vms/2/sessions
> >   GET /ovirt-engine/api/vms/3/sessions
> >   ...
> >
> > And we do that in a synchronous way: send one, wait for the result,
> send
> > another one, wait for the result, etc. This means 

Re: [ovirt-devel] oVirt Engine checkstyle upgrade

[Allon Mureinik]

On Mon, Mar 27, 2017 at 3:09 PM, Allon Mureinik  wrote:

> Indeed, Greg.thanks
>
> As a temporary solution, you could cherry-pick https://gerrit.
> ovirt.org/#/c/74611 and work above it.
> It should solve the issue (even though IMHO it isn't ready for merging to
> master).
>
> Vojta - are you updating https://gerrit.ovirt.org/#/c/74611 as per my
> comments there, or should I take it over?
>
I literally have no idea who I did made this typo.
I meant "Vojtech", of course.​



>
> On Mon, Mar 27, 2017 at 3:03 PM, Greg Sheremeta 
> wrote:
>
>> From the comments in 74619
>>
>> "" "
>> So, tl;dr - it /won't/ work with this patch but without 74611. This patch
>> should be applied before 74611
>> "" "
>>
>> On Mon, Mar 27, 2017 at 7:42 AM, Marek Libra  wrote:
>>
>>> With
>>>   https://gerrit.ovirt.org/#/c/74619/
>>>
>>> applied, following is still failing:
>>>   make gwt-debug DEBUG_MODULE=webadmin DEV_EXTRA_BUILD_FLAGS_GWT_DEFA
>>> ULTS="-Dgwt.cssResourceStyle=pretty -Dgwt.userAgent=gecko1_8"
>>>
>>> with message:
>>> [WARNING] The requested profile "gwt-user" could not be activated
>>> because it does not exist.
>>> [ERROR] Failed to execute goal 
>>> org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check
>>> (checkstyle) on project webadmin: Failed during checkstyle configuration:
>>> Exception was thrown while processing /home/mlibra/IdeaProjects/ovir
>>> t-engine/frontend/webadmin/modules/gwt-extension/src/main/
>>> java/org/ovirt/engine/ui/uioverrides/org/slf4j/Logger.java: can't parse
>>> argument number: For input string: "" -> [Help 1]
>>>
>>>
>>> Any hint, please?
>>> Marek
>>>
>>>
>>>
>>> On Fri, Mar 24, 2017 at 4:14 PM, Vojtech Szocs 
>>> wrote:
>>>


 On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs 
 wrote:

> Found the problem after debugging NlsCheck.
>
> First of all, it checks all kinds of Java sources, including the
> generated ones. That's silly and one of the reasons why Checkstyle
> execution takes a rather long time. I'll fix that.
>
> Next, when checking a Java source that contains string "{}" (without
> quotes) it will log the problem, but Checkstyle message logging infra
> things that "{}" is a placeholder to resolve, but it's not, and it fails 
> on
> NumberFormatException. I'll fix that too.
>

 ​https://gerrit.ovirt.org/#/c/74611/​


> Vojtech
>
>
> On Fri, Mar 24, 2017 at 3:19 PM, Vojtech Szocs 
> wrote:
>
>> Hi Allon,
>>
>> I think I found some strange Checkstyle related problems on master.
>>
>> Engine build with (GWT compilation enabled) works OK.
>>
>> Next, trying to start GWT debugger:
>>
>> $ make gwt-debug DEBUG_MODULE=webadmin \
>>   DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.userAgent=gecko1_8,safari"
>> \
>>   DEV_EXTRA_BUILD_FLAGS="-Dgwt.logLevel=INFO -Dgwt.locale=en_US
>> -Dgwt.compiler.localWorkers=1" \
>>   DEV_BUILD_GWT_SUPER_DEV_MODE=1
>>
>> maven-checkstyle-plugin:check execution fails on
>>
>>   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
>> irt/engine/ui/uioverrides/org/ovirt/engine/core/compat/Forma
>> tterDotnet.java
>>   can't parse argument number: (\\d)\\: For input string: "(\\d)\\"
>>
>> the class isn't used, removed it, retry. Now it fails on:
>>
>>   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
>> irt/engine/ui/uioverrides/org/slf4j/Logger.java
>>   can't parse argument number: For input string: ""
>>
>> I guess it's a bug in our NON-NLS check? But why doesn't the problem
>> occur during Engine build?
>>
>> I'm thinking about disabling Checkstyle for gwt-extension module, as
>> it contains custom GWT RPC serializers and GWT class overrides, and maybe
>> the file path src/main/java/org/ovirt/engine
>> /ui/uioverrides/here/goes/actual/pkg is confusing the Checkstyle now.
>>
>> Thanks,
>> Vojtech
>>
>>
>> On Wed, Mar 22, 2017 at 10:33 PM, Allon Mureinik > > wrote:
>>
>>> Hi all,
>>>
>>> As per [1], I've just merged a series of patches that upgrades the
>>> oVirt engine to use the latest maven-checkstyle-plugin and checkstyle
>>> packages.
>>>
>>> Please note that the newer checkstyle is a tad stricter than the old
>>> one we used to have (read: it contains several fixes for bugs where the 
>>> old
>>> checkstyle was supposed to find issues but missed them).
>>> I also took the opportunity and added a couple of new checks that
>>> enforce rules we were de-facto adhering to anyway.
>>>
>>> If any problems come up, please let me know.
>>>
>>>
>>> -Your friendly neighborhood cleanup dude
>>>
>>> [1] 

Re: [ovirt-devel] oVirt Engine checkstyle upgrade

[Greg Sheremeta]

>From the comments in 74619

"""
So, tl;dr - it /won't/ work with this patch but without 74611. This patch
should be applied before 74611
"""

On Mon, Mar 27, 2017 at 7:42 AM, Marek Libra  wrote:

> With
>   https://gerrit.ovirt.org/#/c/74619/
>
> applied, following is still failing:
>   make gwt-debug DEBUG_MODULE=webadmin DEV_EXTRA_BUILD_FLAGS_GWT_
> DEFAULTS="-Dgwt.cssResourceStyle=pretty -Dgwt.userAgent=gecko1_8"
>
> with message:
> [WARNING] The requested profile "gwt-user" could not be activated because
> it does not exist.
> [ERROR] Failed to execute goal org.apache.maven.plugins:
> maven-checkstyle-plugin:2.17:check (checkstyle) on project webadmin:
> Failed during checkstyle configuration: Exception was thrown while
> processing /home/mlibra/IdeaProjects/ovirt-engine/frontend/
> webadmin/modules/gwt-extension/src/main/java/org/
> ovirt/engine/ui/uioverrides/org/slf4j/Logger.java: can't parse argument
> number: For input string: "" -> [Help 1]
>
>
> Any hint, please?
> Marek
>
>
>
> On Fri, Mar 24, 2017 at 4:14 PM, Vojtech Szocs  wrote:
>
>>
>>
>> On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs  wrote:
>>
>>> Found the problem after debugging NlsCheck.
>>>
>>> First of all, it checks all kinds of Java sources, including the
>>> generated ones. That's silly and one of the reasons why Checkstyle
>>> execution takes a rather long time. I'll fix that.
>>>
>>> Next, when checking a Java source that contains string "{}" (without
>>> quotes) it will log the problem, but Checkstyle message logging infra
>>> things that "{}" is a placeholder to resolve, but it's not, and it fails on
>>> NumberFormatException. I'll fix that too.
>>>
>>
>> ​https://gerrit.ovirt.org/#/c/74611/​
>>
>>
>>> Vojtech
>>>
>>>
>>> On Fri, Mar 24, 2017 at 3:19 PM, Vojtech Szocs 
>>> wrote:
>>>
 Hi Allon,

 I think I found some strange Checkstyle related problems on master.

 Engine build with (GWT compilation enabled) works OK.

 Next, trying to start GWT debugger:

 $ make gwt-debug DEBUG_MODULE=webadmin \
   DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.userAgent=gecko1_8,safari"
 \
   DEV_EXTRA_BUILD_FLAGS="-Dgwt.logLevel=INFO -Dgwt.locale=en_US
 -Dgwt.compiler.localWorkers=1" \
   DEV_BUILD_GWT_SUPER_DEV_MODE=1

 maven-checkstyle-plugin:check execution fails on

   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
 irt/engine/ui/uioverrides/org/ovirt/engine/core/compat/Forma
 tterDotnet.java
   can't parse argument number: (\\d)\\: For input string: "(\\d)\\"

 the class isn't used, removed it, retry. Now it fails on:

   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
 irt/engine/ui/uioverrides/org/slf4j/Logger.java
   can't parse argument number: For input string: ""

 I guess it's a bug in our NON-NLS check? But why doesn't the problem
 occur during Engine build?

 I'm thinking about disabling Checkstyle for gwt-extension module, as it
 contains custom GWT RPC serializers and GWT class overrides, and maybe the
 file path 
 src/main/java/org/ovirt/engine/ui/uioverrides/here/goes/actual/pkg
 is confusing the Checkstyle now.

 Thanks,
 Vojtech


 On Wed, Mar 22, 2017 at 10:33 PM, Allon Mureinik 
 wrote:

> Hi all,
>
> As per [1], I've just merged a series of patches that upgrades the
> oVirt engine to use the latest maven-checkstyle-plugin and checkstyle
> packages.
>
> Please note that the newer checkstyle is a tad stricter than the old
> one we used to have (read: it contains several fixes for bugs where the 
> old
> checkstyle was supposed to find issues but missed them).
> I also took the opportunity and added a couple of new checks that
> enforce rules we were de-facto adhering to anyway.
>
> If any problems come up, please let me know.
>
>
> -Your friendly neighborhood cleanup dude
>
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1433408
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>


>>>
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>



-- 
Greg Sheremeta, MBA
Red Hat, Inc.
Sr. Software Engineer
gsher...@redhat.com
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [ OST Failure Report ] [ oVirt 4.1 ] [ 21-03-2017 ] [ test-repo_ovirt_experimental_4.1 ]

[Piotr Kliczewski]

Pavel,

Is is not network issue but known issue [1] fixed on master and we
waited for some time to make sure that the fix is stable.
It looks like it is a time to backport.

Thanks,
Piotr

[1] https://gerrit.ovirt.org/#/c/73745

On Mon, Mar 27, 2017 at 1:37 PM, Pavel Zhukov  wrote:
> Hi,
> Still
> reproducible:http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1077/
> @Dan too many "network" keywords there can your team take a look please?
>
> --
> Pavel
>
>
> On Tue, Mar 21, 2017 at 1:12 PM, Shlomo Ben David 
> wrote:
>>
>> Hi,
>>
>> Test failed: [ test-repo_ovirt_experimental_4.1 ]
>> Link to suspected patches: N/A
>> Link to Job:
>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1018
>> Link to all logs:
>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1018/artifact/exported-artifacts/basic-suit-4.1-el7/test_logs/basic-suite-4.1/post-004_basic_sanity.py/
>>
>> Error snippet from the log:
>> 
>>
>> ifup/VLAN100_Network::DEBUG::2017-03-21
>> 06:12:03,550::commands::93::root::(execCmd) FAILED:  = 'Running scope
>> as unit
>> 979f3d61-c1f9-49c2-b168-799b882f64d5.scope.\n/etc/sysconfig/network-scripts/ifup-eth:
>> line 297: 30633 Terminated  /sbin/dhclient ${DHCLIENTARGS}
>> ${DEVICE}\nCannot find device "VLAN100_Network"\nDevice "VLAN100_Network"
>> does not exist.\nDevice "VLAN100_Network" does not exist.\nDevice
>> ...
>> ...
>> ...
>> "VLAN100_Network" does not exist.\n';  = 1
>> ifup/VLAN100_Network::ERROR::2017-03-21
>> 06:12:03,551::utils::371::root::(wrapper) Unhandled exception
>> Traceback (most recent call last):
>>   File "/usr/lib/python2.7/site-packages/vdsm/utils.py", line 368, in
>> wrapper
>> return f(*a, **kw)
>>   File "/usr/lib/python2.7/site-packages/vdsm/concurrent.py", line 180, in
>> run
>> return func(*args, **kwargs)
>>   File
>> "/usr/lib/python2.7/site-packages/vdsm/network/configurators/ifcfg.py", line
>> 924, in _exec_ifup
>> _exec_ifup_by_name(iface.name, cgroup)
>>   File
>> "/usr/lib/python2.7/site-packages/vdsm/network/configurators/ifcfg.py", line
>> 910, in _exec_ifup_by_name
>> raise ConfigNetworkError(ERR_FAILED_IFUP, out[-1] if out else '')
>> ConfigNetworkError: (29, 'Determining IPv6 information for
>> VLAN100_Network... failed.')
>>
>> 
>>
>>
>> Best Regards,
>>
>> Shlomi Ben-David | Software Engineer | Red Hat ISRAEL
>> RHCSA | RHCVA | RHCE
>> IRC: shlomibendavid (on #rhev-integ, #rhev-dev, #rhev-ci)
>>
>> OPEN SOURCE - 1 4 011 && 011 4 1
>>
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>
>
>
>
> --
> Pavel Zhukov
> Software Engineer
> RHEV Devops
> IRC: landgraf
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] oVirt Engine checkstyle upgrade

[Marek Libra]

With
  https://gerrit.ovirt.org/#/c/74619/

applied, following is still failing:
  make gwt-debug DEBUG_MODULE=webadmin
DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.cssResourceStyle=pretty
-Dgwt.userAgent=gecko1_8"

with message:
[WARNING] The requested profile "gwt-user" could not be activated because
it does not exist.
[ERROR] Failed to execute goal
org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check (checkstyle) on
project webadmin: Failed during checkstyle configuration: Exception was
thrown while processing
/home/mlibra/IdeaProjects/ovirt-engine/frontend/webadmin/modules/gwt-extension/src/main/java/org/ovirt/engine/ui/uioverrides/org/slf4j/Logger.java:
can't parse argument number: For input string: "" -> [Help 1]


Any hint, please?
Marek



On Fri, Mar 24, 2017 at 4:14 PM, Vojtech Szocs  wrote:

>
>
> On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs  wrote:
>
>> Found the problem after debugging NlsCheck.
>>
>> First of all, it checks all kinds of Java sources, including the
>> generated ones. That's silly and one of the reasons why Checkstyle
>> execution takes a rather long time. I'll fix that.
>>
>> Next, when checking a Java source that contains string "{}" (without
>> quotes) it will log the problem, but Checkstyle message logging infra
>> things that "{}" is a placeholder to resolve, but it's not, and it fails on
>> NumberFormatException. I'll fix that too.
>>
>
> ​https://gerrit.ovirt.org/#/c/74611/​
>
>
>> Vojtech
>>
>>
>> On Fri, Mar 24, 2017 at 3:19 PM, Vojtech Szocs  wrote:
>>
>>> Hi Allon,
>>>
>>> I think I found some strange Checkstyle related problems on master.
>>>
>>> Engine build with (GWT compilation enabled) works OK.
>>>
>>> Next, trying to start GWT debugger:
>>>
>>> $ make gwt-debug DEBUG_MODULE=webadmin \
>>>   DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.userAgent=gecko1_8,safari" \
>>>   DEV_EXTRA_BUILD_FLAGS="-Dgwt.logLevel=INFO -Dgwt.locale=en_US
>>> -Dgwt.compiler.localWorkers=1" \
>>>   DEV_BUILD_GWT_SUPER_DEV_MODE=1
>>>
>>> maven-checkstyle-plugin:check execution fails on
>>>
>>>   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
>>> irt/engine/ui/uioverrides/org/ovirt/engine/core/compat/Forma
>>> tterDotnet.java
>>>   can't parse argument number: (\\d)\\: For input string: "(\\d)\\"
>>>
>>> the class isn't used, removed it, retry. Now it fails on:
>>>
>>>   frontend/webadmin/modules/gwt-extension/src/main/java/org/ov
>>> irt/engine/ui/uioverrides/org/slf4j/Logger.java
>>>   can't parse argument number: For input string: ""
>>>
>>> I guess it's a bug in our NON-NLS check? But why doesn't the problem
>>> occur during Engine build?
>>>
>>> I'm thinking about disabling Checkstyle for gwt-extension module, as it
>>> contains custom GWT RPC serializers and GWT class overrides, and maybe the
>>> file path src/main/java/org/ovirt/engine/ui/uioverrides/here/goes/actual/pkg
>>> is confusing the Checkstyle now.
>>>
>>> Thanks,
>>> Vojtech
>>>
>>>
>>> On Wed, Mar 22, 2017 at 10:33 PM, Allon Mureinik 
>>> wrote:
>>>
 Hi all,

 As per [1], I've just merged a series of patches that upgrades the
 oVirt engine to use the latest maven-checkstyle-plugin and checkstyle
 packages.

 Please note that the newer checkstyle is a tad stricter than the old
 one we used to have (read: it contains several fixes for bugs where the old
 checkstyle was supposed to find issues but missed them).
 I also took the opportunity and added a couple of new checks that
 enforce rules we were de-facto adhering to anyway.

 If any problems come up, please let me know.


 -Your friendly neighborhood cleanup dude

 [1] https://bugzilla.redhat.com/show_bug.cgi?id=1433408

 ___
 Devel mailing list
 Devel@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/devel

>>>
>>>
>>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Firewalld migration.

[Yaniv Dary]

Hi,
I like option number 3 the most from reading the comments.
We are not a configuration management system, so custom rules should not be
a main consideration.
VDSM should probably enable the firewalld rules it needs to work and engine
should not care about firewall at all.
A user wanting to add more rules then this should probably use the ones
planned in:
https://github.com/cockpit-project/system-api-roles


Thanks,


Yaniv Dary
Technical Product Manager
Red Hat Israel Ltd.
34 Jerusalem Road
Building A, 4th floor
Ra'anana, Israel 4350109

Tel : +972 (9) 7692306
8272306
Email: yd...@redhat.com
IRC : ydary


On Mon, Mar 27, 2017 at 2:10 PM, Martin Sivak  wrote:

> > Right, so why not create an Ansible playbook which the users can change
> > (extend?), based on http://docs.ansible.com/
> ansible/firewalld_module.html ?
>
> I think I like the playbook proposal the best.
>
> Lets assume the engine provides a firewall.yaml playbook somewhere in /etc:
>
>  - The default playbook would contain our default firewalld
> configuration (we should also provide it in /usr/share/doc to give the
> user a reference)
>  - The engine or host deploy script can provide ansible variables so
> the playbook can even be a parametrized one (port numbers)
>  - The user can add rules he needs
>  - The user can REPLACE the content with iptables rules if he wishes
> so (we can even say it is unsupported, but possible)
>
> As an extension:
>
>  - We can provide ovirt-engine-firewalld ansible role with the default
> config so we can properly update files using RPM. The user defined (or
> default) playbook would just call this role and would stay intact
> during package upgrades.
>
> This is outside of database, allows customization and adapts to
> whatever firewall backend we might need.
>
> --
> Martin Sivak
> SLA / oVirt
>
> On Mon, Mar 27, 2017 at 12:46 PM, Yaniv Kaul  wrote:
> >
> >
> > On Mon, Mar 27, 2017 at 1:20 PM, Martin Perina 
> wrote:
> >>
> >>
> >>
> >> On Mon, Mar 27, 2017 at 12:00 PM, Yaniv Kaul  wrote:
> >>>
> >>>
> >>>
> >>> On Mon, Mar 27, 2017 at 11:55 AM, Martin Perina 
> >>> wrote:
> 
>  Hi,
> 
>  so personally I don't like the current way how we store firewall
>  configuration within engine (saving complete iptables commands as
> string). I
>  think should change the way how we store firewall configuration:
> 
>  1. On engine side I'd just store which services/ports (or port ranges)
>  need to be enabled on host. By default only those services/ports that
> engine
>  needs, but we can maintain also custom services defined by users
> >>>
> >>>
> >>> Agreed. I hope that's enough on one hand, on the other hand, users can
> >>> probably easily extend it via Ansible to the hosts and execution of a
> more
> >>> customized firewalld configuration there - we probably should not own
> it.
> >>
> >>
> >> Right, we should take care only about services/ports which we need. So
> we
> >> probably could drop the ability for users to define their own custom
> >> services/ports within engine for firewalld and force them to use
> Ansible or
> >> other tools to handle their own configuration.
> >
> >
> > Right, so why not create an Ansible playbook which the users can change
> > (extend?), based on http://docs.ansible.com/
> ansible/firewalld_module.html ?
> > ...
> >>
> >>
> >>>
> 
> 
>  2. Write plugin to ovirt-host-deploy which will translate those
>  services/ports into actual firewall configuration on the host (it
> should
>  detected what firewall is currently enabled and adapt)
> >
> >
> > ...  and then ovirt-host-deploy will be in charge of executing that
> > playbook? Seems fairly straightforward.
> > Y.
> >
> >>>
> >>> Agreed.
> >>>
> 
> 
>  3. For newly installed host I'd just use firewalld
> >>>
> >>>
> >>> Agreed.
> >>>
> 
> 
>  4. Also for 4.2 clusters I'd switch from iptables to firewalld when
> you
>  execute Reinstall (we should document this and make firewalld
> preferred
>  solution)
> >>>
> >>>
> >>> That's a good question. If a user had the default, non-changed policy
> we
> >>> have had in iptables - sure.
> >>> If not, I think it may be a bit of a challenge to switch otherwise.
> >>
> >>
> >> Right. We could detect if there's some custom firewall rules in
> >> IPTablesConfigSiteCustom engine-config option and if not we could
> probably
> >> assume that switching to firewalld could be performed.
> >>
> >> We could also mark iptables configuration as deprecated in 4.2 and
> declare
> >> that it will be removed in 4.3. That would add some time for users to
> >> prepare for the switch ...
> >>
> >>> Y.
> >>>
> 
> 
> 
> 
>  Martin
> 
> 
> 
>  On Mon, Mar 27, 2017 at 8:01 AM, Yedidyah Bar David 
>  wrote:
> >
> > On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 

Re: [ovirt-devel] [ OST Failure Report ] [ oVirt 4.1 ] [ 21-03-2017 ] [ test-repo_ovirt_experimental_4.1 ]

[Pavel Zhukov]

Hi,
Still reproducible:
http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1077/
@Dan too many "network" keywords there can your team take a look please?

--
Pavel


On Tue, Mar 21, 2017 at 1:12 PM, Shlomo Ben David 
wrote:

> Hi,
>
> Test failed: [ test-repo_ovirt_experimental_4.1 ]
> Link to suspected patches: N/A
> Link to Job: http://jenkins.ovirt.org/job/test-repo_ovirt_
> experimental_4.1/1018
> Link to all logs: http://jenkins.ovirt.org/job/test-repo_ovirt_
> experimental_4.1/1018/artifact/exported-artifacts/
> basic-suit-4.1-el7/test_logs/basic-suite-4.1/post-004_basic_sanity.py/
>
> Error snippet from the log:
> 
>
> ifup/VLAN100_Network::DEBUG::2017-03-21 
> 06:12:03,550::commands::93::root::(execCmd)
> FAILED:  = 'Running scope as unit 979f3d61-c1f9-49c2-b168-
> 799b882f64d5.scope.\n/etc/sysconfig/network-scripts/ifup-eth: line 297:
> 30633 Terminated  /sbin/dhclient ${DHCLIENTARGS}
> ${DEVICE}\nCannot find device "VLAN100_Network"\nDevice "VLAN100_Network"
> does not exist.\nDevice "VLAN100_Network" does not exist.\nDevice
> ...
> ...
> ...
> "VLAN100_Network" does not exist.\n';  = 1
> ifup/VLAN100_Network::ERROR::2017-03-21 
> 06:12:03,551::utils::371::root::(wrapper)
> Unhandled exception
> Traceback (most recent call last):
>   File "/usr/lib/python2.7/site-packages/vdsm/utils.py", line 368, in
> wrapper
> return f(*a, **kw)
>   File "/usr/lib/python2.7/site-packages/vdsm/concurrent.py", line 180,
> in run
> return func(*args, **kwargs)
>   File "/usr/lib/python2.7/site-packages/vdsm/network/configurators/ifcfg.py",
> line 924, in _exec_ifup
> _exec_ifup_by_name(iface.name, cgroup)
>   File "/usr/lib/python2.7/site-packages/vdsm/network/configurators/ifcfg.py",
> line 910, in _exec_ifup_by_name
> raise ConfigNetworkError(ERR_FAILED_IFUP, out[-1] if out else '')
> ConfigNetworkError: (29, 'Determining IPv6 information for
> VLAN100_Network... failed.')
>
> 
>
>
> Best Regards,
>
> Shlomi Ben-David | Software Engineer | Red Hat ISRAEL
> RHCSA | RHCVA | RHCE
> IRC: shlomibendavid (on #rhev-integ, #rhev-dev, #rhev-ci)
>
> OPEN SOURCE - 1 4 011 && 011 4 1
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>



-- 
Pavel Zhukov
Software Engineer
RHEV Devops
IRC: landgraf
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] REST API data aggregation

[Juan Hernández]

On 03/27/2017 01:03 PM, Tomas Jelinek wrote:
> 
> 
> On Mon, Mar 27, 2017 at 11:21 AM, Juan Hernández  > wrote:
> 
> Top posting, sorry.
> 
> There are a few things I'd like to clarify, regarding this subject:
> 
> 1. Data aggregation, as requested now by Tomas, and by other people in
> the past.
> 
> We used to have that 'detail' parameter, to aggregate certain very
> specific types of data, in particular to aggregate VM disks and NICs. We
> removed that in version 4 of the API because the implementation was
> extremely inefficient, from the engine point of view. An innocent
> request like this:
> 
>   GET /ovirt-engine/api/vms?detail=+disks,+nics
> 
> Would generate, with the implementation we used to have, 1 query for the
> VMs and then as many queries for disks and NICs as VMs in the system. In
> our scale test environments, for example, with approx 4000 VMs and 1
> disks, that would take more than 20 hours to execute.
> 
> In addition, we didn't have in the past any mechanism to make this
> available in a generic one, because there was no knowledge in the API of
> what are 'details'.
> 
> In version 4 of the API we introduced a formal (kind of) specification
> of the API (a.k.a. the model), and int includes knowledge about what are
> 'links'. For example, the specification of the VM type contains this:
> 
>   @Link DiskAttachment[] diskAttachments();
>   @Link Nic[] nics();
> 
> With this information we are now in a position where we can implement
> this in a generic way.
> 
> We intend to implement this using a mechanism similar to the existing
> 'detail' parameter:
> 
>   GET /ovirt-engine/api/vms/123?follow=disk_attachments,nics
> 
> The naive implementation of this is to let the API call itself. For
> example, when the user requests to follow the 'disk_attachments' detail
> the API can just call itself to get that:
> 
>   GET /ovirt-engine/api/vms/123/disk_attachments
> 
> However, we can't use that naive approach, if we do we end with the
> 1+C*N query problem described before. We need to use specific
> implementations for certain frequent use cases, like VMs+disks+nics, and
> that needs work in the API and in the backend.
> 
> Tomas, if you want to help moving this forward, please open a RFE and
> makes sure it gets attention.
> 
> 
> This sounds pretty good! I will open, but since we are talking already
> here I'll just use the opportunity to clarify the topic more and than
> I'll open the BZ.
> 
> What I can imagine is the GetAllVmsQuery will accept in params also the
> list of details it should provide. Than, the GetAllVmsQuery will
> implement the efficient way of retrieving this info as well.
> 
> So, from the API perspective, it will be about taking the
> ?follow= part and passing it to the backend query params.
> 
> What you think?
>  

Exactly, that is the point! The API by itself can't optimize database
queries, all it can do is call the backend. It is the backend that has
the opportunity and possibility to send optimized queries to the database.

For other less common things we can use the naive approach, and
implement the aggregation in the API itself. But for common use cases,
like VM+disks+nics, we need to do it in an efficient way.

> 
> 
> 2. Reuse of TLS sessions.
> 
> The part of creating TLS sessions that is expensive is the generation of
> the shared session key. That can be avoided if both the server and the
> client are careful and reuse the session, using the session cache
> mechanism built-in into TLS itself. The web servers that we use (Apache
> and Undertow) do implement this mechanism, and so do most of our
> clients. Make sure that your client uses it as well. In Java this is
> achieved re-using the SSLContext. We already do that for the engine to
> VDSM communciation for example. In JavaScript the browser already takes
> care of this.
> 
> 3. Parallelism and latency.
> 
> A typical problem that we have is that we send many request to the
> server. For example, to retrieve user sessions for a set of VMs we tend
> to send many requests like this:
> 
>   GET /ovirt-engine/api/vms/1/sessions
>   GET /ovirt/engine/api/vms/2/sessions
>   GET /ovirt-engine/api/vms/3/sessions
>   ...
> 
> And we do that in a synchronous way: send one, wait for the result, send
> another one, wait for the result, etc. This means that we don't take
> advantage of the parallelism of the server and that we add to each
> request the network round trip time. So if we have N requests, we have
> to wait at least N*RTT.
> 
> The web servers that we use support multiple connections, and the
> protocol that we use, HTTP, supports pipe-lining. This means that you
> can send multiple requests in 

Re: [ovirt-devel] Firewalld migration.

[Martin Sivak]

> Right, so why not create an Ansible playbook which the users can change
> (extend?), based on http://docs.ansible.com/ansible/firewalld_module.html ?

I think I like the playbook proposal the best.

Lets assume the engine provides a firewall.yaml playbook somewhere in /etc:

 - The default playbook would contain our default firewalld
configuration (we should also provide it in /usr/share/doc to give the
user a reference)
 - The engine or host deploy script can provide ansible variables so
the playbook can even be a parametrized one (port numbers)
 - The user can add rules he needs
 - The user can REPLACE the content with iptables rules if he wishes
so (we can even say it is unsupported, but possible)

As an extension:

 - We can provide ovirt-engine-firewalld ansible role with the default
config so we can properly update files using RPM. The user defined (or
default) playbook would just call this role and would stay intact
during package upgrades.

This is outside of database, allows customization and adapts to
whatever firewall backend we might need.

--
Martin Sivak
SLA / oVirt

On Mon, Mar 27, 2017 at 12:46 PM, Yaniv Kaul  wrote:
>
>
> On Mon, Mar 27, 2017 at 1:20 PM, Martin Perina  wrote:
>>
>>
>>
>> On Mon, Mar 27, 2017 at 12:00 PM, Yaniv Kaul  wrote:
>>>
>>>
>>>
>>> On Mon, Mar 27, 2017 at 11:55 AM, Martin Perina 
>>> wrote:

 Hi,

 so personally I don't like the current way how we store firewall
 configuration within engine (saving complete iptables commands as string). 
 I
 think should change the way how we store firewall configuration:

 1. On engine side I'd just store which services/ports (or port ranges)
 need to be enabled on host. By default only those services/ports that 
 engine
 needs, but we can maintain also custom services defined by users
>>>
>>>
>>> Agreed. I hope that's enough on one hand, on the other hand, users can
>>> probably easily extend it via Ansible to the hosts and execution of a more
>>> customized firewalld configuration there - we probably should not own it.
>>
>>
>> Right, we should take care only about services/ports which we need. So we
>> probably could drop the ability for users to define their own custom
>> services/ports within engine for firewalld and force them to use Ansible or
>> other tools to handle their own configuration.
>
>
> Right, so why not create an Ansible playbook which the users can change
> (extend?), based on http://docs.ansible.com/ansible/firewalld_module.html ?
> ...
>>
>>
>>>


 2. Write plugin to ovirt-host-deploy which will translate those
 services/ports into actual firewall configuration on the host (it should
 detected what firewall is currently enabled and adapt)
>
>
> ...  and then ovirt-host-deploy will be in charge of executing that
> playbook? Seems fairly straightforward.
> Y.
>
>>>
>>> Agreed.
>>>


 3. For newly installed host I'd just use firewalld
>>>
>>>
>>> Agreed.
>>>


 4. Also for 4.2 clusters I'd switch from iptables to firewalld when you
 execute Reinstall (we should document this and make firewalld preferred
 solution)
>>>
>>>
>>> That's a good question. If a user had the default, non-changed policy we
>>> have had in iptables - sure.
>>> If not, I think it may be a bit of a challenge to switch otherwise.
>>
>>
>> Right. We could detect if there's some custom firewall rules in
>> IPTablesConfigSiteCustom engine-config option and if not we could probably
>> assume that switching to firewalld could be performed.
>>
>> We could also mark iptables configuration as deprecated in 4.2 and declare
>> that it will be removed in 4.3. That would add some time for users to
>> prepare for the switch ...
>>
>>> Y.
>>>




 Martin



 On Mon, Mar 27, 2017 at 8:01 AM, Yedidyah Bar David 
 wrote:
>
> On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 
> wrote:
> > Effectively, upgrading will leave lingering (but nonetheless
> > operational)
> > iptables rules on the hosts. I'm not even sure there needs to be
> > special
> > upgrade treatment?
>
> Please describe the expected flow.
>
> Please note that at least when I tried, 'systemctl start firewalld'
> stops
> iptables.
>
> Thanks,
>
> >
> > On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David 
> > wrote:
> >>
> >> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
> >> wrote:
> >> > 1) Do we actually need iptables for any reason that isn't a legacy
> >> > consideration?
> >>
> >> No idea personally.
> >>
> >> Perhaps some users prefer that, and/or need that for integration
> >> with
> >> other
> >> systems/solutions/whatever.
> >>
> >> If we drop iptables, how do you 

Re: [ovirt-devel] REST API data aggregation

[Tomas Jelinek]

On Mon, Mar 27, 2017 at 11:21 AM, Juan Hernández 
wrote:

> Top posting, sorry.
>
> There are a few things I'd like to clarify, regarding this subject:
>
> 1. Data aggregation, as requested now by Tomas, and by other people in
> the past.
>
> We used to have that 'detail' parameter, to aggregate certain very
> specific types of data, in particular to aggregate VM disks and NICs. We
> removed that in version 4 of the API because the implementation was
> extremely inefficient, from the engine point of view. An innocent
> request like this:
>
>   GET /ovirt-engine/api/vms?detail=+disks,+nics
>
> Would generate, with the implementation we used to have, 1 query for the
> VMs and then as many queries for disks and NICs as VMs in the system. In
> our scale test environments, for example, with approx 4000 VMs and 1
> disks, that would take more than 20 hours to execute.
>
> In addition, we didn't have in the past any mechanism to make this
> available in a generic one, because there was no knowledge in the API of
> what are 'details'.
>
> In version 4 of the API we introduced a formal (kind of) specification
> of the API (a.k.a. the model), and int includes knowledge about what are
> 'links'. For example, the specification of the VM type contains this:
>
>   @Link DiskAttachment[] diskAttachments();
>   @Link Nic[] nics();
>
> With this information we are now in a position where we can implement
> this in a generic way.
>
> We intend to implement this using a mechanism similar to the existing
> 'detail' parameter:
>
>   GET /ovirt-engine/api/vms/123?follow=disk_attachments,nics
>
> The naive implementation of this is to let the API call itself. For
> example, when the user requests to follow the 'disk_attachments' detail
> the API can just call itself to get that:
>
>   GET /ovirt-engine/api/vms/123/disk_attachments
>
> However, we can't use that naive approach, if we do we end with the
> 1+C*N query problem described before. We need to use specific
> implementations for certain frequent use cases, like VMs+disks+nics, and
> that needs work in the API and in the backend.
>
> Tomas, if you want to help moving this forward, please open a RFE and
> makes sure it gets attention.
>

This sounds pretty good! I will open, but since we are talking already here
I'll just use the opportunity to clarify the topic more and than I'll open
the BZ.

What I can imagine is the GetAllVmsQuery will accept in params also the
list of details it should provide. Than, the GetAllVmsQuery will implement
the efficient way of retrieving this info as well.

So, from the API perspective, it will be about taking the
?follow= part and passing it to the backend query params.

What you think?


>
> 2. Reuse of TLS sessions.
>
> The part of creating TLS sessions that is expensive is the generation of
> the shared session key. That can be avoided if both the server and the
> client are careful and reuse the session, using the session cache
> mechanism built-in into TLS itself. The web servers that we use (Apache
> and Undertow) do implement this mechanism, and so do most of our
> clients. Make sure that your client uses it as well. In Java this is
> achieved re-using the SSLContext. We already do that for the engine to
> VDSM communciation for example. In JavaScript the browser already takes
> care of this.
>
> 3. Parallelism and latency.
>
> A typical problem that we have is that we send many request to the
> server. For example, to retrieve user sessions for a set of VMs we tend
> to send many requests like this:
>
>   GET /ovirt-engine/api/vms/1/sessions
>   GET /ovirt/engine/api/vms/2/sessions
>   GET /ovirt-engine/api/vms/3/sessions
>   ...
>
> And we do that in a synchronous way: send one, wait for the result, send
> another one, wait for the result, etc. This means that we don't take
> advantage of the parallelism of the server and that we add to each
> request the network round trip time. So if we have N requests, we have
> to wait at least N*RTT.
>
> The web servers that we use support multiple connections, and the
> protocol that we use, HTTP, supports pipe-lining. This means that you
> can send multiple requests in parallel, and that you can send multiple
> requests without waiting for the response. To give you an idea of the
> improvement that can be achieved, we recently added asynchronous request
> support to the Ruby SDK, with multiple connections and pipe-lining. In
> our scale testing environment that reduced the time to collect a
> complete inventory from approx 30 min to approx 2 min. Here you have an
> example:
>
>
> https://github.com/oVirt/ovirt-engine-sdk-ruby/blob/master/sdk/examples/
> asynchronous_inventory.rb
>
> So make sure that you take advantage of that in your clients. Sadly
> pipe-lining is disabled by default in most browsers, so this isn't
> helpful for JavaScript applications.
>

But we can try what we can do in moVirt about this:
https://github.com/oVirt/moVirt/issues/260


>
> 4. 

Re: [ovirt-devel] vdsm-hook-ovs dependency

[Daniel Belenky]

thanks

On Mon, Mar 27, 2017 at 12:09 PM, Edward Haas  wrote:

> Hi Daniel,
>
> vdsm-hook-ovs has been removed on both master and 4.1, see
> https://gerrit.ovirt.org/#/c/69849/
>
> Thanks,
> Edy.
>
> On Sun, Mar 26, 2017 at 3:30 PM, Daniel Belenky 
> wrote:
>
>> Hi all,
>>
>> I try to test our master tested repo for repoclosure, and the test fails on 
>> the following dependency:
>>
>> 10:44:49 package: vdsm-hook-ovs-4.20.0-162.gitcc43be6.el7.centos.noarch from 
>> internal_repo
>> 10:44:49   unresolved deps:
>> 10:44:49  vdsm = 0:4.20.0-162.gitcc43be6.el7.centos
>>
>> When looking at the last builds of VDSM, I could not find this package: 
>> *vdsm-hook-ovs* in the 'exported artifacts'.
>> Can someone advise where this package is coming from? Do we need this 
>> package?
>>
>> Thanks,
>>
>> --
>>
>> *Daniel Belenky*
>>
>> *RHV DevOps*
>>
>> *Red Hat Israel*
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>
>


-- 

*Daniel Belenky*

*RHV DevOps*

*Red Hat Israel*
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Firewalld migration.

[Yaniv Kaul]

On Mon, Mar 27, 2017 at 1:20 PM, Martin Perina  wrote:

>
>
> On Mon, Mar 27, 2017 at 12:00 PM, Yaniv Kaul  wrote:
>
>>
>>
>> On Mon, Mar 27, 2017 at 11:55 AM, Martin Perina 
>> wrote:
>>
>>> Hi,
>>>
>>> so personally I don't like the current way how we store firewall
>>> configuration within engine (saving complete iptables commands as string).
>>> I think should change the way how we store firewall configuration:
>>>
>>> 1. On engine side I'd just store which services/ports (or port ranges)
>>> need to be enabled on host. By default only those services/ports that
>>> engine needs, but we can maintain also custom services defined by users
>>>
>>
>> Agreed. I hope that's enough on one hand, on the other hand, users can
>> probably easily extend it via Ansible to the hosts and execution of a more
>> customized firewalld configuration there - we probably should not own it.
>>
>
> ​Right, we should take care only about services/ports which we need. So we
> probably could drop the ability for users to define their own custom
> services/ports within engine​ for firewalld and force them to use Ansible
> or other tools to handle their own configuration.
>

Right, so why not create an Ansible playbook which the users can change
(extend?), based on http://docs.ansible.com/ansible/firewalld_module.html ?
...

>
>
>>
>>>
>>> 2. Write plugin to ovirt-host-deploy which will translate those
>>> services/ports into actual firewall configuration on the host (it should
>>> detected what firewall is currently enabled and adapt)
>>>
>>
...  and then ovirt-host-deploy will be in charge of executing that
playbook? Seems fairly straightforward.
Y.


>> Agreed.
>>
>>
>>>
>>> 3. For newly installed host I'd just use firewalld
>>>
>>
>> Agreed.
>>
>>
>>>
>>> 4. Also for 4.2 clusters I'd switch from iptables to firewalld when you
>>> execute Reinstall (we should document this and make firewalld preferred
>>> solution)
>>>
>>
>> That's a good question. If a user had the default, non-changed policy we
>> have had in iptables - sure.
>> If not, I think it may be a bit of a challenge to switch otherwise.
>>
>
> ​Right​. We could detect if there's some custom firewall rules in
> IPTablesConfigSiteCustom engine-config option and if not we could probably
> assume that switching to firewalld could be performed.
>
> We could also mark iptables configuration as deprecated in 4.2 and declare
> that it will be removed in 4.3. That would add some time for users to
> prepare for the switch ...
>
> Y.
>>
>>
>>>
>>>
>>>
>>> Martin
>>>
>>>
>>>
>>> On Mon, Mar 27, 2017 at 8:01 AM, Yedidyah Bar David 
>>> wrote:
>>>
 On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 
 wrote:
 > Effectively, upgrading will leave lingering (but nonetheless
 operational)
 > iptables rules on the hosts. I'm not even sure there needs to be
 special
 > upgrade treatment?

 Please describe the expected flow.

 Please note that at least when I tried, 'systemctl start firewalld'
 stops
 iptables.

 Thanks,

 >
 > On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David 
 wrote:
 >>
 >> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
 >> wrote:
 >> > 1) Do we actually need iptables for any reason that isn't a legacy
 >> > consideration?
 >>
 >> No idea personally.
 >>
 >> Perhaps some users prefer that, and/or need that for integration with
 >> other
 >> systems/solutions/whatever.
 >>
 >> If we drop iptables, how do you suggest to treat upgrades?
 >>
 >> >
 >> > 2 & 3) I am in favor of treating custom services as a requirement
 and
 >> > plan
 >> > accordingly. Many (most, even) of the services are already
 provided by
 >> > either firewalld itself (e.g. vdsm, libvirt) or the 3rd party
 packages
 >> > (e.g.
 >> > gluster). Some are missing (I've recently created a pull request
 for
 >> > ovirt-imageio to firewalld, for example) and I hope we'll be able
 to get
 >> > all
 >> > the services to be statically provided (by either firewalld or the
 >> > relevant
 >> > 3rd party packages).
 >> >
 >> > Ideally I think we'd like use statically provided services, and
 provide
 >> > the
 >> > capability to provide additional services (I'm not a fan of the
 current
 >> > methodology of converting strings into xmls). I don't think we'd
 want to
 >> > limit usage to just statically provided services. (2)
 >> >
 >> > As previously stated, I don't see a technical reason to keep
 iptables
 >> > under
 >> > consideration. (3)
 >> >
 >> >
 >> > On Sun, Mar 26, 2017 at 2:57 PM, Yedidyah Bar David <
 d...@redhat.com>
 >> > wrote:
 >> >>
 >> >>
 >> >> 1. Do we want to support in some 

Re: [ovirt-devel] Firewalld migration.

[Martin Perina]

On Mon, Mar 27, 2017 at 12:00 PM, Yaniv Kaul  wrote:

>
>
> On Mon, Mar 27, 2017 at 11:55 AM, Martin Perina 
> wrote:
>
>> Hi,
>>
>> so personally I don't like the current way how we store firewall
>> configuration within engine (saving complete iptables commands as string).
>> I think should change the way how we store firewall configuration:
>>
>> 1. On engine side I'd just store which services/ports (or port ranges)
>> need to be enabled on host. By default only those services/ports that
>> engine needs, but we can maintain also custom services defined by users
>>
>
> Agreed. I hope that's enough on one hand, on the other hand, users can
> probably easily extend it via Ansible to the hosts and execution of a more
> customized firewalld configuration there - we probably should not own it.
>

​Right, we should take care only about services/ports which we need. So we
probably could drop the ability for users to define their own custom
services/ports within engine​ for firewalld and force them to use Ansible
or other tools to handle their own configuration.


>
>>
>> 2. Write plugin to ovirt-host-deploy which will translate those
>> services/ports into actual firewall configuration on the host (it should
>> detected what firewall is currently enabled and adapt)
>>
>
> Agreed.
>
>
>>
>> 3. For newly installed host I'd just use firewalld
>>
>
> Agreed.
>
>
>>
>> 4. Also for 4.2 clusters I'd switch from iptables to firewalld when you
>> execute Reinstall (we should document this and make firewalld preferred
>> solution)
>>
>
> That's a good question. If a user had the default, non-changed policy we
> have had in iptables - sure.
> If not, I think it may be a bit of a challenge to switch otherwise.
>

​Right​. We could detect if there's some custom firewall rules in
IPTablesConfigSiteCustom engine-config option and if not we could probably
assume that switching to firewalld could be performed.

We could also mark iptables configuration as deprecated in 4.2 and declare
that it will be removed in 4.3. That would add some time for users to
prepare for the switch ...

Y.
>
>
>>
>>
>>
>> Martin
>>
>>
>>
>> On Mon, Mar 27, 2017 at 8:01 AM, Yedidyah Bar David 
>> wrote:
>>
>>> On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 
>>> wrote:
>>> > Effectively, upgrading will leave lingering (but nonetheless
>>> operational)
>>> > iptables rules on the hosts. I'm not even sure there needs to be
>>> special
>>> > upgrade treatment?
>>>
>>> Please describe the expected flow.
>>>
>>> Please note that at least when I tried, 'systemctl start firewalld' stops
>>> iptables.
>>>
>>> Thanks,
>>>
>>> >
>>> > On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David 
>>> wrote:
>>> >>
>>> >> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
>>> >> wrote:
>>> >> > 1) Do we actually need iptables for any reason that isn't a legacy
>>> >> > consideration?
>>> >>
>>> >> No idea personally.
>>> >>
>>> >> Perhaps some users prefer that, and/or need that for integration with
>>> >> other
>>> >> systems/solutions/whatever.
>>> >>
>>> >> If we drop iptables, how do you suggest to treat upgrades?
>>> >>
>>> >> >
>>> >> > 2 & 3) I am in favor of treating custom services as a requirement
>>> and
>>> >> > plan
>>> >> > accordingly. Many (most, even) of the services are already provided
>>> by
>>> >> > either firewalld itself (e.g. vdsm, libvirt) or the 3rd party
>>> packages
>>> >> > (e.g.
>>> >> > gluster). Some are missing (I've recently created a pull request for
>>> >> > ovirt-imageio to firewalld, for example) and I hope we'll be able
>>> to get
>>> >> > all
>>> >> > the services to be statically provided (by either firewalld or the
>>> >> > relevant
>>> >> > 3rd party packages).
>>> >> >
>>> >> > Ideally I think we'd like use statically provided services, and
>>> provide
>>> >> > the
>>> >> > capability to provide additional services (I'm not a fan of the
>>> current
>>> >> > methodology of converting strings into xmls). I don't think we'd
>>> want to
>>> >> > limit usage to just statically provided services. (2)
>>> >> >
>>> >> > As previously stated, I don't see a technical reason to keep
>>> iptables
>>> >> > under
>>> >> > consideration. (3)
>>> >> >
>>> >> >
>>> >> > On Sun, Mar 26, 2017 at 2:57 PM, Yedidyah Bar David <
>>> d...@redhat.com>
>>> >> > wrote:
>>> >> >>
>>> >> >>
>>> >> >> 1. Do we want to support in some version X both iptables and
>>> firewalld,
>>> >> >> or
>>> >> >> is it ok to stop support for iptables and support only firewalld
>>> >> >> without
>>> >> >> overlap? If so, do we handle upgrades, and how?
>>> >> >>
>>> >> >> 2. Do we want to support custom firewalld xml to be configured on
>>> the
>>> >> >> host by us? Or is it ok to only support choosing among existing
>>> >> >> services,
>>> >> >> which will need to be added to the host using other means
>>> (packaged by
>>> >> >> firewalld, packaged by 3rd parties, 

Re: [ovirt-devel] Writing SQL queries in DAO code

[Eli Mesika]

On Mon, Mar 27, 2017 at 12:26 PM, Yevgeny Zaspitsky 
wrote:

> > I don't think that looking in SQL in Java code is ​clear than looking in
> a SP code
>
> Looking in SQL isn't the problem I'm trying to solve, but finding that.
>
> > 1) You will pass more data on the wire instead of calling a SP with
> parameters
>
> Passing the SQL string shouldn't be a problem unless that is very long one
> (several K) and then we probably do something wrong.
>
> > 2) Your data that is passed on the wire is exposed to attacks since you
> will have to implement DB security in the engine level (for example hidden
> columns).
>
> IIUC currently querying tables/views with hidden columns is implemented in
> a SP that consist of at least 2 SQL's, so those aren't good candidates for
> my proposal and will stay as is.
> BTW how other projects resolve the security problem? AFAIK usually hidden
> columns are hidden by defining a proper view that do not include those.
>

​That's a bad solution as long as your data is passed unmasked on the wire ​
Database security should be done in the database level and you will not be
able to do that without using SPs



>
> > 3) Changes in the SQL code done in patches may be more complicated to
> track
>
> Most of SQL code changes involve changes in a DAO too, so this shouldn't
> be an issue.
>
​It is !!! , changes in DAO are easy to track since it is a Java code , you
are suggesting to write the SQL itself ​


>
> > 4) SQL Injection
>
> Again, how other projects resolve the security problem? Internet is full
> of articles/blogs of why stored procedures should be avoided ("avoid
> ​​
> stored procedures" Google query returned ~6.6M results), so I guess there
> are some other approaches for the security issue if that exists.
>

​"use stored procedures" Google query returned About 4,840,000 results ​



>
> > 5) I think that SP performs better than SQL inside Java
>
> Do we have a measurement of how much better?
> One of my intentions for this proposal is that people evidently avoid
> creating neat SQL queries and re-use the existing ones, which has much
> bigger performance impact. I guess that the biggest limit here is how
> complicated that procedure is in the engine code.
>

​That's why we have code review process and we should nack such ​patches ,
so you think that if people are not familiar with Java8 syntax for example
we should move this code to be performed by a "easier" mechanism ?
If people are not doing the right thing , we have gerrit for that, we can
comment , nack , whatever to make the code good



>
>
> On Mon, Mar 27, 2017 at 11:09 AM, Eli Mesika  wrote:
>
>>
>>
>> On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag  wrote:
>>
>>>
>>>
>>> On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky 
>>> wrote:
>>>
 Hi All,

 Recently I had a task of performance improvement in one of our network
 related flows and had some hard time following our DAL code and one of the
 outcomes of the task was defining a couple of new quite simple, but neat
 queries.
 When I came to coding those new queries I remembered how hard was
 following the existing DAL code, I decided to make my own new methods
 clearer. So I created [1] and [2] patches.

 Everything is quite standard there beside the fact that they do not use
 any stored procedure, but use SQL directly, IMHO by that they save time
 that I spent in trying to follow what a DAO method does. Looking into the
 method code you get the understanding of what this method is all about:

- no looking for a stored procedure name that is buried in the DAO
class hierarchy
- no looking for the SP definition


>>> There are additional pros and cons for the suggestion to consider:
>>>
>>> Pros:
>>>
>>>1. No need to run engine-setup after changing DB related code (in
>>>case of SQL inside Java).
>>>
>>> Cons:
>>>
>>>1. DAO files might become very long.
>>>2. If you decide to return the business entity associated with the
>>>DAO as a returned object, you won't know as a caller which fields to 
>>> expect
>>>to be populated, which lead to 3:
>>>3. An inflation of business entities to represent partial populated
>>>business entity or inflation of mappers inflation (this will be required
>>>for SP as well).
>>>4. SQL code inside of Java:
>>>1. Beside of the fact that a multi-line concatenated string that
>>>   cannot be easily copied and run with psql, it means that we should 
>>> compile
>>>   the code in order to test the change (vs building with DEV_REBUILD=0 
>>> which
>>>   only package the SQL file).
>>>   2. No syntax highlighting when performing code review. i.e. I
>>>   don't think reviewing a patch such as
>>>   https://gerrit.ovirt.org/#/c/66729/10/packaging/dbscripts/ne
>>>   twork_sp.sql
>>>   
>>> 

Re: [ovirt-devel] Writing SQL queries in DAO code

[Yevgeny Zaspitsky]

> I don't think that looking in SQL in Java code is ​clear than looking in
a SP code

Looking in SQL isn't the problem I'm trying to solve, but finding that.

> 1) You will pass more data on the wire instead of calling a SP with
parameters

Passing the SQL string shouldn't be a problem unless that is very long one
(several K) and then we probably do something wrong.

> 2) Your data that is passed on the wire is exposed to attacks since you
will have to implement DB security in the engine level (for example hidden
columns).

IIUC currently querying tables/views with hidden columns is implemented in
a SP that consist of at least 2 SQL's, so those aren't good candidates for
my proposal and will stay as is.
BTW how other projects resolve the security problem? AFAIK usually hidden
columns are hidden by defining a proper view that do not include those.

> 3) Changes in the SQL code done in patches may be more complicated to
track

Most of SQL code changes involve changes in a DAO too, so this shouldn't be
an issue.

> 4) SQL Injection

Again, how other projects resolve the security problem? Internet is full of
articles/blogs of why stored procedures should be avoided ("avoid stored
procedures" Google query returned ~6.6M results), so I guess there are some
other approaches for the security issue if that exists.

> 5) I think that SP performs better than SQL inside Java

Do we have a measurement of how much better?
One of my intentions for this proposal is that people evidently avoid
creating neat SQL queries and re-use the existing ones, which has much
bigger performance impact. I guess that the biggest limit here is how
complicated that procedure is in the engine code.


On Mon, Mar 27, 2017 at 11:09 AM, Eli Mesika  wrote:

>
>
> On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag  wrote:
>
>>
>>
>> On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky 
>> wrote:
>>
>>> Hi All,
>>>
>>> Recently I had a task of performance improvement in one of our network
>>> related flows and had some hard time following our DAL code and one of the
>>> outcomes of the task was defining a couple of new quite simple, but neat
>>> queries.
>>> When I came to coding those new queries I remembered how hard was
>>> following the existing DAL code, I decided to make my own new methods
>>> clearer. So I created [1] and [2] patches.
>>>
>>> Everything is quite standard there beside the fact that they do not use
>>> any stored procedure, but use SQL directly, IMHO by that they save time
>>> that I spent in trying to follow what a DAO method does. Looking into the
>>> method code you get the understanding of what this method is all about:
>>>
>>>- no looking for a stored procedure name that is buried in the DAO
>>>class hierarchy
>>>- no looking for the SP definition
>>>
>>>
>> There are additional pros and cons for the suggestion to consider:
>>
>> Pros:
>>
>>1. No need to run engine-setup after changing DB related code (in
>>case of SQL inside Java).
>>
>> Cons:
>>
>>1. DAO files might become very long.
>>2. If you decide to return the business entity associated with the
>>DAO as a returned object, you won't know as a caller which fields to 
>> expect
>>to be populated, which lead to 3:
>>3. An inflation of business entities to represent partial populated
>>business entity or inflation of mappers inflation (this will be required
>>for SP as well).
>>4. SQL code inside of Java:
>>1. Beside of the fact that a multi-line concatenated string that
>>   cannot be easily copied and run with psql, it means that we should 
>> compile
>>   the code in order to test the change (vs building with DEV_REBUILD=0 
>> which
>>   only package the SQL file).
>>   2. No syntax highlighting when performing code review. i.e. I
>>   don't think reviewing a patch such as
>>   https://gerrit.ovirt.org/#/c/66729/10/packaging/dbscripts/ne
>>   twork_sp.sql
>>   
>> 
>>   would be more clear inside a java file.
>>   3. The user permissions management is implemented on DB level.
>>   That means that SQL will be more complex (more concatenated java 
>> lines).
>>5. Stored procedure are cached by project's code. See
>>SimpleJdbcCallsHandler.getCall(), while the
>>NamedParameterJdbcTemplate are cached by spring's code which seems less
>>optimal (sync all calls using synchronized vs using ConcurrentHashMap as 
>> in
>>SP cache).
>>6. With the NamedParametersJdbcTemplate there is no use of the
>>DbEngineDialect. What's the impact of it ?
>>
>> So besides 5 and 6, the rest is a matter of style. I'd like to hear more
>> opinions from other members.
>>
>
> ​I agree with all you wrote Moti
> I don't think that looking in SQL in Java code is ​clear than looking in a
> SP code
> Additionally
> 1) You will pass more data on 

Re: [ovirt-devel] Writing SQL queries in DAO code

[Yevgeny Zaspitsky]

> 1. DAO files might become very long.

They would not be much longer than the current alternative, the .sql files.

> 2. An inflation of business entities to represent partial populated
business entity or inflation of mappers inflation (this will be required
for SP as well).

This isn't a problem of the DAO layer but bad design of business objects
that do not verify their own state. That could happen in any layer and
happens in existing DAO that uses stored procedures already.

> 3. An inflation of business entities to represent partial populated
business entity or inflation of mappers inflation (this will be required
for SP as well).

Look at my patches: one of them returns list of ids and another one returns
whole Cluster object and uses the existing RowMapper that populates the
whole Cluster object, so in both cases no much effort is needed in order to
keep the current state in terms of BE completness. IMHO having to create a
new class for a specific query would be the limit that will stop people
from creating queries that return something that isn't an existing type.
Anyhow I do not see how the proposed is changes the current state, can't we
do the same with a stored procedure?

> 4.1 Beside of the fact that a multi-line concatenated string that cannot
be easily copied and run with psql, it means that we should compile the
code in order to test the change (vs building with DEV_REBUILD=0 which only
package the SQL file).

Running a unit-test in your IDE after updating the in-code SQL is even
faster than the current flow.

> 4.2 No syntax highlighting when performing code review. i.e. I don't
think reviewing a patch such as https://gerrit.ovirt.org/#/c/
66729/10/packaging/dbscripts/network_sp.sql would be more clear inside a
java file.

If you review the DAO Java code in your IDE (I'd recommend that for any
Java change) you get the hiighlighting too. Beside that a code is
written/reviewed only once but is read many times and here my proposal is
much easier.

> 4.3 The user permissions management is implemented on DB level. That
means that SQL will be more complex (more concatenated java lines).

We can keep those long queries in SPs if you like. But I'm in favor to
enable the shorter one to be in-lined.

> 5. Stored procedure are cached by project's code. See
SimpleJdbcCallsHandler.getCall(), while the NamedParameterJdbcTemplate are
cached by spring's code which seems less optimal (sync all calls using
synchronized vs using ConcurrentHashMap as in SP cache).

I'm not sure that this caching code is needed. Has this proven impact/need?
Are we pioneers in JDBC area? How other projects do that?

> 6. With the NamedParametersJdbcTemplate there is no use of the
DbEngineDialect. What's the impact of it ?

In my patches I do use it through getCustomMapSqlParameterSource.


On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag  wrote:

>
>
> On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky 
> wrote:
>
>> Hi All,
>>
>> Recently I had a task of performance improvement in one of our network
>> related flows and had some hard time following our DAL code and one of the
>> outcomes of the task was defining a couple of new quite simple, but neat
>> queries.
>> When I came to coding those new queries I remembered how hard was
>> following the existing DAL code, I decided to make my own new methods
>> clearer. So I created [1] and [2] patches.
>>
>> Everything is quite standard there beside the fact that they do not use
>> any stored procedure, but use SQL directly, IMHO by that they save time
>> that I spent in trying to follow what a DAO method does. Looking into the
>> method code you get the understanding of what this method is all about:
>>
>>- no looking for a stored procedure name that is buried in the DAO
>>class hierarchy
>>- no looking for the SP definition
>>
>>
> There are additional pros and cons for the suggestion to consider:
>
> Pros:
>
>1. No need to run engine-setup after changing DB related code (in case
>of SQL inside Java).
>
> Cons:
>
>1. DAO files might become very long.
>2. If you decide to return the business entity associated with the DAO
>as a returned object, you won't know as a caller which fields to expect to
>be populated, which lead to 3:
>3. An inflation of business entities to represent partial populated
>business entity or inflation of mappers inflation (this will be required
>for SP as well).
>4. SQL code inside of Java:
>1. Beside of the fact that a multi-line concatenated string that
>   cannot be easily copied and run with psql, it means that we should 
> compile
>   the code in order to test the change (vs building with DEV_REBUILD=0 
> which
>   only package the SQL file).
>   2. No syntax highlighting when performing code review. i.e. I don't
>   think reviewing a patch such as https://gerrit.ovirt.org/#/c/
>   66729/10/packaging/dbscripts/network_sp.sql
>   
> 

Re: [ovirt-devel] Firewalld migration.

[Martin Perina]

Hi,

so personally I don't like the current way how we store firewall
configuration within engine (saving complete iptables commands as string).
I think should change the way how we store firewall configuration:

1. On engine side I'd just store which services/ports (or port ranges) need
to be enabled on host. By default only those services/ports that engine
needs, but we can maintain also custom services defined by users

2. Write plugin to ovirt-host-deploy which will translate those
services/ports into actual firewall configuration on the host (it should
detected what firewall is currently enabled and adapt)

3. For newly installed host I'd just use firewalld

4. Also for 4.2 clusters I'd switch from iptables to firewalld when you
execute Reinstall (we should document this and make firewalld preferred
solution)


Martin



On Mon, Mar 27, 2017 at 8:01 AM, Yedidyah Bar David  wrote:

> On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg 
> wrote:
> > Effectively, upgrading will leave lingering (but nonetheless operational)
> > iptables rules on the hosts. I'm not even sure there needs to be special
> > upgrade treatment?
>
> Please describe the expected flow.
>
> Please note that at least when I tried, 'systemctl start firewalld' stops
> iptables.
>
> Thanks,
>
> >
> > On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David 
> wrote:
> >>
> >> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
> >> wrote:
> >> > 1) Do we actually need iptables for any reason that isn't a legacy
> >> > consideration?
> >>
> >> No idea personally.
> >>
> >> Perhaps some users prefer that, and/or need that for integration with
> >> other
> >> systems/solutions/whatever.
> >>
> >> If we drop iptables, how do you suggest to treat upgrades?
> >>
> >> >
> >> > 2 & 3) I am in favor of treating custom services as a requirement and
> >> > plan
> >> > accordingly. Many (most, even) of the services are already provided by
> >> > either firewalld itself (e.g. vdsm, libvirt) or the 3rd party packages
> >> > (e.g.
> >> > gluster). Some are missing (I've recently created a pull request for
> >> > ovirt-imageio to firewalld, for example) and I hope we'll be able to
> get
> >> > all
> >> > the services to be statically provided (by either firewalld or the
> >> > relevant
> >> > 3rd party packages).
> >> >
> >> > Ideally I think we'd like use statically provided services, and
> provide
> >> > the
> >> > capability to provide additional services (I'm not a fan of the
> current
> >> > methodology of converting strings into xmls). I don't think we'd want
> to
> >> > limit usage to just statically provided services. (2)
> >> >
> >> > As previously stated, I don't see a technical reason to keep iptables
> >> > under
> >> > consideration. (3)
> >> >
> >> >
> >> > On Sun, Mar 26, 2017 at 2:57 PM, Yedidyah Bar David 
> >> > wrote:
> >> >>
> >> >>
> >> >> 1. Do we want to support in some version X both iptables and
> firewalld,
> >> >> or
> >> >> is it ok to stop support for iptables and support only firewalld
> >> >> without
> >> >> overlap? If so, do we handle upgrades, and how?
> >> >>
> >> >> 2. Do we want to support custom firewalld xml to be configured on the
> >> >> host by us? Or is it ok to only support choosing among existing
> >> >> services,
> >> >> which will need to be added to the host using other means (packaged
> by
> >> >> firewalld, packaged by 3rd parties, added manually by users)?
> >> >>
> >> >> 3. Opposite of (2.): Do we want to support firewalld services that
> are
> >> >> added to the host using other means (see there)? Obviously we do,
> but:
> >> >> If we do, do we still want to support also iptables (see (1.))? And
> if
> >> >> so, what do we want to then happen?
> >> >>
> >> >> (2.) and (3.) are not conflicting, each needs its own answer.
> >> >>
> >> >>
> >> >> --
> >> >> Didi
> >> >
> >> >
> >>
> >>
> >>
> >> --
> >> Didi
> >
> >
>
>
>
> --
> Didi
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Writing SQL queries in DAO code

[Martin Sivak]

I agree with Moti, but I think we might have an alternative.

Does our DAO layer support named queries (aka PreparedStatements
loaded from a properties file)? Those are generally plain SQL queries
with arguments that are compiled in advance and still allow direct
access to the JDBC internals without the heavy translation layer.

Martin

On Sun, Mar 26, 2017 at 4:12 PM, Yevgeny Zaspitsky  wrote:
> Hi All,
>
> Recently I had a task of performance improvement in one of our network
> related flows and had some hard time following our DAL code and one of the
> outcomes of the task was defining a couple of new quite simple, but neat
> queries.
> When I came to coding those new queries I remembered how hard was following
> the existing DAL code, I decided to make my own new methods clearer. So I
> created [1] and [2] patches.
>
> Everything is quite standard there beside the fact that they do not use any
> stored procedure, but use SQL directly, IMHO by that they save time that I
> spent in trying to follow what a DAO method does. Looking into the method
> code you get the understanding of what this method is all about:
>
> no looking for a stored procedure name that is buried in the DAO class
> hierarchy
> no looking for the SP definition
>
> So I'd like to propose moving towards this approach in general in all cases
> when nothing beyond a simple SQL is needed (no stored procedure programming
> language is needed).
> From my experience with the performance improvement task it looks like
> people avoid adding new queries for a specific need of a flow, instead they
> use the existing general ones (e.g. dao.getAllForX()) and do the actual join
> in the bll code.
> IMHO the proposed approach would simplify adding new specific queries and by
> that would prevent a decent part of performance issues in the future.
>
> I do not propose changing all existing SP's to inline queries in a once, but
> to allow adding new queries this way. Also we might consider converting
> relatively simple SP's to inline SQL statements later in a graduate way.
>
> [1] - https://gerrit.ovirt.org/#/c/74456
> [2] - https://gerrit.ovirt.org/#/c/74458
>
> Regards,
> Yevgeny
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] master branch host installation fails in ovirt-system-tests - regression due to ABRT integration?

[Yaniv Bronheim]

The abrt packages are indeed not included. thanks for the report
https://gerrit.ovirt.org/#/c/74654/1/common/yum-repos/ovirt-master.repo

Verifying

On Sun, Mar 26, 2017 at 4:56 PM Yaniv Kaul  wrote:

> 2017-03-26 09:50:47,235-04 ERROR
> [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector]
> (VdsDeploy) [2c34dfce] EVENT_ID: VDS_INSTALL_IN_PROGRESS_ERROR(511),
> Correlation ID: 2c34dfce, Call Stack:
> null, Custom Event ID: -1, Message: Failed to install Host
> lago-basic-suite-master-host1. Failed to execute stage 'Package
> installation': [u'vdsm-4.20.0-542.git93156a7.el7.centos.x86_64 requires
> abrt-addon-vmcor
> e', u'vdsm-4.20.0-542.git93156a7.el7.centos.x86_64 requires
> abrt-addon-ccpp', u'vdsm-4.20.0-542.git93156a7.el7.centos.x86_64 requires
> abrt-addon-python'].
>
> --
Yaniv Bronhaim.
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Writing SQL queries in DAO code

[Eli Mesika]

On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag  wrote:

>
>
> On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky 
> wrote:
>
>> Hi All,
>>
>> Recently I had a task of performance improvement in one of our network
>> related flows and had some hard time following our DAL code and one of the
>> outcomes of the task was defining a couple of new quite simple, but neat
>> queries.
>> When I came to coding those new queries I remembered how hard was
>> following the existing DAL code, I decided to make my own new methods
>> clearer. So I created [1] and [2] patches.
>>
>> Everything is quite standard there beside the fact that they do not use
>> any stored procedure, but use SQL directly, IMHO by that they save time
>> that I spent in trying to follow what a DAO method does. Looking into the
>> method code you get the understanding of what this method is all about:
>>
>>- no looking for a stored procedure name that is buried in the DAO
>>class hierarchy
>>- no looking for the SP definition
>>
>>
> There are additional pros and cons for the suggestion to consider:
>
> Pros:
>
>1. No need to run engine-setup after changing DB related code (in case
>of SQL inside Java).
>
> Cons:
>
>1. DAO files might become very long.
>2. If you decide to return the business entity associated with the DAO
>as a returned object, you won't know as a caller which fields to expect to
>be populated, which lead to 3:
>3. An inflation of business entities to represent partial populated
>business entity or inflation of mappers inflation (this will be required
>for SP as well).
>4. SQL code inside of Java:
>1. Beside of the fact that a multi-line concatenated string that
>   cannot be easily copied and run with psql, it means that we should 
> compile
>   the code in order to test the change (vs building with DEV_REBUILD=0 
> which
>   only package the SQL file).
>   2. No syntax highlighting when performing code review. i.e. I don't
>   think reviewing a patch such as https://gerrit.ovirt.org/#/c/
>   66729/10/packaging/dbscripts/network_sp.sql
>   
> 
>   would be more clear inside a java file.
>   3. The user permissions management is implemented on DB level. That
>   means that SQL will be more complex (more concatenated java lines).
>5. Stored procedure are cached by project's code. See
>SimpleJdbcCallsHandler.getCall(), while the NamedParameterJdbcTemplate
>are cached by spring's code which seems less optimal (sync all calls using
>synchronized vs using ConcurrentHashMap as in SP cache).
>6. With the NamedParametersJdbcTemplate there is no use of the
>DbEngineDialect. What's the impact of it ?
>
> So besides 5 and 6, the rest is a matter of style. I'd like to hear more
> opinions from other members.
>

​I agree with all you wrote Moti
I don't think that looking in SQL in Java code is ​clear than looking in a
SP code
Additionally
1) You will pass more data on the wire instead of calling a SP with
parameters
2) Your data that is passed on the wire is exposed to attacks since you
will have to implement DB security in the engine level (for example hidden
columns)
3) Changes in the SQL code done in patches may be more complicated to track
4) SQL Injection
5) I think that SP performs better than SQL inside Java

I see no real reason to replace the SPs with SQL code , SP is just a
container for SQL code



>
> Regards,
> Moti
>
> So I'd like to propose moving towards this approach in general in all
>> cases when nothing beyond a simple SQL is needed (no stored procedure
>> programming language is needed).
>> From my experience with the performance improvement task it looks like
>> people avoid adding new queries for a specific need of a flow, instead they
>> use the existing general ones (e.g. dao.getAllForX()) and do the actual
>> join in the bll code.
>> IMHO the proposed approach would simplify adding new specific queries and
>> by that would prevent a decent part of performance issues in the future.
>>
>> I do not propose changing all existing SP's to inline queries in a once,
>> but to allow adding new queries this way. Also we might consider converting
>> relatively simple SP's to inline SQL statements later in a graduate way.
>>
>> [1] - https://gerrit.ovirt.org/#/c/74456
>> [2] - https://gerrit.ovirt.org/#/c/74458
>>
>> Regards,
>> Yevgeny
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>
>
>
> --
> Regards,
> Moti
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [ OST Failure Report ] [ oVirt 4.1 ] [ 24-03-2017 ] [ 002_bootstrap.list_glance_images ]

[Piotr Kliczewski]

On Mon, Mar 27, 2017 at 9:43 AM, Piotr Kliczewski
 wrote:
> On Sun, Mar 26, 2017 at 12:01 PM, Yaniv Kaul  wrote:
>>
>>
>> On Sun, Mar 26, 2017 at 11:59 AM, Shlomo Ben David 
>> wrote:
>>>
>>> Hi,
>>>
>>> Test failed: [ 002_bootstrap.list_glance_images ]
>>> Link to suspected patches: N/A
>>> Link to Job:
>>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1063
>>> Link to all logs:
>>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1063/artifact/exported-artifacts/basic-suit-4.1-el7/test_logs/basic-suite-4.1/post-002_bootstrap.py/
>>>
>>> Error snippet from the log:
>>
>>
>> Same error as in master - perhaps there's a different issue there? Is test
>> connectivity broken for some reason? Perhaps connectivity issue that we did
>> not overcome?
>
> Based on the logs I see that there was an issue with testing
> connection to the glance provider. There is no more information in the
> logs so we have a place for improvement here [1].

Here is the patch [2]

> Do we know whether glance that we use is OK?
>
> [1] 
> https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/AbstractOpenStackStorageProviderProxy.java#L55

[2] https://gerrit.ovirt.org/#/c/74653

>
>> Y.
>>
>>>
>>> 
>>>
>>> 2017-03-24 08:04:46,983-04 ERROR
>>> [org.ovirt.engine.core.vdsbroker.vdsbroker.PollVDSCommand]
>>> (org.ovirt.thread.pool-7-thread-1) [2b2b4693] Command
>>> 'PollVDSCommand(HostName = lago-basic-suite-4-1-host1,
>>> VdsIdVDSCommandParametersBase:{runAsync='true',
>>> hostId='746e816f-6e21-4185-9d50-3e90ebefb187'})' execution failed:
>>> VDSGenericException: VDSNetworkException: Timeout during rpc call
>>> 2017-03-24 08:04:46,983-04 DEBUG
>>> [org.ovirt.engine.core.vdsbroker.vdsbroker.PollVDSCommand]
>>> (org.ovirt.thread.pool-7-thread-1) [2b2b4693] Exception:
>>> org.ovirt.engine.core.vdsbroker.vdsbroker.VDSNetworkException:
>>> VDSGenericException: VDSNetworkException: Timeout during rpc call
>>> at
>>> org.ovirt.engine.core.vdsbroker.vdsbroker.FutureVDSCommand.get(FutureVDSCommand.java:73)
>>> [vdsbroker.jar:]
>>> at
>>> org.ovirt.engine.core.bll.network.host.HostSetupNetworkPoller.getValue(HostSetupNetworkPoller.java:56)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.network.host.HostSetupNetworkPoller.poll(HostSetupNetworkPoller.java:41)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.network.host.HostSetupNetworksCommand.invokeSetupNetworksCommand(HostSetupNetworksCommand.java:426)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.network.host.HostSetupNetworksCommand.executeCommand(HostSetupNetworksCommand.java:287)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.CommandBase.executeWithoutTransaction(CommandBase.java:1251)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.CommandBase.executeActionInTransactionScope(CommandBase.java:1391)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.CommandBase.runInTransaction(CommandBase.java:2055)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.utils.transaction.TransactionSupport.executeInSuppressed(TransactionSupport.java:164)
>>> [utils.jar:]
>>> at
>>> org.ovirt.engine.core.utils.transaction.TransactionSupport.executeInScope(TransactionSupport.java:103)
>>> [utils.jar:]
>>> at org.ovirt.engine.core.bll.CommandBase.execute(CommandBase.java:1451)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.CommandBase.executeAction(CommandBase.java:397)
>>> [bll.jar:]
>>> at
>>> org.ovirt.engine.core.bll.executor.DefaultBackendActionExecutor.execute(DefaultBackendActionExecutor.java:13)
>>> [bll.jar:]
>>> at org.ovirt.engine.core.bll.Backend.runAction(Backend.java:511)
>>> [bll.jar:]
>>> at org.ovirt.engine.core.bll.Backend.runActionImpl(Backend.java:493)
>>> [bll.jar:]
>>> at org.ovirt.engine.core.bll.Backend.runInternalAction(Backend.java:697)
>>> [bll.jar:]
>>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>> [rt.jar:1.8.0_121]
>>> at
>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>> [rt.jar:1.8.0_121]
>>> at
>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>> [rt.jar:1.8.0_121]
>>> at java.lang.reflect.Method.invoke(Method.java:498) [rt.jar:1.8.0_121]
>>> at
>>> org.jboss.as.ee.component.ManagedReferenceMethodInterceptor.processInvocation(ManagedReferenceMethodInterceptor.java:52)
>>> at
>>> org.jboss.invocation.InterceptorContext.proceed(InterceptorContext.java:340)
>>> at
>>> org.jboss.invocation.InterceptorContext$Invocation.proceed(InterceptorContext.java:437)
>>> at
>>> org.jboss.as.weld.ejb.Jsr299BindingsInterceptor.delegateInterception(Jsr299BindingsInterceptor.java:70)
>>> [wildfly-weld-10.1.0.Final.jar:10.1.0.Final]
>>> at
>>> org.jboss.as.weld.ejb.Jsr299BindingsInterceptor.doMethodInterception(Jsr299BindingsInterceptor.java:80)
>>> [wildfly-weld-10.1.0.Final.jar:10.1.0.Final]
>>> at
>>> 

Re: [ovirt-devel] [ OST Failure Report ] [ oVirt 4.1 ] [ 24-03-2017 ] [ 002_bootstrap.list_glance_images ]

[Piotr Kliczewski]

On Sun, Mar 26, 2017 at 12:01 PM, Yaniv Kaul  wrote:
>
>
> On Sun, Mar 26, 2017 at 11:59 AM, Shlomo Ben David 
> wrote:
>>
>> Hi,
>>
>> Test failed: [ 002_bootstrap.list_glance_images ]
>> Link to suspected patches: N/A
>> Link to Job:
>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1063
>> Link to all logs:
>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.1/1063/artifact/exported-artifacts/basic-suit-4.1-el7/test_logs/basic-suite-4.1/post-002_bootstrap.py/
>>
>> Error snippet from the log:
>
>
> Same error as in master - perhaps there's a different issue there? Is test
> connectivity broken for some reason? Perhaps connectivity issue that we did
> not overcome?

Based on the logs I see that there was an issue with testing
connection to the glance provider. There is no more information in the
logs so we have a place for improvement here [1].
Do we know whether glance that we use is OK?

[1] 
https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/AbstractOpenStackStorageProviderProxy.java#L55

> Y.
>
>>
>> 
>>
>> 2017-03-24 08:04:46,983-04 ERROR
>> [org.ovirt.engine.core.vdsbroker.vdsbroker.PollVDSCommand]
>> (org.ovirt.thread.pool-7-thread-1) [2b2b4693] Command
>> 'PollVDSCommand(HostName = lago-basic-suite-4-1-host1,
>> VdsIdVDSCommandParametersBase:{runAsync='true',
>> hostId='746e816f-6e21-4185-9d50-3e90ebefb187'})' execution failed:
>> VDSGenericException: VDSNetworkException: Timeout during rpc call
>> 2017-03-24 08:04:46,983-04 DEBUG
>> [org.ovirt.engine.core.vdsbroker.vdsbroker.PollVDSCommand]
>> (org.ovirt.thread.pool-7-thread-1) [2b2b4693] Exception:
>> org.ovirt.engine.core.vdsbroker.vdsbroker.VDSNetworkException:
>> VDSGenericException: VDSNetworkException: Timeout during rpc call
>> at
>> org.ovirt.engine.core.vdsbroker.vdsbroker.FutureVDSCommand.get(FutureVDSCommand.java:73)
>> [vdsbroker.jar:]
>> at
>> org.ovirt.engine.core.bll.network.host.HostSetupNetworkPoller.getValue(HostSetupNetworkPoller.java:56)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.network.host.HostSetupNetworkPoller.poll(HostSetupNetworkPoller.java:41)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.network.host.HostSetupNetworksCommand.invokeSetupNetworksCommand(HostSetupNetworksCommand.java:426)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.network.host.HostSetupNetworksCommand.executeCommand(HostSetupNetworksCommand.java:287)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.CommandBase.executeWithoutTransaction(CommandBase.java:1251)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.CommandBase.executeActionInTransactionScope(CommandBase.java:1391)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.CommandBase.runInTransaction(CommandBase.java:2055)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.utils.transaction.TransactionSupport.executeInSuppressed(TransactionSupport.java:164)
>> [utils.jar:]
>> at
>> org.ovirt.engine.core.utils.transaction.TransactionSupport.executeInScope(TransactionSupport.java:103)
>> [utils.jar:]
>> at org.ovirt.engine.core.bll.CommandBase.execute(CommandBase.java:1451)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.CommandBase.executeAction(CommandBase.java:397)
>> [bll.jar:]
>> at
>> org.ovirt.engine.core.bll.executor.DefaultBackendActionExecutor.execute(DefaultBackendActionExecutor.java:13)
>> [bll.jar:]
>> at org.ovirt.engine.core.bll.Backend.runAction(Backend.java:511)
>> [bll.jar:]
>> at org.ovirt.engine.core.bll.Backend.runActionImpl(Backend.java:493)
>> [bll.jar:]
>> at org.ovirt.engine.core.bll.Backend.runInternalAction(Backend.java:697)
>> [bll.jar:]
>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> [rt.jar:1.8.0_121]
>> at
>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>> [rt.jar:1.8.0_121]
>> at
>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> [rt.jar:1.8.0_121]
>> at java.lang.reflect.Method.invoke(Method.java:498) [rt.jar:1.8.0_121]
>> at
>> org.jboss.as.ee.component.ManagedReferenceMethodInterceptor.processInvocation(ManagedReferenceMethodInterceptor.java:52)
>> at
>> org.jboss.invocation.InterceptorContext.proceed(InterceptorContext.java:340)
>> at
>> org.jboss.invocation.InterceptorContext$Invocation.proceed(InterceptorContext.java:437)
>> at
>> org.jboss.as.weld.ejb.Jsr299BindingsInterceptor.delegateInterception(Jsr299BindingsInterceptor.java:70)
>> [wildfly-weld-10.1.0.Final.jar:10.1.0.Final]
>> at
>> org.jboss.as.weld.ejb.Jsr299BindingsInterceptor.doMethodInterception(Jsr299BindingsInterceptor.java:80)
>> [wildfly-weld-10.1.0.Final.jar:10.1.0.Final]
>> at
>> org.jboss.as.weld.ejb.Jsr299BindingsInterceptor.processInvocation(Jsr299BindingsInterceptor.java:93)
>> [wildfly-weld-10.1.0.Final.jar:10.1.0.Final]
>> at
>> org.jboss.as.ee.component.interceptors.UserInterceptorFactory$1.processInvocation(UserInterceptorFactory.java:63)
>> at

Re: [ovirt-devel] [ OST Failure Report ] [ oVirt 4.1 && oVirt master ] [ 20-03-2017 ] [ 004_basic_sanity.snapshots_merge ]

[Pavel Zhukov]

Hi all,
Happened again
http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_master/6017/console


On Wed, Mar 22, 2017 at 4:48 PM, Gil Shinar  wrote:

> Looks like the actuall error is in VDSM log:
>
> 2017-03-22 11:01:22,410-0400 ERROR (jsonrpc/7) [storage.TaskManager.Task] 
> (Task='179424c1-b23d-40aa-a091-3c197971f420') Unexpected error (task:871)
> Traceback (most recent call last):
>   File "/usr/lib/python2.7/site-packages/vdsm/storage/task.py", line 878, in 
> _run
> return fn(*args, **kargs)
>   File "/usr/lib/python2.7/site-packages/vdsm/logUtils.py", line 52, in 
> wrapper
> res = f(*args, **kwargs)
>   File "/usr/share/vdsm/storage/hsm.py", line 3060, in getVolumeInfo
> volUUID=volUUID).getInfo()
>   File "/usr/share/vdsm/storage/sd.py", line 748, in produceVolume
> volUUID)
>   File "/usr/share/vdsm/storage/fileVolume.py", line 361, in __init__
> manifest = self.manifestClass(repoPath, sdUUID, imgUUID, volUUID)
>   File "/usr/share/vdsm/storage/fileVolume.py", line 61, in __init__
> volUUID)
>   File "/usr/share/vdsm/storage/volume.py", line 86, in __init__
> self.validate()
>   File "/usr/share/vdsm/storage/volume.py", line 109, in validate
> self.validateVolumePath()
>   File "/usr/share/vdsm/storage/fileVolume.py", line 120, in 
> validateVolumePath
> raise se.VolumeDoesNotExist(self.volUUID)
> VolumeDoesNotExist: Volume does not exist: 
> (u'5133a146-d4bc-49a1-8358-7e14522ffc4b',)
> 2017-03-22 11:01:22,413-0400 INFO  (jsonrpc/7) [storage.TaskManager.Task] 
> (Task='179424c1-b23d-40aa-a091-3c197971f420') aborting: Task is aborted: 
> 'Volume does not exist' - code 201 (task:1176)
> 2017-03-22 11:01:22,414-0400 ERROR (jsonrpc/7) [storage.Dispatcher] 
> {'status': {'message': "Volume does not exist: 
> (u'5133a146-d4bc-49a1-8358-7e14522ffc4b',)", 'code': 201}} (dispatcher:78)
>
>
> On Wed, Mar 22, 2017 at 10:57 AM, Gil Shinar  wrote:
>
>> It has happened again:
>> http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.
>> 1/1033/artifact/exported-artifacts/basic-suit-4.1-el7/test_
>> logs/basic-suite-4.1/post-004_basic_sanity.py/
>>
>> Any news concerning this issue?
>>
>> Thanks
>> Gil
>>
>> On Mon, Mar 20, 2017 at 5:53 PM, Alexander Wels  wrote:
>>
>>> On Monday, March 20, 2017 11:01:21 AM EDT Yaniv Kaul wrote:
>>> > On Mon, Mar 20, 2017 at 4:18 PM, Shlomo Ben David >> >
>>> >
>>> > wrote:
>>> > > Hi,
>>> > >
>>> > >
>>> > > Test failed: [ 004_basic_sanity.snapshots_merge ]
>>> > >
>>> > > Link to suspected patches: N/A
>>> > >
>>> > > Link to Job:
>>> > >1. http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_4.
>>> 1/1007
>>> > >2. http://jenkins.ovirt.org/job/test-repo_ovirt_experimental_
>>> > >master/5918
>>> >
>>> > They look similar, but master has an additional failure (unrelated to
>>> the
>>> > snap merge failure).
>>> > Allon/Tal - can you have someone from storage look at the failures?
>>> > Alexander - can you take a look at the below?
>>> >
>>> >
>>> > 2017-03-20 09:30:10,283-04 DEBUG
>>> > [org.ovirt.engine.core.dal.dbbroker.PostgresDbEngineDialect$
>>> PostgresSimpleJd
>>> > bcCall] (ServerService Thread Pool -- 48) [] SqlCall for procedure
>>> > [GetAllMacPoolRangesByMacPoolId] compiled
>>> > 2017-03-20 09:30:10,283-04 INFO  [org.ovirt.engine.core.bll.Tag
>>> sDirector]
>>> > (ServerService Thread Pool -- 41) [] Finished initializing TagsDirector
>>> > 2017-03-20 09:30:10,275-04 ERROR
>>> > [org.ovirt.engine.ui.frontend.server.dashboard.DashboardData
>>> Servlet.CacheUpd
>>> > ate.Utilization] (EE-ManagedThreadFactory-default-Thread-2) [] Could
>>> not
>>> > update the Utilization Cache: Error while running SQL query:
>>> > org.ovirt.engine.ui.frontend.server.dashboard.DashboardDataException:
>>> Error
>>> > while running SQL query
>>> > at
>>> > org.ovirt.engine.ui.frontend.server.dashboard.dao.BaseDao.ru
>>> nQuery(BaseDao.j
>>> > ava:60) [frontend.jar:]
>>> > at
>>> > org.ovirt.engine.ui.frontend.server.dashboard.dao.HostDwhDao
>>> .getTotalCpuMemC
>>> > ount(HostDwhDao.java:78) [frontend.jar:]
>>> > at
>>> > org.ovirt.engine.ui.frontend.server.dashboard.HourlySummaryH
>>> elper.getTotalCp
>>> > uMemCount(HourlySummaryHelper.java:43) [frontend.jar:]
>>> > at
>>> > org.ovirt.engine.ui.frontend.server.dashboard.HourlySummaryH
>>> elper.getCpuMemS
>>> > ummary(HourlySummaryHelper.java:21) [frontend.jar:]
>>> > at
>>> > org.ovirt.engine.ui.frontend.server.dashboard.DashboardDataS
>>> ervlet.lookupGlo
>>> > balUtilization(DashboardDataServlet.java:287) [frontend.jar:]
>>> > at
>>> > org.ovirt.engine.ui.frontend.server.dashboard.DashboardDataS
>>> ervlet.getDashbo
>>> > ard(DashboardDataServlet.java:261) [frontend.jar:]
>>> >
>>>
>>> So it looks like a communication failure with the DWH. But when I look
>>> at the
>>> associated logs, the error is not in there, nor do the time stamps
>>> match. Is
>>> this error from the same run or a 

Re: [ovirt-devel] Firewalld migration.

[Yedidyah Bar David]

On Sun, Mar 26, 2017 at 6:01 PM, Leon Goldberg  wrote:
> Effectively, upgrading will leave lingering (but nonetheless operational)
> iptables rules on the hosts. I'm not even sure there needs to be special
> upgrade treatment?

Please describe the expected flow.

Please note that at least when I tried, 'systemctl start firewalld' stops
iptables.

Thanks,

>
> On Sun, Mar 26, 2017 at 4:59 PM, Yedidyah Bar David  wrote:
>>
>> On Sun, Mar 26, 2017 at 4:49 PM, Leon Goldberg 
>> wrote:
>> > 1) Do we actually need iptables for any reason that isn't a legacy
>> > consideration?
>>
>> No idea personally.
>>
>> Perhaps some users prefer that, and/or need that for integration with
>> other
>> systems/solutions/whatever.
>>
>> If we drop iptables, how do you suggest to treat upgrades?
>>
>> >
>> > 2 & 3) I am in favor of treating custom services as a requirement and
>> > plan
>> > accordingly. Many (most, even) of the services are already provided by
>> > either firewalld itself (e.g. vdsm, libvirt) or the 3rd party packages
>> > (e.g.
>> > gluster). Some are missing (I've recently created a pull request for
>> > ovirt-imageio to firewalld, for example) and I hope we'll be able to get
>> > all
>> > the services to be statically provided (by either firewalld or the
>> > relevant
>> > 3rd party packages).
>> >
>> > Ideally I think we'd like use statically provided services, and provide
>> > the
>> > capability to provide additional services (I'm not a fan of the current
>> > methodology of converting strings into xmls). I don't think we'd want to
>> > limit usage to just statically provided services. (2)
>> >
>> > As previously stated, I don't see a technical reason to keep iptables
>> > under
>> > consideration. (3)
>> >
>> >
>> > On Sun, Mar 26, 2017 at 2:57 PM, Yedidyah Bar David 
>> > wrote:
>> >>
>> >>
>> >> 1. Do we want to support in some version X both iptables and firewalld,
>> >> or
>> >> is it ok to stop support for iptables and support only firewalld
>> >> without
>> >> overlap? If so, do we handle upgrades, and how?
>> >>
>> >> 2. Do we want to support custom firewalld xml to be configured on the
>> >> host by us? Or is it ok to only support choosing among existing
>> >> services,
>> >> which will need to be added to the host using other means (packaged by
>> >> firewalld, packaged by 3rd parties, added manually by users)?
>> >>
>> >> 3. Opposite of (2.): Do we want to support firewalld services that are
>> >> added to the host using other means (see there)? Obviously we do, but:
>> >> If we do, do we still want to support also iptables (see (1.))? And if
>> >> so, what do we want to then happen?
>> >>
>> >> (2.) and (3.) are not conflicting, each needs its own answer.
>> >>
>> >>
>> >> --
>> >> Didi
>> >
>> >
>>
>>
>>
>> --
>> Didi
>
>



-- 
Didi
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel