[PATCH 0/4] rbd: keep reference to lingering object requests

[PATCH 0/4] rbd: keep reference to lingering object requests

[Alex Elder]

This series applies on top of the new rbd request code.

When an osd request is marked to linger the osd client will keep
a copy of the request, and will resubmit it if necessary.  If it
gets resubmitted, it will also call the completion routine again,
and because of that we need to make sure the associated object
request structure remains valid.  The last patch in this series
ensures that by taking an extra reference for an object request
set to linger.

					-Alex

[PATCH 1/4] rbd: unregister linger in watch sync routine
[PATCH 2/4] rbd: track object rather than osd request for watch
[PATCH 3/4] rbd: decrement obj request count when deleting
[PATCH 4/4] rbd: don't drop watch requests on completion

[PATCH 1/4] rbd: unregister linger in watch sync routine

[Alex Elder]

Move the code that unregisters an rbd device's lingering header
object watch request into rbd_dev_header_watch_sync(), so it
occurs in the same function that originally sets up that request.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 47e5798..363a813 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1730,6 +1730,10 @@ static int rbd_dev_header_watch_sync(struct
rbd_device *rbd_dev, int start)
 	if (start) {
 		rbd_dev->watch_request = obj_request->osd_req;
 		ceph_osdc_set_request_linger(osdc, rbd_dev->watch_request);
+	} else {
+		ceph_osdc_unregister_linger_request(osdc,
+						rbd_dev->watch_request);
+		rbd_dev->watch_request = NULL;
 	}
 	ret = rbd_obj_request_submit(osdc, obj_request);
 	if (ret)
@@ -4040,12 +4044,6 @@ static void rbd_dev_release(struct device *dev)
 {
 	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);

-	if (rbd_dev->watch_request) {
-		struct ceph_client *client = rbd_dev->rbd_client->client;
-
-		ceph_osdc_unregister_linger_request(&client->osdc,
-						    rbd_dev->watch_request);
-	}
 	if (rbd_dev->watch_event)
 		rbd_dev_header_watch_sync(rbd_dev, 0);

-- 
1.7.9.5

Re: [PATCH 1/4] rbd: unregister linger in watch sync routine

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 01/26/2013 12:40 PM, Alex Elder wrote:
> Move the code that unregisters an rbd device's lingering header
> object watch request into rbd_dev_header_watch_sync(), so it
> occurs in the same function that originally sets up that request.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 47e5798..363a813 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1730,6 +1730,10 @@ static int rbd_dev_header_watch_sync(struct
> rbd_device *rbd_dev, int start)
>   	if (start) {
>   		rbd_dev->watch_request = obj_request->osd_req;
>   		ceph_osdc_set_request_linger(osdc, rbd_dev->watch_request);
> +	} else {
> +		ceph_osdc_unregister_linger_request(osdc,
> +						rbd_dev->watch_request);
> +		rbd_dev->watch_request = NULL;
>   	}
>   	ret = rbd_obj_request_submit(osdc, obj_request);
>   	if (ret)
> @@ -4040,12 +4044,6 @@ static void rbd_dev_release(struct device *dev)
>   {
>   	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
>
> -	if (rbd_dev->watch_request) {
> -		struct ceph_client *client = rbd_dev->rbd_client->client;
> -
> -		ceph_osdc_unregister_linger_request(&client->osdc,
> -						    rbd_dev->watch_request);
> -	}
>   	if (rbd_dev->watch_event)
>   		rbd_dev_header_watch_sync(rbd_dev, 0);
>

[PATCH 2/4] rbd: track object rather than osd request for watch

[Alex Elder]

Switch to keeping track of the object request pointer rather than
the osd request used to watch the rbd image header object.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 363a813..4e78402 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -270,7 +270,7 @@ struct rbd_device {
 	struct ceph_file_layout	layout;

 	struct ceph_osd_event   *watch_event;
-	struct ceph_osd_request *watch_request;
+	struct rbd_obj_request	*watch_request;

 	struct rbd_spec		*parent_spec;
 	u64			parent_overlap;
@@ -1728,11 +1728,11 @@ static int rbd_dev_header_watch_sync(struct
rbd_device *rbd_dev, int start)
 		goto out_cancel;

 	if (start) {
-		rbd_dev->watch_request = obj_request->osd_req;
-		ceph_osdc_set_request_linger(osdc, rbd_dev->watch_request);
+		ceph_osdc_set_request_linger(osdc, obj_request->osd_req);
+		rbd_dev->watch_request = obj_request;
 	} else {
 		ceph_osdc_unregister_linger_request(osdc,
-						rbd_dev->watch_request);
+					rbd_dev->watch_request->osd_req);
 		rbd_dev->watch_request = NULL;
 	}
 	ret = rbd_obj_request_submit(osdc, obj_request);
-- 
1.7.9.5

Re: [PATCH 2/4] rbd: track object rather than osd request for watch

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 01/26/2013 12:40 PM, Alex Elder wrote:
> Switch to keeping track of the object request pointer rather than
> the osd request used to watch the rbd image header object.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |    8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 363a813..4e78402 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -270,7 +270,7 @@ struct rbd_device {
>   	struct ceph_file_layout	layout;
>
>   	struct ceph_osd_event   *watch_event;
> -	struct ceph_osd_request *watch_request;
> +	struct rbd_obj_request	*watch_request;
>
>   	struct rbd_spec		*parent_spec;
>   	u64			parent_overlap;
> @@ -1728,11 +1728,11 @@ static int rbd_dev_header_watch_sync(struct
> rbd_device *rbd_dev, int start)
>   		goto out_cancel;
>
>   	if (start) {
> -		rbd_dev->watch_request = obj_request->osd_req;
> -		ceph_osdc_set_request_linger(osdc, rbd_dev->watch_request);
> +		ceph_osdc_set_request_linger(osdc, obj_request->osd_req);
> +		rbd_dev->watch_request = obj_request;
>   	} else {
>   		ceph_osdc_unregister_linger_request(osdc,
> -						rbd_dev->watch_request);
> +					rbd_dev->watch_request->osd_req);
>   		rbd_dev->watch_request = NULL;
>   	}
>   	ret = rbd_obj_request_submit(osdc, obj_request);
>

[PATCH 3/4] rbd: decrement obj request count when deleting

[Alex Elder]

Decrement the obj_request_count value when deleting an object
request from its image request's list.  Rearrange a few lines
in the surrounding code.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 4e78402..340773f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1071,22 +1071,29 @@ static void rbd_img_request_put(struct
rbd_img_request *img_request)
 static inline void rbd_img_obj_request_add(struct rbd_img_request
*img_request,
 					struct rbd_obj_request *obj_request)
 {
+	rbd_assert(obj_request->img_request == NULL);
+
 	rbd_obj_request_get(obj_request);
 	obj_request->img_request = img_request;
-	list_add_tail(&obj_request->links, &img_request->obj_requests);
-	obj_request->which = img_request->obj_request_count++;
+	obj_request->which = img_request->obj_request_count;
 	rbd_assert(obj_request->which != BAD_WHICH);
+	img_request->obj_request_count++;
+	list_add_tail(&obj_request->links, &img_request->obj_requests);
 }

 static inline void rbd_img_obj_request_del(struct rbd_img_request
*img_request,
 					struct rbd_obj_request *obj_request)
 {
 	rbd_assert(obj_request->which != BAD_WHICH);
-	obj_request->which = BAD_WHICH;
+
 	list_del(&obj_request->links);
+	rbd_assert(img_request->obj_request_count > 0);
+	img_request->obj_request_count--;
+	rbd_assert(obj_request->which == img_request->obj_request_count);
+	obj_request->which = BAD_WHICH;
 	rbd_assert(obj_request->img_request == img_request);
-	obj_request->callback = NULL;
 	obj_request->img_request = NULL;
+	obj_request->callback = NULL;
 	rbd_obj_request_put(obj_request);
 }

@@ -1482,6 +1489,7 @@ static void rbd_img_request_destroy(struct kref *kref)

 	for_each_obj_request_safe(img_request, obj_request, next_obj_request)
 		rbd_img_obj_request_del(img_request, obj_request);
+	rbd_assert(img_request->obj_request_count == 0);

 	if (img_request->write_request)
 		ceph_put_snap_context(img_request->snapc);
-- 
1.7.9.5

Re: [PATCH 3/4] rbd: decrement obj request count when deleting

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 01/26/2013 12:41 PM, Alex Elder wrote:
> Decrement the obj_request_count value when deleting an object
> request from its image request's list.  Rearrange a few lines
> in the surrounding code.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 4e78402..340773f 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1071,22 +1071,29 @@ static void rbd_img_request_put(struct
> rbd_img_request *img_request)
>   static inline void rbd_img_obj_request_add(struct rbd_img_request
> *img_request,
>   					struct rbd_obj_request *obj_request)
>   {
> +	rbd_assert(obj_request->img_request == NULL);
> +
>   	rbd_obj_request_get(obj_request);
>   	obj_request->img_request = img_request;
> -	list_add_tail(&obj_request->links, &img_request->obj_requests);
> -	obj_request->which = img_request->obj_request_count++;
> +	obj_request->which = img_request->obj_request_count;
>   	rbd_assert(obj_request->which != BAD_WHICH);
> +	img_request->obj_request_count++;
> +	list_add_tail(&obj_request->links, &img_request->obj_requests);
>   }
>
>   static inline void rbd_img_obj_request_del(struct rbd_img_request
> *img_request,
>   					struct rbd_obj_request *obj_request)
>   {
>   	rbd_assert(obj_request->which != BAD_WHICH);
> -	obj_request->which = BAD_WHICH;
> +
>   	list_del(&obj_request->links);
> +	rbd_assert(img_request->obj_request_count > 0);
> +	img_request->obj_request_count--;
> +	rbd_assert(obj_request->which == img_request->obj_request_count);
> +	obj_request->which = BAD_WHICH;
>   	rbd_assert(obj_request->img_request == img_request);
> -	obj_request->callback = NULL;
>   	obj_request->img_request = NULL;
> +	obj_request->callback = NULL;
>   	rbd_obj_request_put(obj_request);
>   }
>
> @@ -1482,6 +1489,7 @@ static void rbd_img_request_destroy(struct kref *kref)
>
>   	for_each_obj_request_safe(img_request, obj_request, next_obj_request)
>   		rbd_img_obj_request_del(img_request, obj_request);
> +	rbd_assert(img_request->obj_request_count == 0);
>
>   	if (img_request->write_request)
>   		ceph_put_snap_context(img_request->snapc);
>

[PATCH 4/4] rbd: don't drop watch requests on completion

[Alex Elder]

The new request code arranges to get a callback for every osd
request we submit (this was not the case previously).

We register a lingering object watch request for the header object
for each mapped rbd image.

If a connection problem occurs, the osd client will re-submit
lingering requests.  And each time such a request is re-submitted,
its callback function will get called again.

We therefore need to ensure the object request associated with the
lingering osd request stays valid, and the way to do that is to have
an extra reference to the lingering osd request.

So when a request to initiate a watch has completed, do not drop a
reference as one normally would.  Instead, hold off dropping that
reference until the request to tear down that watch request is done.

Also, only set the rbd device's watch_request pointer after the
watch request has been completed successfully, and clear the
pointer once it's been torn down.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 340773f..177ba0c 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1716,6 +1716,7 @@ static int rbd_dev_header_watch_sync(struct
rbd_device *rbd_dev, int start)
 						&rbd_dev->watch_event);
 		if (ret < 0)
 			return ret;
+		rbd_assert(rbd_dev->watch_event != NULL);
 	}

 	ret = -ENOMEM;
@@ -1735,32 +1736,44 @@ static int rbd_dev_header_watch_sync(struct
rbd_device *rbd_dev, int start)
 	if (!obj_request->osd_req)
 		goto out_cancel;

-	if (start) {
+	if (start)
 		ceph_osdc_set_request_linger(osdc, obj_request->osd_req);
-		rbd_dev->watch_request = obj_request;
-	} else {
+	else
 		ceph_osdc_unregister_linger_request(osdc,
 					rbd_dev->watch_request->osd_req);
-		rbd_dev->watch_request = NULL;
-	}
 	ret = rbd_obj_request_submit(osdc, obj_request);
 	if (ret)
 		goto out_cancel;
 	ret = rbd_obj_request_wait(obj_request);
 	if (ret)
 		goto out_cancel;
-
 	ret = obj_request->result;
 	if (ret)
 		goto out_cancel;

-	if (start)
-		goto done;	/* Done if setting up the watch request */
+	/*
+	 * Since a watch request is set to linger the osd client
+	 * will hang onto it in case it needs to be re-sent in the
+	 * event of connection loss.  If we're initiating the watch
+	 * we therefore do *not* want to drop our reference to the
+	 * object request now; we'll effectively transfer ownership
+	 * of it to the osd client instead.  Instead, we'll drop
+	 * that reference when the watch request gets torn down.
+	 */
+	if (start) {
+		rbd_dev->watch_request = obj_request;
+
+		return 0;
+	}
+
+	/* We have successfully torn down the watch request */
+
+	rbd_obj_request_put(rbd_dev->watch_request);
+	rbd_dev->watch_request = NULL;
 out_cancel:
 	/* Cancel the event if we're tearing down, or on error */
 	ceph_osdc_cancel_event(rbd_dev->watch_event);
 	rbd_dev->watch_event = NULL;
-done:
 	if (obj_request)
 		rbd_obj_request_put(obj_request);

-- 
1.7.9.5

Re: [PATCH 4/4] rbd: don't drop watch requests on completion

[Josh Durgin]

On 01/26/2013 12:41 PM, Alex Elder wrote:
> The new request code arranges to get a callback for every osd
> request we submit (this was not the case previously).
>
> We register a lingering object watch request for the header object
> for each mapped rbd image.
>
> If a connection problem occurs, the osd client will re-submit
> lingering requests.  And each time such a request is re-submitted,
> its callback function will get called again.

I think this should be fixed in the osd_client - rbd should only get
the callback once, when the watch is first registered. Later we
could add a separate callback to handle re-registration if we need to.

> We therefore need to ensure the object request associated with the
> lingering osd request stays valid, and the way to do that is to have
> an extra reference to the lingering osd request.
>
> So when a request to initiate a watch has completed, do not drop a
> reference as one normally would.  Instead, hold off dropping that
> reference until the request to tear down that watch request is done.
>
> Also, only set the rbd device's watch_request pointer after the
> watch request has been completed successfully, and clear the
> pointer once it's been torn down.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   31 ++++++++++++++++++++++---------
>   1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 340773f..177ba0c 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1716,6 +1716,7 @@ static int rbd_dev_header_watch_sync(struct
> rbd_device *rbd_dev, int start)
>   						&rbd_dev->watch_event);
>   		if (ret < 0)
>   			return ret;
> +		rbd_assert(rbd_dev->watch_event != NULL);
>   	}
>
>   	ret = -ENOMEM;
> @@ -1735,32 +1736,44 @@ static int rbd_dev_header_watch_sync(struct
> rbd_device *rbd_dev, int start)
>   	if (!obj_request->osd_req)
>   		goto out_cancel;
>
> -	if (start) {
> +	if (start)
>   		ceph_osdc_set_request_linger(osdc, obj_request->osd_req);
> -		rbd_dev->watch_request = obj_request;
> -	} else {
> +	else
>   		ceph_osdc_unregister_linger_request(osdc,
>   					rbd_dev->watch_request->osd_req);
> -		rbd_dev->watch_request = NULL;
> -	}
>   	ret = rbd_obj_request_submit(osdc, obj_request);
>   	if (ret)
>   		goto out_cancel;
>   	ret = rbd_obj_request_wait(obj_request);
>   	if (ret)
>   		goto out_cancel;
> -
>   	ret = obj_request->result;
>   	if (ret)
>   		goto out_cancel;
>
> -	if (start)
> -		goto done;	/* Done if setting up the watch request */
> +	/*
> +	 * Since a watch request is set to linger the osd client
> +	 * will hang onto it in case it needs to be re-sent in the
> +	 * event of connection loss.  If we're initiating the watch
> +	 * we therefore do *not* want to drop our reference to the
> +	 * object request now; we'll effectively transfer ownership
> +	 * of it to the osd client instead.  Instead, we'll drop
> +	 * that reference when the watch request gets torn down.
> +	 */
> +	if (start) {
> +		rbd_dev->watch_request = obj_request;
> +
> +		return 0;
> +	}
> +
> +	/* We have successfully torn down the watch request */
> +
> +	rbd_obj_request_put(rbd_dev->watch_request);
> +	rbd_dev->watch_request = NULL;
>   out_cancel:
>   	/* Cancel the event if we're tearing down, or on error */
>   	ceph_osdc_cancel_event(rbd_dev->watch_event);
>   	rbd_dev->watch_event = NULL;
> -done:
>   	if (obj_request)
>   		rbd_obj_request_put(obj_request);
>

Re: [PATCH 4/4] rbd: don't drop watch requests on completion

[Alex Elder]

On 01/30/2013 01:43 PM, Josh Durgin wrote:
> On 01/26/2013 12:41 PM, Alex Elder wrote:
>> The new request code arranges to get a callback for every osd
>> request we submit (this was not the case previously).
>>
>> We register a lingering object watch request for the header object
>> for each mapped rbd image.
>>
>> If a connection problem occurs, the osd client will re-submit
>> lingering requests.  And each time such a request is re-submitted,
>> its callback function will get called again.
> 
> I think this should be fixed in the osd_client - rbd should only get
> the callback once, when the watch is first registered. Later we
> could add a separate callback to handle re-registration if we need to.

I agree.  Even so, I would like to maintain a reference
to this lingering object request as is done in this patch.
I think it makes sense even if we'll never get another
callback.

I would like to therefore address the multiple callback
from the osd client as a separate issue.  If I update
the comments here accordingly, and open a tracker issue
for the other thing, would that be OK with you?

					-Alex

>> We therefore need to ensure the object request associated with the
>> lingering osd request stays valid, and the way to do that is to have
>> an extra reference to the lingering osd request.
>>
>> So when a request to initiate a watch has completed, do not drop a
>> reference as one normally would.  Instead, hold off dropping that
>> reference until the request to tear down that watch request is done.
>>
>> Also, only set the rbd device's watch_request pointer after the
>> watch request has been completed successfully, and clear the
>> pointer once it's been torn down.
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>>   drivers/block/rbd.c |   31 ++++++++++++++++++++++---------
>>   1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 340773f..177ba0c 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -1716,6 +1716,7 @@ static int rbd_dev_header_watch_sync(struct
>> rbd_device *rbd_dev, int start)
>>                           &rbd_dev->watch_event);
>>           if (ret < 0)
>>               return ret;
>> +        rbd_assert(rbd_dev->watch_event != NULL);
>>       }
>>
>>       ret = -ENOMEM;
>> @@ -1735,32 +1736,44 @@ static int rbd_dev_header_watch_sync(struct
>> rbd_device *rbd_dev, int start)
>>       if (!obj_request->osd_req)
>>           goto out_cancel;
>>
>> -    if (start) {
>> +    if (start)
>>           ceph_osdc_set_request_linger(osdc, obj_request->osd_req);
>> -        rbd_dev->watch_request = obj_request;
>> -    } else {
>> +    else
>>           ceph_osdc_unregister_linger_request(osdc,
>>                       rbd_dev->watch_request->osd_req);
>> -        rbd_dev->watch_request = NULL;
>> -    }
>>       ret = rbd_obj_request_submit(osdc, obj_request);
>>       if (ret)
>>           goto out_cancel;
>>       ret = rbd_obj_request_wait(obj_request);
>>       if (ret)
>>           goto out_cancel;
>> -
>>       ret = obj_request->result;
>>       if (ret)
>>           goto out_cancel;
>>
>> -    if (start)
>> -        goto done;    /* Done if setting up the watch request */
>> +    /*
>> +     * Since a watch request is set to linger the osd client
>> +     * will hang onto it in case it needs to be re-sent in the
>> +     * event of connection loss.  If we're initiating the watch
>> +     * we therefore do *not* want to drop our reference to the
>> +     * object request now; we'll effectively transfer ownership
>> +     * of it to the osd client instead.  Instead, we'll drop
>> +     * that reference when the watch request gets torn down.
>> +     */
>> +    if (start) {
>> +        rbd_dev->watch_request = obj_request;
>> +
>> +        return 0;
>> +    }
>> +
>> +    /* We have successfully torn down the watch request */
>> +
>> +    rbd_obj_request_put(rbd_dev->watch_request);
>> +    rbd_dev->watch_request = NULL;
>>   out_cancel:
>>       /* Cancel the event if we're tearing down, or on error */
>>       ceph_osdc_cancel_event(rbd_dev->watch_event);
>>       rbd_dev->watch_event = NULL;
>> -done:
>>       if (obj_request)
>>           rbd_obj_request_put(obj_request);
>>
> 

Re: [PATCH 4/4] rbd: don't drop watch requests on completion

[Josh Durgin]

On 01/30/2013 12:38 PM, Alex Elder wrote:
> On 01/30/2013 01:43 PM, Josh Durgin wrote:
>> On 01/26/2013 12:41 PM, Alex Elder wrote:
>>> The new request code arranges to get a callback for every osd
>>> request we submit (this was not the case previously).
>>>
>>> We register a lingering object watch request for the header object
>>> for each mapped rbd image.
>>>
>>> If a connection problem occurs, the osd client will re-submit
>>> lingering requests.  And each time such a request is re-submitted,
>>> its callback function will get called again.
>>
>> I think this should be fixed in the osd_client - rbd should only get
>> the callback once, when the watch is first registered. Later we
>> could add a separate callback to handle re-registration if we need to.
>
> I agree.  Even so, I would like to maintain a reference
> to this lingering object request as is done in this patch.
> I think it makes sense even if we'll never get another
> callback.
>
> I would like to therefore address the multiple callback
> from the osd client as a separate issue.  If I update
> the comments here accordingly, and open a tracker issue
> for the other thing, would that be OK with you?

That's fine with me.

Re: [PATCH 4/4] rbd: don't drop watch requests on completion

[Alex Elder]

On 01/30/2013 02:39 PM, Josh Durgin wrote:
> On 01/30/2013 12:38 PM, Alex Elder wrote:
>> On 01/30/2013 01:43 PM, Josh Durgin wrote:
>>> On 01/26/2013 12:41 PM, Alex Elder wrote:
>>>> The new request code arranges to get a callback for every osd
>>>> request we submit (this was not the case previously).
>>>>
>>>> We register a lingering object watch request for the header object
>>>> for each mapped rbd image.
>>>>
>>>> If a connection problem occurs, the osd client will re-submit
>>>> lingering requests.  And each time such a request is re-submitted,
>>>> its callback function will get called again.
>>>
>>> I think this should be fixed in the osd_client - rbd should only get
>>> the callback once, when the watch is first registered. Later we
>>> could add a separate callback to handle re-registration if we need to.
>>
>> I agree.  Even so, I would like to maintain a reference
>> to this lingering object request as is done in this patch.
>> I think it makes sense even if we'll never get another
>> callback.
>>
>> I would like to therefore address the multiple callback
>> from the osd client as a separate issue.  If I update
>> the comments here accordingly, and open a tracker issue
>> for the other thing, would that be OK with you?
> 
> That's fine with me.

http://tracker.ceph.com/issues/3967

					-Alex