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 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
> 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
