dheeraj12347 commented on PR #14694: URL: https://github.com/apache/cloudstack/pull/14694#issuecomment-6831788761
> Thanks for picking this up. The fix itself looks right to me: decrementing in a `finally` covers the `CloudRuntimeException` from `getStoragePool()`, and gating it on `refCountIncremented` also stops the old catch from decrementing a primary pool that was never counted. > > One problem: this depends on #14189 but targets 4.22 without it. `createStoragePoolLocked` only exists in #14189, and `incStoragePoolRefCount`/`decStoragePoolRefCount` are private on 4.22 (#14189 makes them protected). Applied to 4.22 alone, `LibvirtStorageAdaptorTest` doesn't compile: > > ``` > LibvirtStorageAdaptorTest.java:[197,19] error: cannot find symbol > symbol: method createStoragePoolLocked(String,String,int,String,<null>,StoragePoolType,<null>,boolean) > LibvirtStorageAdaptorTest.java:[210,35] error: incStoragePoolRefCount(String) has private access in LibvirtStorageAdaptor > LibvirtStorageAdaptorTest.java:[211,35] error: decStoragePoolRefCount(String) has private access in LibvirtStorageAdaptor > ``` > > On top of #14189, the main change applies cleanly but the test file conflicts with the tests #14189 adds there. > > Two ways to go: rebase this onto 4.22 once #14189 merges, which is the simplest; or make it standalone now by testing through the public `createStoragePool(...)` and checking the ref count through `storagePoolRefCounts` rather than spying on the private methods. Either works for me. Thanks for pointing that out, @bhouse-nexthop! I’ll wait for #14189 to get merged and then update my PR on top of it. Appreciate you taking a look! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
