{"action":"created","issue":{"url":"https://api.github.com/repos/apache/cloudstack/issues/1441","repository_url":"https://api.github.com/repos/apache/cloudstack","labels_url":"https://api.github.com/repos/apache/cloudstack/issues/1441/labels{/name}","comments_url":"https://api.github.com/repos/apache/cloudstack/issues/1441/comments","events_url":"https://api.github.com/repos/apache/cloudstack/issues/1441/events","html_url":"https://github.com/apache/cloudstack/pull/1441","id":141173296,"number":1441,"title":"CLOUDSTACK-9297 - Reworked logic in StorageSystemSnapshotStrategy and XenserverSnapshotStrategy","user":{"login":"mike-tutkowski","id":3019295,"avatar_url":"https://avatars.githubusercontent.com/u/3019295?v=3","gravatar_id":"","url":"https://api.github.com/users/mike-tutkowski","html_url":"https://github.com/mike-tutkowski","followers_url":"https://api.github.com/users/mike-tutkowski/followers","following_url":"https://api.github.com/users/mike-tutkowski/following{/other_user}","gists_url":"https://api.github.com/users/mike-tutkowski/gists{/gist_id}","starred_url":"https://api.github.com/users/mike-tutkowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mike-tutkowski/subscriptions","organizations_url":"https://api.github.com/users/mike-tutkowski/orgs","repos_url":"https://api.github.com/users/mike-tutkowski/repos","events_url":"https://api.github.com/users/mike-tutkowski/events{/privacy}","received_events_url":"https://api.github.com/users/mike-tutkowski/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2016-03-16T04:57:48Z","updated_at":"2016-03-17T16:12:21Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apache/cloudstack/pulls/1441","html_url":"https://github.com/apache/cloudstack/pull/1441","diff_url":"https://github.com/apache/cloudstack/pull/1441.diff","patch_url":"https://github.com/apache/cloudstack/pull/1441.patch"},"body":"The ticket this PR fixes was opened because KVM-specific code had been added to the StorageSystemSnapshotStrategy class and that class' canHandle method was only prepared to handle managed storage being used with XenServer (and a case was hit for KVM that triggered a CloudRuntimeException to be thrown).\r\n\r\nTo solve the problem, I moved the KVM logic to the default snapshot strategy class, which is (unfortunately) named XenserverSnapshotStrategy.\r\n\r\nI plan to rename XenserverSnapshotStrategy to something like DefaultSnapshotStrategy in 4.9.\r\n\r\nMy guess is that when XenserverSnapshotStrategy was originally written, it was written only for XenServer, but has since that time had its usage increased to support other hypervisors (with non-managed storage)."},"comment":{"url":"https://api.github.com/repos/apache/cloudstack/issues/comments/197952462","html_url":"https://github.com/apache/cloudstack/pull/1441#issuecomment-197952462","issue_url":"https://api.github.com/repos/apache/cloudstack/issues/1441","id":197952462,"user":{"login":"mike-tutkowski","id":3019295,"avatar_url":"https://avatars.githubusercontent.com/u/3019295?v=3","gravatar_id":"","url":"https://api.github.com/users/mike-tutkowski","html_url":"https://github.com/mike-tutkowski","followers_url":"https://api.github.com/users/mike-tutkowski/followers","following_url":"https://api.github.com/users/mike-tutkowski/following{/other_user}","gists_url":"https://api.github.com/users/mike-tutkowski/gists{/gist_id}","starred_url":"https://api.github.com/users/mike-tutkowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mike-tutkowski/subscriptions","organizations_url":"https://api.github.com/users/mike-tutkowski/orgs","repos_url":"https://api.github.com/users/mike-tutkowski/repos","events_url":"https://api.github.com/users/mike-tutkowski/events{/privacy}","received_events_url":"https://api.github.com/users/mike-tutkowski/received_events","type":"User","site_admin":false},"created_at":"2016-03-17T16:12:21Z","updated_at":"2016-03-17T16:12:21Z","body":"Maybe we're looking at different code?\r\n\r\nI see the canHandle method being used from StorageStrategyFactoryImpl.getSnapshotStrategy.\r\n\r\nIt has a method, bestMatch, that it makes use of to pick a snapshot strategy with the ordinal value."}}