On 12/28/2015 10:33 AM, Wido den Hollander wrote: reads better as: If no port number was provided for a storage pool libvirt defaults to port 6789; however, librbd/librados already default to 6789 when no port number is provided. s/Ceph will however switch/Ceph will switch/ Is there a specific librdb/librados that would "fail" if a port number wasn't assigned/passed? Think in terms of support some "older" version that some customer may have installed. Also, I searched on "6789" and found: src/util/virstoragefile.c/virStorageSourceRBDAddHost docs/formatdomain.html.in I think in particular the docs will need to be updated... John

If no port number was provided for a storage pool libvirt defaults to port 6789; however, librbd/librados already default to 6789 when no port number is provided. In the future Ceph will switch to a new port for the Ceph monitors since port 6789 is already assigned to a different application by IANA. Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as the 'Ceph monitor' port. In this case it is the best solution to not hardcode any port number into libvirt and let librados handle the connection. Only if a user specifies a different port number we pass it down to librados, otherwise we leave it blank. Signed-off-by: Wido den Hollander <wido(a)widodh.nl&gt; --- docs/formatdomain.html.in | 2 +- docs/storage.html.in | 8 +++++--- src/storage/storage_backend_rbd.c | 2 +- src/util/virstoragefile.c | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ce46f06..889e721 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2151,7 +2151,7 @@ <td> rbd </td> <td> monitor servers of RBD </td> <td> one or more </td> - <td> 6789 </td> + <td> librados default </td> </tr> <tr> <td> sheepdog </td> diff --git a/docs/storage.html.in b/docs/storage.html.in index 6c8222a..1f01330 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -550,7 +550,9 @@ backend supports cephx authentication for communication with the Ceph cluster. Storing the cephx authentication key is done with the libvirt secret mechanism. The UUID in the example pool input - refers to the UUID of the stored secret. + refers to the UUID of the stored secret.<br /> + The port attribute for a Ceph monitor does not have to be provided. + If not provided librados will use the default Ceph monitor port. <span class="since">Since 0.9.13</span> </p> @@ -560,8 +562,8 @@ &lt;name&gt;myrbdpool&lt;/name&gt; &lt;source&gt; &lt;name&gt;rbdpool&lt;/name&gt; - &lt;host name='1.2.3.4' port='6789'/&gt; - &lt;host name='my.ceph.monitor' port='6789'/&gt; + &lt;host name='1.2.3.4'/&gt; + &lt;host name='my.ceph.monitor'/&gt; &lt;host name='third.ceph.monitor' port='6789'/&gt; &lt;auth username='admin' type='ceph'&gt; &lt;secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/&gt; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80684eb..3235a3e 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -173,7 +173,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, for (i = 0; i < source->nhost; i++) { if (source->hosts[i].name != NULL && !source->hosts[i].port) { - virBufferAsprintf(&mon_host, "%s:6789,", + virBufferAsprintf(&mon_host, "%s,", source->hosts[i].name); } else if (source->hosts[i].name != NULL && source->hosts[i].port) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2aa1d90..1354601 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2246,7 +2246,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) goto error; } else { - if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) + if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, NULL) < 0) goto error; } -- 1.9.1

On 01/06/2016 09:49 AM, Andrea Bolognani wrote: What does that benefit? We set it to 6789 and to NULL afterwards? I just want to get rid of any hardcoded monitor ports in libvirt for Ceph.

If no port number was provided for a storage pool libvirt defaults to port 6789; however, librbd/librados already default to 6789 when no port number is provided. In the future Ceph will switch to a new port for the Ceph monitors since port 6789 is already assigned to a different application by IANA. Port 6789 is assigned to SMC-HTTPS and Ceph now has port 3300 assigned as the 'Ceph monitor' port. In this case it is the best solution to not hardcode any port number into libvirt and let librados handle the connection. Only if a user specifies a different port number we pass it down to librados, otherwise we leave it blank. Signed-off-by: Wido den Hollander <wido(a)widodh.nl&gt; --- .gnulib | 2 +- docs/formatdomain.html.in | 2 +- docs/storage.html.in | 8 +++++--- src/storage/storage_backend_rbd.c | 2 +- src/util/virstoragefile.c | 3 +-- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.gnulib b/.gnulib index 6cc32c6..f39477d 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 6cc32c63e80bc1a30c521b2f07f2b54909b59892 +Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ce46f06..889e721 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2151,7 +2151,7 @@ <td> rbd </td> <td> monitor servers of RBD </td> <td> one or more </td> - <td> 6789 </td> + <td> librados default </td> </tr> <tr> <td> sheepdog </td> diff --git a/docs/storage.html.in b/docs/storage.html.in index 6c8222a..1f01330 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -550,7 +550,9 @@ backend supports cephx authentication for communication with the Ceph cluster. Storing the cephx authentication key is done with the libvirt secret mechanism. The UUID in the example pool input - refers to the UUID of the stored secret. + refers to the UUID of the stored secret.<br /> + The port attribute for a Ceph monitor does not have to be provided. + If not provided librados will use the default Ceph monitor port. <span class="since">Since 0.9.13</span> </p> @@ -560,8 +562,8 @@ &lt;name&gt;myrbdpool&lt;/name&gt; &lt;source&gt; &lt;name&gt;rbdpool&lt;/name&gt; - &lt;host name='1.2.3.4' port='6789'/&gt; - &lt;host name='my.ceph.monitor' port='6789'/&gt; + &lt;host name='1.2.3.4'/&gt; + &lt;host name='my.ceph.monitor'/&gt; &lt;host name='third.ceph.monitor' port='6789'/&gt; &lt;auth username='admin' type='ceph'&gt; &lt;secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/&gt; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 80684eb..3235a3e 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -173,7 +173,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, for (i = 0; i < source->nhost; i++) { if (source->hosts[i].name != NULL && !source->hosts[i].port) { - virBufferAsprintf(&mon_host, "%s:6789,", + virBufferAsprintf(&mon_host, "%s,", source->hosts[i].name); } else if (source->hosts[i].name != NULL && source->hosts[i].port) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2aa1d90..f6ba0c8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2246,8 +2246,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0) goto error; } else { - if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, "6789") < 0) - goto error; + src->hosts[src->nhosts - 1].port = NULL; } parts = virStringSplit(hostport, "\\:", 0); -- 1.9.1

On 06-01-16 14:55, John Ferlan wrote: Argh! That one got in during my rebase. My apologies. Super, tnx! Wido