On May 29, 2014, at 11:20 AM, Rajani Karuturi <rajani.karut...@citrix.com>
wrote:
> The fix is already in 4.4
>
> commit 3a3457e7132d22f52aa38179d40a6eb9b0b29677
> Author: Sam Schmit <sam.sch...@appcore.com>
> AuthorDate: Fri May 2 13:07:34 2014 -0500
> Commit: Daan Hoogland <d...@onecht.net>
> CommitDate: Tue May 6 17:46:20 2014 +0200
>
> CLOUDSTACK-6472 listUsageRecords: Pull information from removed items as
> well, fixing NPEs/Null UUIDs with usage API calls.
>
> M server/src/com/cloud/api/ApiResponseHelper.java
>
still need to check if this fix in 4.4 is complete. Hiroki's change was to 4.3
branch. I did not check 4.4.
>
> @sebgoa
> i think it would be better to include bug id in the commit messages as thats
> the only way to track all the commits for a defect.
>
yes my bad, did not take time to look up the bug id.
>
> ~Rajani
>
>
>
> On 29-May-2014, at 2:25 pm, Daan Hoogland <daan.hoogl...@gmail.com> wrote:
>
>> Hiroki,
>>
>> Did you have a look at 4.4 or master as well? tell us when it works,
>> so we can cherry-pick the fix to later versions.
>>
>> On Thu, May 29, 2014 at 10:25 AM, Hiroki Ohashi <hiroki.s...@gmail.com>
>> wrote:
>>> sebgoa
>>>
>>>
>>> 2014-05-29 16:52 GMT+09:00 sebgoa <run...@gmail.com>:
>>>>
>>>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hiroki.s...@gmail.com> wrote:
>>>>
>>>>> Dear guys
>>>>>
>>>>> I found a bug about listUsageRecords API in 4.3 related to a issue
>>>>> CLOUDSTACK-6472 and made a patch. Would you review the patch and
>>>>> correct me if I am wrong?
>>>>>
>>>>> Sam Schmit push a commit about NPEs/Null UUIDs at May 5th but I think
>>>>> his commit is not enough.
>>>>>
>>>>> commit e8047e11db7ef2bdc44bbedfdc47e63c55f080c5
>>>>> Author: Sam Schmit <sam.sch...@appcore.com>
>>>>> Date: Mon May 5 16:38:23 2014 -0500
>>>>>
>>>>> CLOUDSTACK-6472 (4.3 specific) listUsageRecords: Pull
>>>>> information from removed items as well, fixing NPEs/Null UUIDs with
>>>>> usage API calls.
>>>>>
>>>>> Signed-off-by: Sebastien Goasguen <run...@gmail.com>
>>>>>
>>>>> When I called listUsageRecords against CloudStack 4.3 management
>>>>> server that was build at latest 4.3 branch, usage records about
>>>>> destroyed instances of usage type 1 and 2 lacked 'virtualmachineid' as
>>>>> below.
>>>>>
>>>>> {
>>>>> "account": "sgcadm",
>>>>> "accountid": "f9e4e1ca-69fd-4ae3-b70c-15bbcc13406e",
>>>>> "description": "mycluster-front allocated (ServiceOffering: 13)
>>>>> (Template: 203)",
>>>>> "domainid": "84dd635d-fb99-4895-b199-7d777aa144d5",
>>>>> "enddate": "2014-05-20'T'08:59:59+09:00",
>>>>> "name": "mycluster-front",
>>>>> "offeringid": "e5137018-8fca-4e4a-a79e-9e4d1707e079",
>>>>> "rawusage": "0.223611",
>>>>> "startdate": "2014-05-19'T'09:00:00+09:00",
>>>>> "templateid": "1ebdc71e-dcbe-4d3e-82d3-d31897f78d4c",
>>>>> "type": "KVM",
>>>>> "usage": "0.223611 Hrs",
>>>>> "usageid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
>>>>> "usagetype": 2,
>>>>> "zoneid": "c2ba1354-0c15-4284-bdba-4f201767362d"
>>>>> },
>>>>>
>>>>> Hence, it is unable to refer to any destroyed instance id. I patched
>>>>> createUsageResponse method like this.
>>>>>
>>>>> diff --git a/server/src/com/cloud/api/ApiResponseHelper.java
>>>>> b/server/src/com/cloud/api/ApiResponseHelper.java
>>>>> index 7718fc1..cbf3914 100755
>>>>> --- a/server/src/com/cloud/api/ApiResponseHelper.java
>>>>> +++ b/server/src/com/cloud/api/ApiResponseHelper.java
>>>>> @@ -3304,7 +3304,7 @@ public class ApiResponseHelper implements
>>>>> ResponseGenerator {
>>>>> usageRecResponse.setUsage(usageRecord.getUsageDisplay());
>>>>> usageRecResponse.setUsageType(usageRecord.getUsageType());
>>>>> if (usageRecord.getVmInstanceId() != null) {
>>>>> - VMInstanceVO vm =
>>>>> _entityMgr.findById(VMInstanceVO.class,
>>>>> usageRecord.getVmInstanceId());
>>>>> + VMInstanceVO vm =
>>>>> _entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
>>>>> usageRecord.getVmInstanceId());
>>>>> if (vm != null) {
>>>>> usageRecResponse.setVirtualMachineId(vm.getUuid());
>>>>> }
>>>>>
>>>>
>>>> Looks like an omission to me, I went ahead and patched it:
>>>>
>>>> commit 24e03bed560feb959a61126b76c2d6971e77092e
>>>> Author: Sebastien Goasguen <run...@gmail.com>
>>>> Date: Thu May 29 09:48:46 2014 +0200
>>>>
>>>> Fix usage response, patch by Hiroki Ohashi
>>>>
>>>> We can always revert if people don't agree.
>>>>
>>>> Can you pull the latest 4.3 and try it.
>>>>
>>>
>>> Thanks! I will try it.
>>>
>>>> Thanks for the patch. you can always attach a patch at
>>>> http://reviews.apache.org
>>>> and follow the instructions at http://cloudstack.apache.org/developers.html
>>>
>>> OK, I will follow the instruction next time.
>>>
>>>
>>>>> After that, listUsageRecords API contains 'virtualmachineid' whether
>>>>> or not instances are destroyed. Here is a result.
>>>>>
>>>>> {
>>>>> "account": "sgcadm",
>>>>> "accountid": "f9e4e1ca-69fd-4ae3-b70c-15bbcc13406e",
>>>>> "description": "mycluster-front allocated (ServiceOffering: 13)
>>>>> (Template: 203)",
>>>>> "domainid": "84dd635d-fb99-4895-b199-7d777aa144d5",
>>>>> "enddate": "2014-05-20'T'08:59:59+09:00",
>>>>> "name": "mycluster-front",
>>>>> "offeringid": "e5137018-8fca-4e4a-a79e-9e4d1707e079",
>>>>> "rawusage": "0.223611",
>>>>> "startdate": "2014-05-19'T'09:00:00+09:00",
>>>>> "templateid": "1ebdc71e-dcbe-4d3e-82d3-d31897f78d4c",
>>>>> "type": "KVM",
>>>>> "usage": "0.223611 Hrs",
>>>>> "usageid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
>>>>> "usagetype": 2,
>>>>> "virtualmachineid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
>>>>> "zoneid": "c2ba1354-0c15-4284-bdba-4f201767362d"
>>>>> },
>>>>>
>>>>> I think this is an expectecd behaviour, so please review and test this
>>>>> patch.
>>>>>
>>>>>
>>>>> Best Regards
>>>>>
>>>>> --
>>>>> Hiroki Ohashi
>>>>
>>>
>>>
>>> Best Regards
>>>
>>> --
>>> Hiroki Ohashi
>>
>>
>>
>> --
>> Daan
>
