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.
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. 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
