Need some help with the RBD Java bindings

Need some help with the RBD Java bindings

[Wido den Hollander]

Hi,

I'm having some issues with the Java bindings for librbd and I'm not 
sure what the problem is here.

The problem is with listing snapshot of a RBD image. I test it in this 
test case:

public void testCreateAndClone() {
     try {
         ..
         ..
         image.snapCreate(snapName);
         image.snapProtect(snapName);

         List<RbdSnapInfo> snaps = image.snapList();
         assertEquals("There should only be one snapshot", 1, snaps.size());
     ..
     ..
     }
}

This test fails 95% of the time with my whole JVM crashing.

The code to blame is in RbdImage.java:

public List<RbdSnapInfo> snapList() throws RbdException {
     IntByReference numSnaps = new IntByReference(16);
     PointerByReference snaps = new PointerByReference();
     List<RbdSnapInfo> list = new ArrayList<RbdSnapInfo>();
     RbdSnapInfo snapInfo, snapInfos[];

     while (true) {
         int r = rbd.rbd_snap_list(this.getPointer(), snaps, numSnaps);
         if (r >= 0) {
             numSnaps.setValue(r);
             break;
         } else {
             throw new RbdException("Failed listing snapshots", r);
         }
     }

     Pointer p = snaps.getValue(); <<<< crash
     snapInfo = new RbdSnapInfo(p);

     ..
     ..
}

So it crashes when it wants to get the value of the snaps var.

I can't figure out why this is happening and why it isn't consistent. 
I'm not a real JNA expert and I was hoping somebody around here would be 
able to figure out what I'm doing wrong.

Any Java/JNA experts around who might have a clue?

The source of rados-java: https://github.com/wido/rados-java

-- 
Wido den Hollander
42on B.V.

Phone: +31 (0)20 700 9902
Skype: contact42on

Re: Need some help with the RBD Java bindings

[Noah Watkins]

On Thu, Aug 15, 2013 at 8:51 AM, Wido den Hollander <wido@42on.com> wrote:
>
> public List<RbdSnapInfo> snapList() throws RbdException {
>     IntByReference numSnaps = new IntByReference(16);
>     PointerByReference snaps = new PointerByReference();
>     List<RbdSnapInfo> list = new ArrayList<RbdSnapInfo>();
>     RbdSnapInfo snapInfo, snapInfos[];
>
>     while (true) {
>         int r = rbd.rbd_snap_list(this.getPointer(), snaps, numSnaps);

I think you need to allocate the memory for `snaps` yourself. Here is
the RBD wrapper for Python which does that:

  self.snaps = (rbd_snap_info_t * num_snaps.value)()
  ret = self.librbd.rbd_snap_list(image.image, byref(self.snaps),
                       byref(num_snaps))

- Noah

Re: Need some help with the RBD Java bindings

[Noah Watkins]

Wido,

I pushed up a patch to

   https://github.com/ceph/rados-java/commit/ca16d82bc5b596620609880e429ec9f4eaa4d5ce

That includes a fix for this problem. The fix is a bit hacky, but the
tests pass now. I included more details about the hack in the code.

On Thu, Aug 15, 2013 at 9:57 AM, Noah Watkins <noah.watkins@inktank.com> wrote:
> On Thu, Aug 15, 2013 at 8:51 AM, Wido den Hollander <wido@42on.com> wrote:
>>
>> public List<RbdSnapInfo> snapList() throws RbdException {
>>     IntByReference numSnaps = new IntByReference(16);
>>     PointerByReference snaps = new PointerByReference();
>>     List<RbdSnapInfo> list = new ArrayList<RbdSnapInfo>();
>>     RbdSnapInfo snapInfo, snapInfos[];
>>
>>     while (true) {
>>         int r = rbd.rbd_snap_list(this.getPointer(), snaps, numSnaps);
>
> I think you need to allocate the memory for `snaps` yourself. Here is
> the RBD wrapper for Python which does that:
>
>   self.snaps = (rbd_snap_info_t * num_snaps.value)()
>   ret = self.librbd.rbd_snap_list(image.image, byref(self.snaps),
>                        byref(num_snaps))
>
> - Noah

Re: Need some help with the RBD Java bindings

[Wido den Hollander]

On 08/20/2013 11:26 PM, Noah Watkins wrote:
> Wido,
>
> I pushed up a patch to
>
>     https://github.com/ceph/rados-java/commit/ca16d82bc5b596620609880e429ec9f4eaa4d5ce
>
> That includes a fix for this problem. The fix is a bit hacky, but the
> tests pass now. I included more details about the hack in the code.
>

I see. Works like a charm for me now. I'll do some further testing with 
CloudStack.

Wido

> On Thu, Aug 15, 2013 at 9:57 AM, Noah Watkins <noah.watkins@inktank.com> wrote:
>> On Thu, Aug 15, 2013 at 8:51 AM, Wido den Hollander <wido@42on.com> wrote:
>>>
>>> public List<RbdSnapInfo> snapList() throws RbdException {
>>>      IntByReference numSnaps = new IntByReference(16);
>>>      PointerByReference snaps = new PointerByReference();
>>>      List<RbdSnapInfo> list = new ArrayList<RbdSnapInfo>();
>>>      RbdSnapInfo snapInfo, snapInfos[];
>>>
>>>      while (true) {
>>>          int r = rbd.rbd_snap_list(this.getPointer(), snaps, numSnaps);
>>
>> I think you need to allocate the memory for `snaps` yourself. Here is
>> the RBD wrapper for Python which does that:
>>
>>    self.snaps = (rbd_snap_info_t * num_snaps.value)()
>>    ret = self.librbd.rbd_snap_list(image.image, byref(self.snaps),
>>                         byref(num_snaps))
>>
>> - Noah
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Wido den Hollander
42on B.V.

Phone: +31 (0)20 700 9902
Skype: contact42on

Re: Need some help with the RBD Java bindings

[Noah Watkins]

Wido,

How would you feel about creating two RbdSnapInfo objects. The first
would be something like ceph.rbd.RbdSnapInfo and the second would be
ceph.rbd.jna.RbdSnapInfo. The former is what will be exposed through
the API, and the later is used only internally. That should address
the hacky-ness of my snap listing fix: just create a copy of the
SnapInfo into the public struct. it also means we can avoid exposing
users to JNA structures.

On Wed, Aug 21, 2013 at 5:11 AM, Wido den Hollander <wido@42on.com> wrote:
> On 08/20/2013 11:26 PM, Noah Watkins wrote:
>>
>> Wido,
>>
>> I pushed up a patch to
>>
>>
>> https://github.com/ceph/rados-java/commit/ca16d82bc5b596620609880e429ec9f4eaa4d5ce
>>
>> That includes a fix for this problem. The fix is a bit hacky, but the
>> tests pass now. I included more details about the hack in the code.
>>
>
> I see. Works like a charm for me now. I'll do some further testing with
> CloudStack.
>
> Wido
>
>> On Thu, Aug 15, 2013 at 9:57 AM, Noah Watkins <noah.watkins@inktank.com>
>> wrote:
>>>
>>> On Thu, Aug 15, 2013 at 8:51 AM, Wido den Hollander <wido@42on.com>
>>> wrote:
>>>>
>>>>
>>>> public List<RbdSnapInfo> snapList() throws RbdException {
>>>>      IntByReference numSnaps = new IntByReference(16);
>>>>      PointerByReference snaps = new PointerByReference();
>>>>      List<RbdSnapInfo> list = new ArrayList<RbdSnapInfo>();
>>>>      RbdSnapInfo snapInfo, snapInfos[];
>>>>
>>>>      while (true) {
>>>>          int r = rbd.rbd_snap_list(this.getPointer(), snaps, numSnaps);
>>>
>>>
>>> I think you need to allocate the memory for `snaps` yourself. Here is
>>> the RBD wrapper for Python which does that:
>>>
>>>    self.snaps = (rbd_snap_info_t * num_snaps.value)()
>>>    ret = self.librbd.rbd_snap_list(image.image, byref(self.snaps),
>>>                         byref(num_snaps))
>>>
>>> - Noah
>>
>> --
>>
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
> --
> Wido den Hollander
> 42on B.V.
>
> Phone: +31 (0)20 700 9902
> Skype: contact42on

Re: Need some help with the RBD Java bindings

[Wido den Hollander]

On 08/21/2013 09:47 PM, Noah Watkins wrote:
> Wido,
>
> How would you feel about creating two RbdSnapInfo objects. The first
> would be something like ceph.rbd.RbdSnapInfo and the second would be
> ceph.rbd.jna.RbdSnapInfo. The former is what will be exposed through
> the API, and the later is used only internally. That should address
> the hacky-ness of my snap listing fix: just create a copy of the
> SnapInfo into the public struct. it also means we can avoid exposing
> users to JNA structures.
>

Yes, seems like a good thing to do. I wasn't sure myself when I was 
writing the bindings on how the packaging should be.

One of the things I haven't tested thoroughly enough is if you as a user 
of the bindings are able to crash the JVM. Since that should never happen.

Wido

> On Wed, Aug 21, 2013 at 5:11 AM, Wido den Hollander <wido@42on.com> wrote:
>> On 08/20/2013 11:26 PM, Noah Watkins wrote:
>>>
>>> Wido,
>>>
>>> I pushed up a patch to
>>>
>>>
>>> https://github.com/ceph/rados-java/commit/ca16d82bc5b596620609880e429ec9f4eaa4d5ce
>>>
>>> That includes a fix for this problem. The fix is a bit hacky, but the
>>> tests pass now. I included more details about the hack in the code.
>>>
>>
>> I see. Works like a charm for me now. I'll do some further testing with
>> CloudStack.
>>
>> Wido
>>
>>> On Thu, Aug 15, 2013 at 9:57 AM, Noah Watkins <noah.watkins@inktank.com>
>>> wrote:
>>>>
>>>> On Thu, Aug 15, 2013 at 8:51 AM, Wido den Hollander <wido@42on.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> public List<RbdSnapInfo> snapList() throws RbdException {
>>>>>       IntByReference numSnaps = new IntByReference(16);
>>>>>       PointerByReference snaps = new PointerByReference();
>>>>>       List<RbdSnapInfo> list = new ArrayList<RbdSnapInfo>();
>>>>>       RbdSnapInfo snapInfo, snapInfos[];
>>>>>
>>>>>       while (true) {
>>>>>           int r = rbd.rbd_snap_list(this.getPointer(), snaps, numSnaps);
>>>>
>>>>
>>>> I think you need to allocate the memory for `snaps` yourself. Here is
>>>> the RBD wrapper for Python which does that:
>>>>
>>>>     self.snaps = (rbd_snap_info_t * num_snaps.value)()
>>>>     ret = self.librbd.rbd_snap_list(image.image, byref(self.snaps),
>>>>                          byref(num_snaps))
>>>>
>>>> - Noah
>>>
>>> --
>>>
>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> --
>> Wido den Hollander
>> 42on B.V.
>>
>> Phone: +31 (0)20 700 9902
>> Skype: contact42on
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Wido den Hollander
42on B.V.

Phone: +31 (0)20 700 9902
Skype: contact42on

Re: Need some help with the RBD Java bindings

[Noah Watkins]

On Wed, Aug 21, 2013 at 11:20 PM, Wido den Hollander <wido@42on.com> wrote:
>
> Yes, seems like a good thing to do. I wasn't sure myself when I was writing
> the bindings on how the packaging should be.

I'm not entirely sure either. With JNI it's pretty simple, but JNA
introduces all sorts of additional classes. Do you know of any large
projects using JNA? It'd be nice to find some references to examine.

> One of the things I haven't tested thoroughly enough is if you as a user of
> the bindings are able to crash the JVM. Since that should never happen.

I think that JNA is safe in that it will prevent itself from being
used incorrectly, but I don't think anything would prevent someone
from say creating a pointer off into space with JNA and then passing
it to a external library that would dereference it.