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
