You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Tanner Danzey <ar...@gmail.com> on 2014/04/15 05:43:45 UTC

Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/
-----------------------------------------------------------

Review request for cloudstack.


Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
    https://issues.apache.org/jira/browse/CLOUDSTACK-5907
    https://issues.apache.org/jira/browse/CLOUDSTACK-6396


Repository: cloudstack-git


Description
-------

Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)


Diffs
-----

  server/src/com/cloud/api/ApiDBUtils.java 67e47f7 

Diff: https://reviews.apache.org/r/20357/diff/


Testing
-------

Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.


Thanks,

Tanner Danzey


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review42151
-----------------------------------------------------------


601827e6b34cb7debe67a8415a09440c389ba4a1 on master
2c4ae0883be04840a90b7238ad32aeaa38cc43ae on 4.4-forward
f8419b9303ea0c81310885acdc5498f6003ca4c6 on 4.4

- daan Hoogland


On April 16, 2014, 3:23 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 16, 2014, 3:23 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.

> On April 16, 2014, 4:02 a.m., Yoshikazu Nojima wrote:
> > It seems getHypervisorTypeFromFormat method is only called from VolumeJoinDaoImpl#newVolumeResponse. 
> > It should be fixed, but does it affect snapshot feature?
> 
> Tanner Danzey wrote:
>     Yes. Without this fix, if you go into the Storage UI panel the volumes show as OVM without any snapshot button.
> 
> Yoshikazu Nojima wrote:
>     I understand. Thank you for explanation.

You're most welcome!


- Tanner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40494
-----------------------------------------------------------


On April 16, 2014, 3:23 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 16, 2014, 3:23 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Yoshikazu Nojima <ma...@ynojima.net>.

> On April 16, 2014, 4:02 a.m., Yoshikazu Nojima wrote:
> > It seems getHypervisorTypeFromFormat method is only called from VolumeJoinDaoImpl#newVolumeResponse. 
> > It should be fixed, but does it affect snapshot feature?
> 
> Tanner Danzey wrote:
>     Yes. Without this fix, if you go into the Storage UI panel the volumes show as OVM without any snapshot button.

I understand. Thank you for explanation.


- Yoshikazu


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40494
-----------------------------------------------------------


On April 16, 2014, 3:23 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 16, 2014, 3:23 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.

> On April 16, 2014, 4:02 a.m., Yoshikazu Nojima wrote:
> > It seems getHypervisorTypeFromFormat method is only called from VolumeJoinDaoImpl#newVolumeResponse. 
> > It should be fixed, but does it affect snapshot feature?

Yes. Without this fix, if you go into the Storage UI panel the volumes show as OVM without any snapshot button.


- Tanner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40494
-----------------------------------------------------------


On April 16, 2014, 3:23 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 16, 2014, 3:23 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Yoshikazu Nojima <ma...@ynojima.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40494
-----------------------------------------------------------


It seems getHypervisorTypeFromFormat method is only called from VolumeJoinDaoImpl#newVolumeResponse. 
It should be fixed, but does it affect snapshot feature?

- Yoshikazu Nojima


On April 16, 2014, 3:23 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 16, 2014, 3:23 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/
-----------------------------------------------------------

(Updated April 16, 2014, 3:23 a.m.)


Review request for cloudstack.


Changes
-------

Nasty typo errors from the past have been removed.


Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
    https://issues.apache.org/jira/browse/CLOUDSTACK-5907
    https://issues.apache.org/jira/browse/CLOUDSTACK-6396


Repository: cloudstack-git


Description
-------

Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)


Diffs (updated)
-----

  server/src/com/cloud/api/ApiDBUtils.java 67e47f7 

Diff: https://reviews.apache.org/r/20357/diff/


Testing
-------

Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.


Thanks,

Tanner Danzey


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/
-----------------------------------------------------------

(Updated April 16, 2014, 1:34 a.m.)


Review request for cloudstack.


Changes
-------

Removed trailing whitespace in comments.


Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
    https://issues.apache.org/jira/browse/CLOUDSTACK-5907
    https://issues.apache.org/jira/browse/CLOUDSTACK-6396


Repository: cloudstack-git


Description
-------

Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)


Diffs (updated)
-----

  server/src/com/cloud/api/ApiDBUtils.java 67e47f7 

Diff: https://reviews.apache.org/r/20357/diff/


Testing
-------

Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.


Thanks,

Tanner Danzey


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/
-----------------------------------------------------------

(Updated April 16, 2014, 1:31 a.m.)


Review request for cloudstack.


Changes
-------

I changed the iteration method to be more proper, using alternating .next() and .prev() calls was hacky and prone to malfunction.


Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
    https://issues.apache.org/jira/browse/CLOUDSTACK-5907
    https://issues.apache.org/jira/browse/CLOUDSTACK-6396


Repository: cloudstack-git


Description
-------

Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)


Diffs (updated)
-----

  server/src/com/cloud/api/ApiDBUtils.java 67e47f7 

Diff: https://reviews.apache.org/r/20357/diff/


Testing
-------

Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.


Thanks,

Tanner Danzey


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by sebgoa <ru...@gmail.com>.
I think it's better if we make one patch per bug..



On Apr 15, 2014, at 5:59 PM, Tanner Danzey <ar...@gmail.com> wrote:

> 
> 
>> On April 15, 2014, 2:52 p.m., daan Hoogland wrote:
>>> server/src/com/cloud/api/ApiDBUtils.java, line 1088
>>> <https://reviews.apache.org/r/20357/diff/1/?file=557802#file557802line1088>
>>> 
>>>    I don't fully understand the logic of querying .next() and then .previous()
>>> 
>>>    It seems like guessing or is there some hard restrained that makes this the way to determine this is KVM.
>>> 
>>>    Is this really the place to decide on hypervisor type?
>> 
>> Tanner Danzey wrote:
>>    I had nearly finished a somewhat wordy reply when my tablet decided to reboot itself, so please forgive me if I seem brief in reply this time around.
>> 
>>    1) From what I understand of Iterators, using .next() and .previous() in succession allow iterating over the same element twice. If this is not so, it's an easy enough fix.
>> 
>>    2&3) This is a guess, yes. There is a similar guess above for deciding whether VHD should be associated with HyperV or Xenserver because of a similar situation (2 hypervisors, 1 format vs. 1 hypervisor, two formats) so this seemed like a logical place to put a similar fix. As illustrated in CS-6396, for the function getHypervisorTypeFromFormat() there is only one possible return for each format, which is not true for KVM as it can support RAW images (in the case of RBD or CLVM) as well as qcow2 images. The only thing I can think of that would remedy this is implementing a way to return multiple supported types of formats for hypervisors OR a separate format for RBD raw images & CLVM raw images, but that's a fair bit more legwork than I feel comfortable doing being as I am relatively new to the codebase.
>> 
>>    Another way to patch this that I thought of while writing this reply would be to check the datacenter for KVM and OVM clusters, and for example the presence of OVM clusters but not KVM clusters would indicate that RAW images are for OVM, whereas in the presence of KVM clusters without OVM cluststers RAW images could be assigned to KVM for RBD / CLVM.
>> 
>>    Let me know what you think, I hope I answered all your questions.
> 
> I just did some additional reading and there is a better way to do that loop, I will have to wait until I'm at my home computer to submit a revision.
> 
> 
> - Tanner
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/#review40394
> -----------------------------------------------------------
> 
> 
> On April 15, 2014, 3:43 a.m., Tanner Danzey wrote:
>> 
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/20357/
>> -----------------------------------------------------------
>> 
>> (Updated April 15, 2014, 3:43 a.m.)
>> 
>> 
>> Review request for cloudstack.
>> 
>> 
>> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>>    https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>>    https://issues.apache.org/jira/browse/CLOUDSTACK-6396
>> 
>> 
>> Repository: cloudstack-git
>> 
>> 
>> Description
>> -------
>> 
>> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
>> 
>> 
>> Diffs
>> -----
>> 
>>  server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
>> 
>> Diff: https://reviews.apache.org/r/20357/diff/
>> 
>> 
>> Testing
>> -------
>> 
>> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
>> 
>> 
>> Thanks,
>> 
>> Tanner Danzey
>> 
>> 
> 


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.

> On April 15, 2014, 2:52 p.m., daan Hoogland wrote:
> > server/src/com/cloud/api/ApiDBUtils.java, line 1088
> > <https://reviews.apache.org/r/20357/diff/1/?file=557802#file557802line1088>
> >
> >     I don't fully understand the logic of querying .next() and then .previous()
> >     
> >     It seems like guessing or is there some hard restrained that makes this the way to determine this is KVM.
> >     
> >     Is this really the place to decide on hypervisor type?
> 
> Tanner Danzey wrote:
>     I had nearly finished a somewhat wordy reply when my tablet decided to reboot itself, so please forgive me if I seem brief in reply this time around.
>     
>     1) From what I understand of Iterators, using .next() and .previous() in succession allow iterating over the same element twice. If this is not so, it's an easy enough fix.
>     
>     2&3) This is a guess, yes. There is a similar guess above for deciding whether VHD should be associated with HyperV or Xenserver because of a similar situation (2 hypervisors, 1 format vs. 1 hypervisor, two formats) so this seemed like a logical place to put a similar fix. As illustrated in CS-6396, for the function getHypervisorTypeFromFormat() there is only one possible return for each format, which is not true for KVM as it can support RAW images (in the case of RBD or CLVM) as well as qcow2 images. The only thing I can think of that would remedy this is implementing a way to return multiple supported types of formats for hypervisors OR a separate format for RBD raw images & CLVM raw images, but that's a fair bit more legwork than I feel comfortable doing being as I am relatively new to the codebase.
>     
>     Another way to patch this that I thought of while writing this reply would be to check the datacenter for KVM and OVM clusters, and for example the presence of OVM clusters but not KVM clusters would indicate that RAW images are for OVM, whereas in the presence of KVM clusters without OVM cluststers RAW images could be assigned to KVM for RBD / CLVM.
>     
>     Let me know what you think, I hope I answered all your questions.

I just did some additional reading and there is a better way to do that loop, I will have to wait until I'm at my home computer to submit a revision.


- Tanner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40394
-----------------------------------------------------------


On April 15, 2014, 3:43 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 3:43 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by Tanner Danzey <ar...@gmail.com>.

> On April 15, 2014, 2:52 p.m., daan Hoogland wrote:
> > server/src/com/cloud/api/ApiDBUtils.java, line 1088
> > <https://reviews.apache.org/r/20357/diff/1/?file=557802#file557802line1088>
> >
> >     I don't fully understand the logic of querying .next() and then .previous()
> >     
> >     It seems like guessing or is there some hard restrained that makes this the way to determine this is KVM.
> >     
> >     Is this really the place to decide on hypervisor type?

I had nearly finished a somewhat wordy reply when my tablet decided to reboot itself, so please forgive me if I seem brief in reply this time around.

1) From what I understand of Iterators, using .next() and .previous() in succession allow iterating over the same element twice. If this is not so, it's an easy enough fix.

2&3) This is a guess, yes. There is a similar guess above for deciding whether VHD should be associated with HyperV or Xenserver because of a similar situation (2 hypervisors, 1 format vs. 1 hypervisor, two formats) so this seemed like a logical place to put a similar fix. As illustrated in CS-6396, for the function getHypervisorTypeFromFormat() there is only one possible return for each format, which is not true for KVM as it can support RAW images (in the case of RBD or CLVM) as well as qcow2 images. The only thing I can think of that would remedy this is implementing a way to return multiple supported types of formats for hypervisors OR a separate format for RBD raw images & CLVM raw images, but that's a fair bit more legwork than I feel comfortable doing being as I am relatively new to the codebase.

Another way to patch this that I thought of while writing this reply would be to check the datacenter for KVM and OVM clusters, and for example the presence of OVM clusters but not KVM clusters would indicate that RAW images are for OVM, whereas in the presence of KVM clusters without OVM cluststers RAW images could be assigned to KVM for RBD / CLVM.

Let me know what you think, I hope I answered all your questions.


- Tanner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40394
-----------------------------------------------------------


On April 15, 2014, 3:43 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 3:43 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>


Re: Review Request 20357: [CS-5907] [CS-6396] Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)

Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20357/#review40394
-----------------------------------------------------------



server/src/com/cloud/api/ApiDBUtils.java
<https://reviews.apache.org/r/20357/#comment73374>

    I don't fully understand the logic of querying .next() and then .previous()
    
    It seems like guessing or is there some hard restrained that makes this the way to determine this is KVM.
    
    Is this really the place to decide on hypervisor type?


- daan Hoogland


On April 15, 2014, 3:43 a.m., Tanner Danzey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20357/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 3:43 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Bugs: CLOUDSTACK-5907 and CLOUDSTACK-6396
>     https://issues.apache.org/jira/browse/CLOUDSTACK-5907
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6396
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for RBD (and potentially CLVM) volumes w/ KVM incorrectly identified as OVM volumes (not able to snapshot)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/api/ApiDBUtils.java 67e47f7 
> 
> Diff: https://reviews.apache.org/r/20357/diff/
> 
> 
> Testing
> -------
> 
> Applied on otherwise clean 4.4 branch and tested in a live testing environment with KVM hypervisors and RBD primary storage pool that would otherwise identify as OVM. No errors and no weird behavior. Just the expected result.
> 
> 
> Thanks,
> 
> Tanner Danzey
> 
>