Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-138426187
@ustcweizhou this caused a new coverity issue. Can you check?
```
411 SnapshotInfo snapshotOnPrimaryStore =
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-136708467
No, seems good. We can merge this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-136705849
any other comment about this PR ?
If no, I will merge it into master.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user ustcweizhou closed the pull request at:
https://github.com/apache/cloudstack/pull/732
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-135661166
@bhaisaab I updated this PR . The reponses is changed to SnapshotResponse
for backward compatibility.
@karuturi I will add some unit tests for snapshot
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-135667378
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ustcweizhou commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/732#discussion_r37963222
--- Diff:
api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java
---
@@ -31,25 +31,36 @@
import
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-134820562
Can you add some tests please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user milamberspace commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-134553590
LGTM to me too.
Tested on a master test deployment (Ubuntu 14.04 / KVM / NFS)
Success with a manual snapshot from cloudmonkey and web ui
Github user bhaisaab commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/732#discussion_r37854560
--- Diff:
api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java
---
@@ -31,25 +31,36 @@
import
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-134558876
In general LGTM, please see if you could fix the response class and object
of the API (pl see the comments).
---
If your project is set up for it, you can reply
Github user bhaisaab commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/732#discussion_r37854506
--- Diff:
api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java
---
@@ -91,7 +102,6 @@ public void execute() {
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/732#issuecomment-134179676
LGTM to me.
We should however stay as far away as possible from invoking all kinds of
scripts.
Implementing this for RBD is also a lot easier since
GitHub user ustcweizhou opened a pull request:
https://github.com/apache/cloudstack/pull/732
CLOUDSTACK-5863: revert volume snapshot for KVM/QCOW2
Guys, can you review it? things need to be discussed:
(1) this supports KVM/QCOW2 only. Anyone want to implement for other
14 matches
Mail list logo
