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