[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-175667434
  
Merging as it has 2LGTMs


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/830


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-174510528
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162453199
  
Though I would like to see how this situation is reproduced, the code is 
fine and sensible.

LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[sureshanaparti]

Github user sureshanaparti commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162493826
  
@ustcweizhou , Same with my result set as well.
mysql> select * from vm_instance where state is null;
Empty set (0.00 sec)

mysql> SELECT id, name, volume_type, state, vm_state FROM cloud.volume_view 
where vm_state is null and vm_id is not null;
Empty set (0.00 sec)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[ustcweizhou]

Github user ustcweizhou commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162473175
  
I have the same concern with Daan. How to reproduce this issue ?
Check my env

mysql> select * from volumes where volume_type is null or provisioning_type 
is null or state is null;
Empty set (0.00 sec)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[sureshanaparti]

Github user sureshanaparti commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162483571
  
@DaanHoogland , @ustcweizhou 
This issue is reproduced when the volume associated vm instance has null or 
invalid state (vm_state in volume_view). The code changes would guard this 
situation since it should not block volume listing.

mysql> SELECT id, name, volume_type, state, vm_state FROM cloud.volume_view 
where vm_state is null;
++--+-+--+--+
| id | name | volume_type | state| vm_state |
++--+-+--+--+
| 57 | ROOT-57  | ROOT| Expunged | NULL |
| 69 | mytestvolume | DATADISK| Ready| NULL |
| 71 | DATA-69  | DATADISK| Expunged | NULL |
| 73 | DATA-70  | DATADISK| Expunged | NULL |
| 78 | testvolcreated   | DATADISK| Ready| NULL |
| 81 | test2volume  | DATADISK| Destroy  | NULL |
| 83 | testvol07| DATADISK| Ready| NULL |
| 87 | testattachvolcreated | DATADISK| Ready| NULL |
++--+-+--+--+
8 rows in set (0.00 sec)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162484552
  
@sureshanaparti How do we force such a situation to occur?
It seems to me that should be guarded on input and alerted on on output 
instead of silently filtered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[sureshanaparti]

Github user sureshanaparti commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162494808
  
The volumes, in Ready state and are not attached to any VM have their 
vm_state has null and querying such volumes may result in NPE. These changes 
are only to safe guard.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[ustcweizhou]

Github user ustcweizhou commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162486767
  
@sureshanaparti 

mysql> SELECT id, name, volume_type, state, vm_state FROM cloud.volume_view 
where vm_state is null and vm_id is not null;
Empty set (0.00 sec)

even in vm_instance

mysql> select * from vm_instance where state is null;
Empty set (0.00 sec)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[ustcweizhou]

Github user ustcweizhou commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162520618
  
@sureshanaparti 
I do not get NPE when listVolumes.

please notice the code:
long instanceId = volume.getVmId();
if (instanceId > 0 && volume.getState() != Volume.State.Destroy) {
...

if the volume is not attached, this judgment returns false.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[sureshanaparti]

Github user sureshanaparti commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162612642
  
@ustcweizhou , Yes. I agree. In case on instance id, vm_state is not null.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/830#issuecomment-162345934
  
PIng @DaanHoogland can you review this please? I'll run some tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8858: listVolumes API fails fo...

[sureshanaparti]

GitHub user sureshanaparti opened a pull request:

https://github.com/apache/cloudstack/pull/830

CLOUDSTACK-8858: listVolumes API fails for a particular domain with NPE.

CLOUDSTACK-8858: listVolumes API fails for a particular domain with NPE.

Summary: listVolumes API fails when volume associated vm instance has NULL 
or invalid state. Fix the code to guard this situation since this should not 
block volume listing.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sureshanaparti/cloudstack CLOUDSTACK-8858

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/830.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #830


commit c3777632c5817cc1e635ee59eb18d6cb4f210e29
Author: Suresh Kumar Anaparti 
Date:   2015-09-15T10:52:06Z

CLOUDSTACK-8858: listVolumes API fails for a particular domain with NPE.

Summary: listVolumes API fails when volume associated vm instance has NULL 
or invalid state. Fix the code to guard this situation since this should not 
block volume listing.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---