[libvirt] [PATCH] rbd: Do not append Ceph monitor port number 6789 if not provided

[Wido den Hollander]

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 at 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) {
>>