You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by mike-tutkowski <gi...@git.apache.org> on 2015/09/01 06:08:59 UTC
[GitHub] cloudstack pull request: Support live migration on older version o...
GitHub user mike-tutkowski opened a pull request:
https://github.com/apache/cloudstack/pull/767
Support live migration on older version of Libvirt
https://issues.apache.org/jira/browse/CLOUDSTACK-8792
A flag being passed to Libvirt assumes v1.0.0 or later.
We need to put a check in the code to pass in a different flag if the version of Libvirt is < 1.0.0.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-tutkowski/cloudstack CLOUDSTACK-8792
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/767.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #767
----
commit c5a0d5e01c98a1a1915ea2d778baa2f30807f788
Author: Mike Tutkowski <mi...@solidfire.com>
Date: 2015-08-31T18:40:08Z
Support live migration on older version of Libvirt
----
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/767#discussion_r38414670
--- Diff: plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java ---
@@ -1273,10 +1274,20 @@ public void testMigrateCommand() {
verify(libvirtComputingResource, times(1)).getDisks(conn, vmName);
try {
verify(conn, times(1)).domainLookupByName(vmName);
- verify(dm, times(1)).getXMLDesc(8);
} catch (final LibvirtException e) {
fail(e.getMessage());
}
+
+ try {
+ verify(dm, times(1)).getXMLDesc(8);
+ } catch (final Throwable t) {
--- End diff --
It's a Throwable because if the verify fails, it seems an Error is thrown
instead of an Exception type. My first pass tried to catch
LibvirtException, but that didn't work. Then I tried to catch Exception,
but that didn't work either.
On Tuesday, September 1, 2015, Daan Hoogland <no...@github.com>
wrote:
> In
> plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
> <https://github.com/apache/cloudstack/pull/767#discussion_r38395369>:
>
> > } catch (final LibvirtException e) {
> > fail(e.getMessage());
> > }
> > +
> > + try {
> > + verify(dm, times(1)).getXMLDesc(8);
> > + } catch (final Throwable t) {
>
> why such e general catch? this would probably not be the place to get out
> of memory or other system failures. I'd say catch only LibvirtException
> here.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/767/files#r38395369>.
>
--
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/767#issuecomment-136684709
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/767#issuecomment-136939528
Something went wrong in this PR, why is @hubot here and not asfgit? The PR is still getting listed in the list of open PRs, even though it has been merged on master.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski closed the pull request at:
https://github.com/apache/cloudstack/pull/767
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/767#issuecomment-136585198
LGTM, whoever merged, please merge on 4.5 branch as well.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/767#issuecomment-136940141
May be there is some issue with ACS or github. I also noticed that commits are not getting closed on merge and @hubot is marking it as merged after sometime. For these I guess the author needs to close, at least thats what I did for one of my PRs.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/767#issuecomment-136877843
Rohit - I just submitted https://github.com/apache/cloudstack/pull/771/files
for the support of live migration on older versions of Libvirt in CS 4.5.
Please take a look at it when you get a chance.
Thanks,
Mike
On Tue, Sep 1, 2015 at 7:19 AM, asfbot <no...@github.com> wrote:
> Mike Tutkowski on dev@cloudstack.apache.org replies:
> Sure, I can submit a follow-up PR for 4.5.
> 5
> ur
> e
> se
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/767#issuecomment-136717133>.
>
--
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] cloudstack pull request: Support live migration on older
version o...
Posted by Mike Tutkowski <mi...@solidfire.com>.
Sure, I can submit a follow-up PR for 4.5.
On Tuesday, September 1, 2015, bhaisaab <gi...@git.apache.org> wrote:
> Github user bhaisaab commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/767#issuecomment-136686420
>
> @mike-tutkowski I've merged this on master, cherry-pick failing on 4.5
> branch; can you send a new PR for 4.5 branch?
>
>
> ---
> 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 feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org <javascript:;> or
> file a JIRA ticket
> with INFRA.
> ---
>
--
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/767#issuecomment-136686420
@mike-tutkowski I've merged this on master, cherry-pick failing on 4.5 branch; can you send a new PR for 4.5 branch?
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Support live migration on older version o...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/767#discussion_r38395369
--- Diff: plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java ---
@@ -1273,10 +1274,20 @@ public void testMigrateCommand() {
verify(libvirtComputingResource, times(1)).getDisks(conn, vmName);
try {
verify(conn, times(1)).domainLookupByName(vmName);
- verify(dm, times(1)).getXMLDesc(8);
} catch (final LibvirtException e) {
fail(e.getMessage());
}
+
+ try {
+ verify(dm, times(1)).getXMLDesc(8);
+ } catch (final Throwable t) {
--- End diff --
why such e general catch? this would probably not be the place to get out of memory or other system failures. I'd say catch only LibvirtException here.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---