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