On 04/22/2013 04:39 PM, Alex Elder wrote:
On 04/22/2013 04:57 PM, Josh Durgin wrote:
A couple small things below. With those fixed:
Reviewed-by: Josh Durgin <josh.durgin@xxxxxxxxxxx>
On 04/21/2013 02:17 PM, Alex Elder wrote:
Callers of rbd_obj_method_sync() don't know how many bytes of data
got returned by the class method call. As a result, they have been
assuming enough got returned to decode whatever was expected.
This isn't safe. We know how many bytes got transferred, so have
rbd_obj_method_sync() return that amount (rather than just 0) if
the call is successful.
. . .
@@ -3709,9 +3714,9 @@ static int rbd_dev_v2_parent_info(struct
rbd_device *rbd_dev)
if (ret < 0)
goto out_err;
- ret = -ERANGE;
p = reply_buf;
end = reply_buf + size;
You didn't mention this, but now that I look at this I
managed to screw up the point of this patch in this spot.
The above line should (and will) be:
end = reply_buf + ret;
(That was why the "ret = -ERANGE" had to be moved to begin
with.) I did it right in one similar spot elsewhere in
the patch.
+ ret = -ERANGE;
maybe check for the full parent_spec here? even if there's no parent,
a complete parent_spec should be returned.
I'm not sure I understand this comment.
As it stands, we allocate a local parent_spec structure, and
that gets filled based on what comes back from the "get_parent"
method call. If anything goes wrong, we discard the parent
spec and return an error. Only when all goes well do we assign
rbd_dev->parent_spec = parent_spec;
(and the same goes for the overlap). ceph_extract_encoded_string()
will return an error if it ends up short, and the other things
that extract values also check for length.
Ok, that makes sense now.
Is that what you were referring to--return either a complete
one or none? Or was there something else?
I meant that the class method should always return a complete
parent_spec, so the response length should be checked. Letting
ceph_extract_encoded_string() check the correct length like you
mentioned is fine.
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
