cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tanner Danzey" <arkan...@gmail.com>
Subject 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)
Date Tue, 15 Apr 2014 15:59:11 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message