cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [4.6] RC1 soon ?
Date Thu, 30 Jul 2015 20:00:34 GMT
Thanks Mike,

I was thinking that the order of applying the statement parameters was
reversed. Maybe you can try and apply this snippit:

diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
index d3c29f7..c388da6 100644
--- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
@@ -399,17 +399,16 @@
             sql.append(ZoneWideDetailsSqlSuffix);
             TransactionLegacy txn = TransactionLegacy.currentTxn();
             try (PreparedStatement pstmt =
txn.prepareStatement(sql.toString());){
-                int i=0;
-                for (Map.Entry<String, String> detail : details.entrySet()) {
-                    pstmt.setString(++i,detail.getKey());
-                    pstmt.setString(++i,detail.getValue());
-                }
                 List<StoragePoolVO> pools = new ArrayList<StoragePoolVO>();
                 if (pstmt != null) {
-                    i = 1;
+                    int i = 1;
                     pstmt.setLong(i++, dcId);
                     pstmt.setString(i++, ScopeType.ZONE.toString());
                     pstmt.setInt(i++, details.size());
+                    for (Map.Entry<String, String> detail :
details.entrySet()) {
+                        pstmt.setString(i++,detail.getKey());
+                        pstmt.setString(i++,detail.getValue());
+                    }
                     try(ResultSet rs = pstmt.executeQuery();) {
                         while (rs.next()) {
                             pools.add(toEntityBean(rs, false));

On Thu, Jul 30, 2015 at 9:54 PM, Mike Tutkowski
<mike.tutkowski@solidfire.com> wrote:
> I'm doing other testing locally anyways. If you happen to find a fix, then I
> won't bother to revert. I can just try it again with your fix. I'll wait
> until tonight MST to revert.
>
> On Thu, Jul 30, 2015 at 1:51 PM, Daan Hoogland <daan.hoogland@gmail.com>
> wrote:
>>
>> leasure time is on
>>
>> On Thu, Jul 30, 2015 at 9:44 PM, Mike Tutkowski
>> <mike.tutkowski@solidfire.com> wrote:
>> > Sure, I can revert it and leave it for your leisure to find an
>> > acceptable
>> > fix to satisfy FindBugs.
>> >
>> > On Thu, Jul 30, 2015 at 1:42 PM, Daan Hoogland <daan.hoogland@gmail.com>
>> > wrote:
>> >>
>> >> I am now thinking of how to isolate this code to write a proper test.
>> >> This is not going to be successful tonight, while the original seven
>> >> samurai is on tv. Maybe reverting is the best option and we live with
>> >> findbugs regression for a day. I will think of a way to fix this
>> >> tomorrow wit a more clear head.
>> >>
>> >> On Thu, Jul 30, 2015 at 9:33 PM, Daan Hoogland
>> >> <daan.hoogland@gmail.com>
>> >> wrote:
>> >> > But reintroduced the vulnerability that findbugs was complaining
>> >> > about...
>> >> > I think the problem is in this bit:
>> >> >
>> >> >                 int i=0;
>> >> >                 for (Map.Entry<String, String> detail :
>> >> > details.entrySet()) {
>> >> >                     pstmt.setString(++i,detail.getKey());
>> >> >                     pstmt.setString(++i,detail.getValue());
>> >> >                 }
>> >> >
>> >> > ++i should have been i++ in both cases. I messed those in my review,
>> >> > sorry
>> >> >
>> >> >
>> >> > On Thu, Jul 30, 2015 at 9:24 PM, Mike Tutkowski
>> >> > <mike.tutkowski@solidfire.com> wrote:
>> >> >> No problem, Daan. :)
>> >> >>
>> >> >> Just from an empirical standpoint, though, reverting the commit
in
>> >> >> my
>> >> >> local
>> >> >> repo fixed the problem.
>> >> >>
>> >> >> On Thu, Jul 30, 2015 at 1:20 PM, Daan Hoogland
>> >> >> <daan.hoogland@gmail.com>
>> >> >> wrote:
>> >> >>>
>> >> >>> never mind that again, answerring to fast as I probably do
one out
>> >> >>> of
>> >> >>> two or three times :( Looking further...
>> >> >>>
>> >> >>> On Thu, Jul 30, 2015 at 9:19 PM, Daan Hoogland
>> >> >>> <daan.hoogland@gmail.com>
>> >> >>> wrote:
>> >> >>> > Mike, I am looking at the commit and it makes perfect
sense as
>> >> >>> > the
>> >> >>> > the
>> >> >>> > prior situation was creating a prepared statement based
on
>> >> >>> > dynamicly
>> >> >>> > added strings. The only queer thing is that the int i
= 1 is
>> >> >>> > replaced
>> >> >>> > with i = 1, reusing the loop counter instead of hiding
it. Maybe
>> >> >>> > this
>> >> >>> > is the problem
>> >> >>> >
>> >> >>> > On Thu, Jul 30, 2015 at 8:55 PM, Mike Tutkowski
>> >> >>> > <mike.tutkowski@solidfire.com> wrote:
>> >> >>> >> I see the problem.
>> >> >>> >>
>> >> >>> >> The way a SQL statement was constructed was changed
by commit
>> >> >>> >> b84093f691ae0b09d2c525d50f2e2d200c709b2c:
>> >> >>> >>
>> >> >>> >>
>> >> >>> >>
>> >> >>> >>
>> >> >>> >> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blobdiff;f=engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java;h=d3c29f70d6ab56379c2436b5cafc933049200f31;hp=faf5291554a68506b14438a7e1cda61cd4c3cc0c;hb=b84093f691ae0b09d2c525d50f2e2d200c709b2c;hpb=1407033cc2e0742653d82bb0181c041b31253693
>> >> >>> >>
>> >> >>> >> and no longer makes any sense
>> >> >>> >>
>> >> >>> >> SELECT storage_pool.* from storage_pool LEFT JOIN
>> >> >>> >> storage_pool_details
>> >> >>> >> ON
>> >> >>> >> storage_pool.id = storage_pool_details.pool_id WHERE
>> >> >>> >> storage_pool.removed is
>> >> >>> >> null and storage_pool.status = 'Up' and
>> >> >>> >> storage_pool.data_center_id
>> >> >>> >> = 1
>> >> >>> >> and
>> >> >>> >> storage_pool.scope = 'ZONE' and (((storage_pool_details.name=1)
>> >> >>> >> AND
>> >> >>> >> (storage_pool_details.value=** NOT SPECIFIED **)))
GROUP BY
>> >> >>> >> storage_pool_details.pool_id HAVING
>> >> >>> >> COUNT(storage_pool_details.name) >=
>> >> >>> >> **
>> >> >>> >> NOT SPECIFIED **
>> >> >>> >>
>> >> >>> >> I think I'm just going to revert this commit. It was
related to
>> >> >>> >> a
>> >> >>> >> change put
>> >> >>> >> in at the request of FindBugs.
>> >> >>> >>
>> >> >>> >> I've CCed the relevant participants in the commit
in case they
>> >> >>> >> wish
>> >> >>> >> to
>> >> >>> >> re-evaluate the FindBugs request and resubmit a fix.
>> >> >>> >>
>> >> >>> >>
>> >> >>> >> On Thu, Jul 30, 2015 at 12:13 PM, Mike Tutkowski
>> >> >>> >> <mike.tutkowski@solidfire.com> wrote:
>> >> >>> >>>
>> >> >>> >>> FYI that I get the same error message when trying
to attach a
>> >> >>> >>> data
>> >> >>> >>> disk
>> >> >>> >>> that is based on zone-wide primary storage.
>> >> >>> >>>
>> >> >>> >>> On Thu, Jul 30, 2015 at 12:10 PM, Mike Tutkowski
>> >> >>> >>> <mike.tutkowski@solidfire.com> wrote:
>> >> >>> >>>>
>> >> >>> >>>> I'm actually having trouble creating a VM
whose root disk runs
>> >> >>> >>>> on
>> >> >>> >>>> zone-wide primary storage.
>> >> >>> >>>>
>> >> >>> >>>> This is definitely a blocker for 4.6. I'm
just beginning to
>> >> >>> >>>> look
>> >> >>> >>>> into
>> >> >>> >>>> this, but this is the error message I receive:
>> >> >>> >>>>
>> >> >>> >>>> findZoneWideStoragePoolsByTags:Exception:No
value specified
>> >> >>> >>>> for
>> >> >>> >>>> parameter
>> >> >>> >>>> 4
>> >> >>> >>>>
>> >> >>> >>>> On Mon, Jul 27, 2015 at 8:51 PM, Pierre-Luc
Dion
>> >> >>> >>>> <pdion891@apache.org>
>> >> >>> >>>> wrote:
>> >> >>> >>>>>
>> >> >>> >>>>> Hi,
>> >> >>> >>>>>
>> >> >>> >>>>> I've create this jira filter[1] for the
Release Manager,
>> >> >>> >>>>> based
>> >> >>> >>>>> on
>> >> >>> >>>>> it,
>> >> >>> >>>>> there
>> >> >>> >>>>> would be only 4 maybe just 3 blockers
on 4.6. Based on this,
>> >> >>> >>>>> should
>> >> >>> >>>>> we
>> >> >>> >>>>> consider placing a target date for RC1
somewhere like end of
>> >> >>> >>>>> August
>> >> >>> >>>>> ?
>> >> >>> >>>>>
>> >> >>> >>>>> What's missing and to do in 4.6 as far
as I know:
>> >> >>> >>>>>
>> >> >>> >>>>> 1. Basic documentation of new features,
>> >> >>> >>>>> 2. Decide if we let it in master and freeze
merge during RC ?
>> >> >>> >>>>> 3. Build all job as 4.6 in jenkins ?
>> >> >>> >>>>> 4. Organize a QA-party, like old time
lan-party
>> >> >>> >>>>>
>> >> >>> >>>>> [1] https://issues.apache.org/jira/issues/?filter=12332940
>> >> >>> >>>>
>> >> >>> >>>>
>> >> >>> >>>>
>> >> >>> >>>>
>> >> >>> >>>> --
>> >> >>> >>>> Mike Tutkowski
>> >> >>> >>>> Senior CloudStack Developer, SolidFire Inc.
>> >> >>> >>>> e: mike.tutkowski@solidfire.com
>> >> >>> >>>> o: 303.746.7302
>> >> >>> >>>> Advancing the way the world uses the cloud™
>> >> >>> >>>
>> >> >>> >>>
>> >> >>> >>>
>> >> >>> >>>
>> >> >>> >>> --
>> >> >>> >>> Mike Tutkowski
>> >> >>> >>> Senior CloudStack Developer, SolidFire Inc.
>> >> >>> >>> e: mike.tutkowski@solidfire.com
>> >> >>> >>> o: 303.746.7302
>> >> >>> >>> Advancing the way the world uses the cloud™
>> >> >>> >>
>> >> >>> >>
>> >> >>> >>
>> >> >>> >>
>> >> >>> >> --
>> >> >>> >> Mike Tutkowski
>> >> >>> >> Senior CloudStack Developer, SolidFire Inc.
>> >> >>> >> e: mike.tutkowski@solidfire.com
>> >> >>> >> o: 303.746.7302
>> >> >>> >> Advancing the way the world uses the cloud™
>> >> >>> >
>> >> >>> >
>> >> >>> >
>> >> >>> > --
>> >> >>> > Daan
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> --
>> >> >>> Daan
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Mike Tutkowski
>> >> >> Senior CloudStack Developer, SolidFire Inc.
>> >> >> e: mike.tutkowski@solidfire.com
>> >> >> o: 303.746.7302
>> >> >> Advancing the way the world uses the cloud™
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Daan
>> >>
>> >>
>> >>
>> >> --
>> >> Daan
>> >
>> >
>> >
>> >
>> > --
>> > Mike Tutkowski
>> > Senior CloudStack Developer, SolidFire Inc.
>> > e: mike.tutkowski@solidfire.com
>> > o: 303.746.7302
>> > Advancing the way the world uses the cloud™
>>
>>
>>
>> --
>> Daan
>
>
>
>
> --
> Mike Tutkowski
> Senior CloudStack Developer, SolidFire Inc.
> e: mike.tutkowski@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the cloud™



-- 
Daan

Mime
View raw message