You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/07/30 10:35:40 UTC

[GitHub] [cloudstack] wido opened a new pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

wido opened a new pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231


   ## Description
   Ceph used to use port 6789 (no need to specify it), but with the messenger v2
   from Ceph it switched to port 3300 while 6789 still works.
   
   librados/librbd/libvirt will automatically figure out the ports to use if none is
   specified.
   
   Therefor there is no need for CloudStack to explicitely define the port in the XML
   passed to Libvirt or Qemu.
   
   Leave blank if no port number has been defined by the user.
   
   ## Types of changes
   - [x] Enhancement (improves an existing feature and functionality)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#discussion_r551309199



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java
##########
@@ -28,7 +28,7 @@ public static String RBDStringBuilder(String monHost, int monPort, String authUs
 
         rbdOpts = "rbd:" + image;
         rbdOpts += ":mon_host=" + monHost;
-        if (monPort != 6789) {
+        if (monPort > 0) {
             rbdOpts += "\\\\:" + monPort;

Review comment:
       @wido @weizhouapache can you comment on if this is the only issue?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] wido commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
wido commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-666332373


   @rhtyd I actually have rados-java 0.6.0 ready. Just need to have somebody upload it to maven central.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#discussion_r551308642



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java
##########
@@ -28,7 +28,7 @@ public static String RBDStringBuilder(String monHost, int monPort, String authUs
 
         rbdOpts = "rbd:" + image;
         rbdOpts += ":mon_host=" + monHost;
-        if (monPort != 6789) {
+        if (monPort > 0) {
             rbdOpts += "\\\\:" + monPort;

Review comment:
       this double escape seems to byte us now ;P during release of 4.15




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-668394349


   @blueorangutan test


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-668394531


   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-668740106


   <b>Trillian test result (tid-2252)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41608 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4231-t2252-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Smoke tests completed. 83 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd merged pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-666297560


   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-666309626


   Packaging result: ✔centos7 ✔debian. JID-1629


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-668768613


   seems ready to merge, right @wido @rhtyd ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-666297141


   @wido reminder - can you help do the next https://github.com/ceph/rados-java release? It would be great if we can do it soon/before 4.15 to support the latest ceph release. Thanks.
   @blueorangutan package
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] wido commented on pull request #4231: kvm/ceph: Only if a port number has been specified define in the XML

Posted by GitBox <gi...@apache.org>.
wido commented on pull request #4231:
URL: https://github.com/apache/cloudstack/pull/4231#issuecomment-669030033


   For me it is


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org