Is jdm_delete_filehandle part of a public API?

Is jdm_delete_filehandle part of a public API?

[Eric Sandeen]

I was cleaning up xfsprogs to plug some leaks, and wanted to use
jdm_delete_filehandle().  I noticed that it has an "hlen" argument which
is unused.

Can we remove that, or is this part of a public API?  It's not in any
manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
but it is in a public header...

anyone know?

If needed I guess I can just call it with hlen==0, but that seems odd.

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Mark Tinguely]

On 07/29/14 12:31, Eric Sandeen wrote:
> I was cleaning up xfsprogs to plug some leaks, and wanted to use
> jdm_delete_filehandle().  I noticed that it has an "hlen" argument which
> is unused.
>
> Can we remove that, or is this part of a public API?  It's not in any
> manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
> but it is in a public header...
>
> anyone know?
>
> If needed I guess I can just call it with hlen==0, but that seems odd.
>
> Thanks,
> -Eric

The first thing that comes to mind is maybe they trying to distinguish 
between a fshandle or handle. Or they we trying to be consistent with 
the allocation calls.

The libhandle free_handle has the same calling parameters. It also does 
nothing with the length. That we cannot change without breaking existing 
code.

I will look/ask around.

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Mark Tinguely]

On 07/29/14 13:18, Mark Tinguely wrote:
> On 07/29/14 12:31, Eric Sandeen wrote:
>> I was cleaning up xfsprogs to plug some leaks, and wanted to use
>> jdm_delete_filehandle(). I noticed that it has an "hlen" argument which
>> is unused.
>>
>> Can we remove that, or is this part of a public API? It's not in any
>> manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
>> but it is in a public header...
>>
>> anyone know?
>>
>> If needed I guess I can just call it with hlen==0, but that seems odd.
>>
>> Thanks,
>> -Eric
>
> The first thing that comes to mind is maybe they trying to distinguish
> between a fshandle or handle. Or they we trying to be consistent with
> the allocation calls.
>
> The libhandle free_handle has the same calling parameters. It also does
> nothing with the length. That we cannot change without breaking existing
> code.
>
> I will look/ask around.
>
> --Mark.

Looks like the code is pretty sloppy with freeing the handles.

Best guess is jdm_delete_filehandle() and  free_handle are trying to 
keep the API similar to DMAPI. The DMAPI handle free routine, 
dm_handle_free(), also has a second length parameter that is not used in 
the library.

The code example that I saw are similar to the use in xfsdump, where the 
length used in the free comes from the handle creation/conversion routine.

Since the xfsprogs do not open handles with calls that provide a length. 
How about FSHANDLE_SZ and FILEHANDLE_SZ depending on if it is a 
xfs_fshandle or xfs_handle?

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Eric Sandeen]

On 7/29/14, 3:04 PM, Mark Tinguely wrote:
> On 07/29/14 13:18, Mark Tinguely wrote:
>> On 07/29/14 12:31, Eric Sandeen wrote:
>>> I was cleaning up xfsprogs to plug some leaks, and wanted to use
>>> jdm_delete_filehandle(). I noticed that it has an "hlen" argument which
>>> is unused.
>>>
>>> Can we remove that, or is this part of a public API? It's not in any
>>> manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
>>> but it is in a public header...
>>>
>>> anyone know?
>>>
>>> If needed I guess I can just call it with hlen==0, but that seems odd.
>>>
>>> Thanks,
>>> -Eric
>>
>> The first thing that comes to mind is maybe they trying to distinguish
>> between a fshandle or handle. Or they we trying to be consistent with
>> the allocation calls.
>>
>> The libhandle free_handle has the same calling parameters. It also does
>> nothing with the length. That we cannot change without breaking existing
>> code.
>>
>> I will look/ask around.
>>
>> --Mark.
> 
> Looks like the code is pretty sloppy with freeing the handles.

yeah, that's what I was going to fix :)

> Best guess is jdm_delete_filehandle() and free_handle are trying to
> keep the API similar to DMAPI. The DMAPI handle free routine,
> dm_handle_free(), also has a second length parameter that is not used
> in the library.
> 
> The code example that I saw are similar to the use in xfsdump, where
> the length used in the free comes from the handle creation/conversion
> routine.

yup but I don't think jdm_getfshandle has anything similar does it?
 
> Since the xfsprogs do not open handles with calls that provide a
> length. How about FSHANDLE_SZ and FILEHANDLE_SZ depending on if it is
> a xfs_fshandle or xfs_handle?

*shrug* it's not used anyway - I'm not sure why we'd need to invent a
macro to pass in only to have it ignored.  Is there any advantage to that?


-Eric

> 
> --Mark.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Mark Tinguely]

On 07/29/14 15:18, Eric Sandeen wrote:
> On 7/29/14, 3:04 PM, Mark Tinguely wrote:
>> On 07/29/14 13:18, Mark Tinguely wrote:
>>> On 07/29/14 12:31, Eric Sandeen wrote:
>>>> I was cleaning up xfsprogs to plug some leaks, and wanted to use
>>>> jdm_delete_filehandle(). I noticed that it has an "hlen" argument which
>>>> is unused.
>>>>
>>>> Can we remove that, or is this part of a public API? It's not in any
>>>> manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
>>>> but it is in a public header...
>>>>
>>>> anyone know?
>>>>
>>>> If needed I guess I can just call it with hlen==0, but that seems odd.
>>>>
>>>> Thanks,
>>>> -Eric
>>>
>>> The first thing that comes to mind is maybe they trying to distinguish
>>> between a fshandle or handle. Or they we trying to be consistent with
>>> the allocation calls.
>>>
>>> The libhandle free_handle has the same calling parameters. It also does
>>> nothing with the length. That we cannot change without breaking existing
>>> code.
>>>
>>> I will look/ask around.
>>>
>>> --Mark.
>>
>> Looks like the code is pretty sloppy with freeing the handles.
>
> yeah, that's what I was going to fix :)
>
>> Best guess is jdm_delete_filehandle() and free_handle are trying to
>> keep the API similar to DMAPI. The DMAPI handle free routine,
>> dm_handle_free(), also has a second length parameter that is not used
>> in the library.
>>
>> The code example that I saw are similar to the use in xfsdump, where
>> the length used in the free comes from the handle creation/conversion
>> routine.
>
> yup but I don't think jdm_getfshandle has anything similar does it?

nope. Do you know why there is a jdm and a libhandle libs?

>
>> Since the xfsprogs do not open handles with calls that provide a
>> length. How about FSHANDLE_SZ and FILEHANDLE_SZ depending on if it is
>> a xfs_fshandle or xfs_handle?
>
> *shrug* it's not used anyway - I'm not sure why we'd need to invent a
> macro to pass in only to have it ignored.  Is there any advantage to that?

never mind... handles are opaque and we should not be defining a size.

I did the grep and saw that the sizes were defined and thought they were 
better than nothing. I did not not realize that the defines are are in 
jdm.c and not a header file. In that case, nothing is better than adding 
a define for an opaque item.

> -Eric

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Dave Chinner]

On Tue, Jul 29, 2014 at 12:31:43PM -0500, Eric Sandeen wrote:
> I was cleaning up xfsprogs to plug some leaks, and wanted to use
> jdm_delete_filehandle().  I noticed that it has an "hlen" argument which
> is unused.
> 
> Can we remove that, or is this part of a public API?  It's not in any
> manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
> but it is in a public header...

Which means it is part of the published API, regardless of whether
it is documented in the man page or not.

As it is, we've never really cared about ABI stability for these
libraries - I don't think we've ever guaranteed stability in any
way. If you want to change it, though, I suspect we'll need to bump
the library version....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Christoph Hellwig]

On Tue, Jul 29, 2014 at 12:31:43PM -0500, Eric Sandeen wrote:
> I was cleaning up xfsprogs to plug some leaks, and wanted to use
> jdm_delete_filehandle().  I noticed that it has an "hlen" argument which
> is unused.
> 
> Can we remove that, or is this part of a public API?  It's not in any
> manpage (or even called anywhere in xfsprogs/xfstests/xfsdump/dmapi)
> but it is in a public header...
> 
> anyone know?
> 
> If needed I guess I can just call it with hlen==0, but that seems odd.

If it's in a public header we shouldn't touch it unless there's a
pressing need.


Talking about libhandle: this one has been a bit bitrotted.  Maybe it's
a good time to move everything over to the kernel by handle syscalls
and deprecated it?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Is jdm_delete_filehandle part of a public API?

[Dave Chinner]

On Fri, Aug 01, 2014 at 06:53:05AM -0700, Christoph Hellwig wrote:
> Talking about libhandle: this one has been a bit bitrotted.  Maybe it's
> a good time to move everything over to the kernel by handle syscalls
> and deprecated it?

First you need to make the kernel by-handle interfaces handle the
sae functionality as the XFS by-handle ioctls.

	1. O_NOCMTIME doesn't exist, and so there's no way to do
	   invisible IO on files.
	2. Can we construct VFs filehandles in userspace from
	   bulkstat information (dump, xfs_fsr and others rely on
	   this capability)?
	3. The kernel by-handle interfaces cannot manipulate
	   attributes at all (i.e. attr-list, attr-multi
	   functionality).

So until the VFS by-handle interfaces can do these things, we can't
change libhandle over to use the newer kernel interfaces.

I'm also pretty sure the incompatible handle format would mean
big problems if someone were to be storing file handles in userspace
and they upgrade their version of libhandle. i.e. such a library
implementation change is not forwards or backwards compatible with
existing applications, but bumping the major shared library version
should solve that problem.

However, these are all solvable problems. It's definitely a low
priority for me to do this, but if you want to do it patches will
definitely be accepted ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs