Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78031911
Some remarks:
- Use `with lock:` instead of `lock.acquire()` then `try/finally`
(compatible with Python 2.6)
- Use `with file(...) as f:` (idem
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/commit/8e953fe8593cf624c37213d94f79ea9e17a12f27#commitcomment-10118901
In systemvm/patches/debian/config/opt/cloud/bin/passwd_server_ip.py:
In systemvm/patches/debian/config/opt/cloud/bin
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/commit/8e953fe8593cf624c37213d94f79ea9e17a12f27#commitcomment-10118935
In systemvm/patches/debian/config/opt/cloud/bin/passwd_server_ip.py:
In systemvm/patches/debian/config/opt/cloud/bin
GitHub user vincentbernat opened a pull request:
https://github.com/apache/cloudstack/pull/776
sysctl: don't modify /etc/sysctl.conf
To configure firewall rules, CloudStack modifies `/etc/sysctl.conf` and
execute those modifications. This may be harmful for several re
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-137765042
@resmo Yes, it could be in `/etc/sysctl.d` instead (and no sysctl call
would be needed). I don't have a strong opinion on this (except in this case,
the
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138473948
@resmo the only problem with shipping a `/etc/sysctl.d/cloudstack.conf` is
that this is more a packaging issue. It has to be handled for all
distributions
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1541#discussion_r73830499
--- Diff: debian/cloudstack-agent.install ---
@@ -18,12 +18,11 @@
/etc/cloudstack/agent/agent.properties
/etc/cloudstack/agent
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1541#discussion_r73830557
--- Diff: debian/control ---
@@ -3,7 +3,7 @@ Section: libs
Priority: extra
Maintainer: Wido den Hollander
Build-Depends: debhelper
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1541#discussion_r73830730
--- Diff: debian/rules ---
@@ -5,9 +5,18 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f
1)
SYSCONFDIR = "/et
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1541#discussion_r73830801
--- Diff: debian/rules ---
@@ -116,17 +132,24 @@ override_dh_auto_install:
# cloudstack-usage
mkdir $(DESTDIR)/$(SYSCONFDIR
Github user vincentbernat commented on the issue:
https://github.com/apache/cloudstack/pull/1541
The only change I have noticed is the change in postinst. This could bypass
the local administrator policy. It would not restart the agent on upgrade
either. I would suggest to
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1541#discussion_r74198886
--- Diff: debian/rules ---
@@ -136,5 +156,8 @@ override_dh_auto_install:
override_dh_installinit:
dh_installinit -pcloudstack
Github user vincentbernat commented on the issue:
https://github.com/apache/cloudstack/pull/1541
Except the extra `--with systemd`, 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
Github user vincentbernat commented on the issue:
https://github.com/apache/cloudstack/pull/1647
LGTM for the Ubuntu part. You could share `/etc/sysconfig/*` and
`/etc/default/*` files between RHEL/Ubuntu (same files, just installed at
different locations). Same for the service files
Github user vincentbernat commented on the issue:
https://github.com/apache/cloudstack/pull/1647
it is not possible to have systemd support and build on 12.04 at the same
time. dh-systemd doesn't exist on 12.04 and its actions are quite important. If
you remove it and just in
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76039128
--- Diff: debian/rules ---
@@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f
1)
SYSCONFDIR = "/et
Github user vincentbernat commented on the issue:
https://github.com/apache/cloudstack/pull/1647
Otherwise, I think the change to be compatible with 12.04 is OK (no Upstart
script should ever be provided in this case).
---
If your project is set up for it, you can reply to this
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76200059
--- Diff: debian/rules ---
@@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f
1)
SYSCONFDIR = "/et
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76206826
--- Diff: debian/rules ---
@@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f
1)
SYSCONFDIR = "/et
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76225900
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76226318
--- Diff: debian/cloudstack-agent.postinst ---
@@ -41,6 +41,12 @@ case "$1" in
mkdir /etc/libvirt/hooks
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76226635
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76281415
--- Diff: packaging/systemd/cloudstack-usage.default ---
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76281544
--- Diff: debian/rules ---
@@ -35,13 +37,20 @@ override_dh_auto_install:
# cloudstack-agent
mkdir $(DESTDIR)/$(SYSCONFDIR
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76320192
--- Diff: debian/rules ---
@@ -35,13 +37,20 @@ override_dh_auto_install:
# cloudstack-agent
mkdir $(DESTDIR)/$(SYSCONFDIR
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76320932
--- Diff: packaging/systemd/cloudstack-agent.default ---
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Github user vincentbernat commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76392345
--- Diff: packaging/systemd/cloudstack-agent.default ---
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
27 matches
Mail list logo
