Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/140#issuecomment-88928090
Yes, it is, but we should log somewhere. Now a machine just reboots. We
should send something to syslog prior to rebooting.
---
If your project is set up for it, you
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/679#discussion_r36946797
--- Diff: server/src/com/cloud/api/query/dao/SecurityGroupJoinDaoImpl.java
---
@@ -125,6 +130,15 @@ public SecurityGroupResponse
newSecurityGroupResponse
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/709#issuecomment-132145283
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/703#issuecomment-132145192
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/708#issuecomment-132152921
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/711#issuecomment-132153264
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/696#issuecomment-132146849
LGTM
The resizing in general should be done using libvirt-java and not by
executing 'virsh' at all.
It could be that the Java bindings for libvirt
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/707#issuecomment-132154456
LGTM to me
---
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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/705#issuecomment-131812682
Pending the checks 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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/615#issuecomment-131911894
I wouldn't revert the commit either, we can probably fix this in the UI.
@borisroman could you look at this maybe with @kevindierkx ?
---
If your project
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/726#issuecomment-133397841
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/723#issuecomment-133398506
LGTM
Keep them coming!
---
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/725#issuecomment-133399741
Although I think the code is good and I want to give a LGTM, I'm just not
100% sure about this one.
---
If your project is set up for it, you can reply to this email
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/721#issuecomment-132962997
LGTM here as well. Used this script a couple of times
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-132472682
@runseb Yes, indeed. But the PR is not ready to merge yet, it's blocked by
a GSON issue which I don't know how to resolve.
This is what @DaanHoogland found out
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/716#issuecomment-132497888
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 wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35625795
--- Diff: reporter/usage-report-collector.py ---
@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-125505835
@DaanHoogland I will. Although I don't understand why these are happening
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35625832
--- Diff: reporter/usage-report-collector.py ---
@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35625883
--- Diff: reporter/usage-report-collector.py ---
@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35625751
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,487 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-126695290
@DaanHoogland I need 2.3.1 for this patch to work. It builds with 2.3.1,
but not with 1.7.2
---
If your project is set up for it, you can reply to this email and have
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-126684715
@DaanHoogland I see, but I'm not even getting near that code. So it's
unclear to me why this PR seems unstable.
---
If your project is set up for it, you can reply
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-126674015
I still don't know why travis fails. It fails on a storage part, but this
commit doesn't touch anything from the storage. So I can't see why.
---
If your project
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/651#issuecomment-127569273
Reviewed the code. 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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-127927660
@DaanHoogland You are right. There has been a change between GSON 1.7 and
2.0 which causes a failure of the tests.
Looking into this, but my Java Reflecting
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/651#issuecomment-127376598
@remibergsma I will look at this with @borisroman tomorrow at the office. I
want to verify his logic. Code seems good for now.
---
If your project is set up
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-127377070
I just don't know why the tests keep failing on this one. They seem to fail
on code not touched by this PR at all. Really want to get this in to 4.6.
---
If your
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/603#issuecomment-127378233
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/604#issuecomment-127378331
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/637#issuecomment-126214607
Code wise it is 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
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/647
CLOUDSTACK-8640: Revert to AWS SDK 1.3.22
The newer SDKs API changed which causes our S3 Template Downloader to never
complete.
Although we should fix the Template Downloader we can
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/658#issuecomment-128686068
Code LGTM, but is it the case right now that anybody can change a template?
---
If your project is set up for it, you can reply to this email and have your
reply
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/664#issuecomment-128685238
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/659#issuecomment-128686244
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/666#issuecomment-128685143
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/661#issuecomment-128685407
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/625#issuecomment-131030515
I have create a issue regarding the new Gson version:
https://issues.apache.org/jira/browse/CLOUDSTACK-8708
---
If your project is set up for it, you can reply
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/654#issuecomment-131164822
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 wido opened a pull request:
https://github.com/apache/cloudstack/pull/625
CLOUDSTACK-8677: Call-home functionality for CloudStack
With this commit the Management Server will be default generate a anonymous
Usage
report every 7 (seven) days and submit
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35533953
--- Diff: setup/db/db/schema-452to460.sql ---
@@ -398,3 +398,5 @@ CREATE TABLE `cloud`.`external_bigswitch_bcf_devices` (
CONSTRAINT
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35535873
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,473 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/621#issuecomment-124243179
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/606#issuecomment-124249061
@karuturi Fixed that. See the new commit
---
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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/605#issuecomment-124246163
LGTM, but as @runseb says, please squash 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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/607#issuecomment-125133501
I think this one can be merged after simulator is done, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/613#issuecomment-125133196
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/623#issuecomment-125132961
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/606#issuecomment-123381112
Shouldn't we then pass params to the configure method? That seems to be
missing in your example.
---
If your project is set up for it, you can reply to this email
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/481#issuecomment-121140868
The description of the pull request isn't really specific. Looking at the
files changed it is only a test case which changed, right? No CS code itself.
---
If your
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/580
CLOUDSTACK-8628: kvm: Disable Fencing when no NFS storage pools are pâ¦
â¦resent
On NFS we write a heartbeat, but without those we can not safely
fence off a host.
If we
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-121140056
Any progress on this PR? Since I really do like the proposal, but my
knowledge of Jetty/Tomcat is indeed to limited.
I can test the packaging, but I have
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/481#issuecomment-121159832
In that case LGTM
Next time try to be more descriptive, since the Jira issue didn't show it
either. I really didn't know what you meant.
---
If your project
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/584#issuecomment-121177268
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 wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/582#discussion_r34554904
--- Diff:
plugins/storage/image/s3/src/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java
---
@@ -60,7 +60,9 @@ public DataStoreTO
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/582
CLOUDSTACK-8581: S3, make connection TTL and TCP KeepAlive configureable
Signed-off-by: Wido den Hollander w...@widodh.nl
You can merge this pull request into a Git repository by running
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/582#discussion_r34786110
--- Diff: api/src/com/cloud/agent/api/to/S3TO.java ---
@@ -118,6 +122,14 @@ public boolean equals(final Object thatObject) {
return false
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/593#issuecomment-121954927
LGTM
A test would be welcome indeed
---
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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/598#issuecomment-121951761
LGTM
Seems like a use-case which was missed when this was changed
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/580#issuecomment-121967519
Working on a additional commit which brings in more logging and a unit test.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/580#issuecomment-122016035
Indeed, I noticed that to late. Fixing the tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/589#issuecomment-121414567
The pull request is against the 4.5 branch while it should be against the
master branch. We can't merge it into 4.5.
Also, you seem to change more
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/588
CLOUDSTACK-8635: Depend on the headless JRE for Ubuntu packages
This will install less packages on the system running CloudStack.
The -headless JRE doesn't include packages for running
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/572#issuecomment-122210319
Since the comment of @karuturi says there is something wrong with the code
I don't think we can say LGTM here?
I actually encountered this issue today
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/593#issuecomment-122211686
Since this one got 3 LGTM's but there are outstanding test requests, how to
proceed? Do I merge it or wait for the tests?
---
If your project is set up for it, you
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/572#issuecomment-122240171
I just tested it on 4.5 management server and my deployment errors are
gone. I'm fine with it.
LGTM
---
If your project is set up for it, you can reply
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/572#issuecomment-122246019
@karuturi @DaanHoogland @bhaisaab I suggest we merge it into 4.5 as well so
that it makes 4.5.2
---
If your project is set up for it, you can reply to this email
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/580#issuecomment-122236231
@DaanHoogland It is in the KVMFencer in the MGMT server. The Agent can't
send an alert, so I had to do this in the mgmt server.
---
If your project is set up
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/577#issuecomment-122247318
Code-wise I'm not to happy. There are all kinds of assumptions about paths.
mkisofs for example always being there in /usr/bin.
Using /tmp for temporary
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/580#issuecomment-122234914
The tests have been fixed, Mocking issue in the test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/600#issuecomment-122246414
@DaanHoogland You can merge it I think
---
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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/566#issuecomment-122240477
I think it looks good? But it depends on earlier PR's I think since it's
only a test
---
If your project is set up for it, you can reply to this email and have your
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/606
CLOUDSTACK-8648: Do not configure the ImageFormat Processor when fetcâ¦
â¦hing filesize
It will throw an exception and that's needed.
Also, make the log show about which file we
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/966#issuecomment-150553318
Agreed with @remibergsma, but the code-wise this is a LGTM.
One PR which includes #812 as well would be nice.
---
If your project is set up for it, you can
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/977#issuecomment-151094003
@ustcweizhou It's just a matter of sending patches to RedHat and they will
accept it. You can also request a new release of the bindings when you need
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/977#discussion_r42975042
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
---
@@ -3388,4 +3419,83 @@ public String mapRbdDevice
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/977#discussion_r42975005
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreVMSnapshotCommandWrapper.java
---
@@ -0,0 +1,121
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/977#discussion_r42975057
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
---
@@ -3388,4 +3419,83 @@ public String mapRbdDevice
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/977#issuecomment-151412595
@ustcweizhou I think we shouldn't want a enable/disable flag in the
agent.properties, it should work for anybody.
That we already have a lot of commands being
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/981#issuecomment-151412107
Is there any way we can test this automatically? I think there is no Unit
Test for this, right?
I currently don't have a setup to test this, but can you
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/977#issuecomment-151442130
@DaanHoogland Indeed, that is the case.
The problems I had were with libvirt, not the bindings. libvirt-java is
distributed via Maven and we include it in our
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/985
CLOUDSTACK-8715: Add VirtIO channel to all Instances for the Qemu Gueâ¦
â¦st Agent
This commit adds a additional VirtIO channel with the name
'org.qemu.guest_agent.0'
to all
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/978#issuecomment-151444216
LGTM
Nice feature indeed :)
---
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
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/986
Add Unit Tests for Libvirt/KVM storage code
These classes were not covered by Unit Tests and this commit
adds some tests for their basic functionality.
You can merge this pull request
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/985#issuecomment-151836172
@remibergsma Hmm, was SELinux enabled on that system? Can't see any reason
why it wouldn't work. All the directories exist.
Do you by any chance have the XML
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/985#issuecomment-152188711
Hmm, ok. That should be allowed. Since we want the Guest tools also to be
supported on SSVMs to control them properly.
---
If your project is set up for it, you can
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/985#issuecomment-152157162
@borisroman Could you give this PR a spin?
---
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
GitHub user wido opened a pull request:
https://github.com/apache/cloudstack/pull/1005
kvm: Add UnitTests for LibvirtUtilitiesHelper
These were lacking, but this helper is used in various places
inside the KVM code.
Some simple tests to verify the helper is doing what
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1004#issuecomment-152157062
Isn't 4.5 closed for features? I know this might sound picky, but as far as
I know we are only supposed to fix bugs in a point release.
---
If your project is set up
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/985#issuecomment-151871826
@remibergsma Ok, that is odd.
A proper XML should look like
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/942#issuecomment-151547785
Based on the fact that all the code builds and the UnitTests are also
running properly I can give this one a LGTM
---
If your project is set up for it, you can reply
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/977#issuecomment-151516059
@ustcweizhou Great to see that these are there! @borisroman has some
pending patches as well. If we submit some patches towards
libvir-l...@redhat.com we can probably
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/995#issuecomment-151784319
Seems good to me, but while we are working on this. Any way we can Unit
Test this? Looking at the code it's possible.
If you could just write a test
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/991#issuecomment-151784718
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/990#issuecomment-151784690
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 wido commented on the pull request:
https://github.com/apache/cloudstack/pull/993#issuecomment-151791418
@bhaisaab With systemd we no longer write a out and err file. We did this
with JSVC, but now it's send to stdout and stderr which is then picked up by
journald
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/884#issuecomment-152114694
@remibergsma ping? ^^
---
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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/992#issuecomment-152134450
LGTM now DEB is included
---
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
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1003#issuecomment-152149949
My only concern is, IPv6 is coming as well into CS, are we also going to
display that address? For now this seems fine with me, but adding more columns
later on makes
1 - 100 of 1529 matches
Mail list logo
