Re: [libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided
On 01/05/2016 11:36 PM, John Ferlan wrote:
On 12/28/2015 10:33 AM, Wido den Hollander wrote:
> If no port number was provided for a storage pool libvirt would default
> to port 6789.
>
> librbd/librados will however already default to 6789 when no port number
> is provided.
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.
Ah, true. I'll send a new patch with also a doc fix.
>
> In the future Ceph will however switch to a new port for the Ceph monitors
s/Ceph will however switch/Ceph will switch/
> 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>
> ---
> src/storage/storage_backend_rbd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Is there a specific librdb/librados that would "fail" if a port number
wasn't assigned/passed?
No, there is not. This was just something I did 'wrong' when I wrote the
RBD integration a few years ago.
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...
Yes, I'll come up with a new patch.
Wido
John
> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index cdbfdee..df4a3d3 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) {
>