On Fri, Aug 7, 2015 at 7:18 AM, Andriy Gapon <[email protected]>
wrote:

>
> Your proposal seems like a more flexible approach. One thing that bothers
> me a
> little bit is that every consumer of the new lzc_receive would have to
> duplicate
> the code for reading / interpreting the BEGIN record.
>

Yes, but every consumer might want to interpret the BEGIN record a
different way (i.e. use the fromsnap name in different ways to determine
the name of the snapshot to create)


>
> Possible alternative is to not read the begin record in userland at all
> and let
> the kernel driver do it. After all, the current lzc_receive just passes the
> record down to the kernel without any interpretation. That would require
> changing the ZFS_IOC_RECV interface, of course.
>

But then you'd have to teach the kernel all possible ways that userland
might want to specify the name to receive based on the fromsnap name in the
BEGIN record.  Today /sbin/zfs supports at least 4 ways of doing it:

 - specify full pool/fs@snap
 - specify pool/fs, get @snap from BEGIN
 - specify pool/fs -d, append /everything/but/the/poolname@snap from BEGIN
 - specify pool/fs -e, get /last_elem@snap from BEGIN

--matt


>
> On 04/08/2015 02:10, Matthew Ahrens wrote:
> > I don't think this is really the direction we want to go with
> libzfs_core; it
> > increases the complexity of the operation and the possible error cases.
> That
> > said, I understand why you want to do it since otherwise there's no way
> to set
> > the name based on the BEGIN record, using libzfs_core. Maybe the existing
> > lzc_receive is just the wrong interface, and it should take the BEGIN
> record
> > as an argument, forcing the caller to read and decode it (and decide on
> the
> > name based on it, if desired). We could then provide a convenience
> routine
> > that works like the current lzc_receive().
> >
> > What do you think?
> >
> >
> > --matt
> >
> > On Fri, Jul 3, 2015 at 10:46 AM, Andriy Gapon <
> [email protected]
> > <mailto:[email protected]>> wrote:
> >
> >
> >     Currently lzc_receive() requires that its 'snapname' argument is a
> >     snapshot name
> >     (contains '@').  zfs receive allows to specify just a dataset name
> and
> >     would try
> >     to deduce the snapshot name from the stream.  I propose to allow
> lzc_receive()
> >     to do the same.  That is quite easy to achieve, it requires only a
> small
> >     amount
> >     of logic, it does not require any additional system calls or any
> >     additional data
> >     from the stream.
> >     Below is a proof of concept patch that seems to work.
> >
> >     --- lib/libzfs_core/common/libzfs_core.c
> >     +++ lib/libzfs_core/common/libzfs_core.c
> >     @@ -721,9 +721,8 @@ lzc_receive
> >             /* zc_name is name of containing filesystem */
> >             (void) strlcpy(zc.zc_name, snapname, sizeof (zc.zc_name));
> >             atp = strchr(zc.zc_name, '@');
> >     -       if (atp == NULL)
> >     -               return (EINVAL);
> >     -       *atp = '\0';
> >     +       if (atp != NULL)
> >     +               *atp = '\0';
> >
> >             /* if the fs does not exist, try its parent. */
> >             if (!lzc_exists(zc.zc_name)) {
> >     @@ -734,9 +733,6 @@ lzc_receive
> >
> >             }
> >
> >     -       /* zc_value is full name of the snapshot to create */
> >     -       (void) strlcpy(zc.zc_value, snapname, sizeof (zc.zc_value));
> >     -
> >             if (props != NULL) {
> >                     /* zc_nvlist_src is props to set */
> >                     packed = fnvlist_pack(props, &size);
> >     @@ -754,6 +750,20 @@ lzc_receive
> >                     goto out;
> >             zc.zc_begin_record = drr.drr_u.drr_begin;
> >
> >     +       /* zc_value is full name of the snapshot to create */
> >     +       (void) strlcpy(zc.zc_value, snapname, sizeof (zc.zc_value));
> >     +
> >     +       /* if snapshot name is not provided try to take it from the
> stream */
> >     +       atp = strchr(zc.zc_value, '@');
> >     +       if (atp == NULL) {
> >     +               atp = strchr(zc.zc_begin_record.drr_toname, '@');
> >     +               if (atp == NULL)
> >     +                       return (EINVAL);
> >     +               if (strlen(zc.zc_value) + strlen(atp) >=
> sizeof(zc.zc_value))
> >     +                       return (ENAMETOOLONG);
> >     +               strcat(zc.zc_value, atp);
> >     +       }
> >     +
> >             /* zc_cookie is fd to read from */
> >             zc.zc_cookie = fd;
> >
> >
> >     --
> >     Andriy Gapon
> >     _______________________________________________
> >     developer mailing list
> >     [email protected] <mailto:[email protected]>
> >     http://lists.open-zfs.org/mailman/listinfo/developer
> >
> >
>
>
> --
> Andriy Gapon
>
>
>
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer

Reply via email to