On 11/08/2015 20:54, Matthew Ahrens wrote:
> On Fri, Aug 7, 2015 at 7:18 AM, Andriy Gapon <[email protected]
> <mailto:[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
Indeed. So, you have me convinced.
The only potentially unfortunate side effect is that struct drr_begin would
become a part of the API. We could try to make it opaque to libzfs_core users,
though, by providing a function to read the record from a file descriptor and
then providing a number of accessor functions to extract useful values from the
record. But not sure if that approach is worth the trouble, it certainly seems
a bit cumbersome.
I'm attaching my current work-in-progress on implementing your suggestion (as I
understood it, possibly incorrectly). 'lzc_receive_basic' is the best name with
which I could come up, but it's probably not the best name. I had a trouble
picking up a name that would be expressive (of the intent) or descriptive (of
the behavior) enough while still being not very long.
--
Andriy Gapon
--- a/usr/src/lib/libzfs_core/common/libzfs_core.h
+++ b/usr/src/lib/libzfs_core/common/libzfs_core.h
@@ -67,6 +67,11 @@ enum lzc_send_flags {
int lzc_send(const char *, const char *, int, enum lzc_send_flags);
-int lzc_receive(const char *, nvlist_t *, const char *, boolean_t, int);
+
+struct drr_begin;
+
+int lzc_receive(const char *, nvlist_t *, const char *, boolean_t, int,
+ const struct drr_begin *);
+int lzc_receive_basic(const char *, nvlist_t *, const char *, boolean_t, int);
int lzc_send_space(const char *, const char *, uint64_t *);
int lzc_send_progress(const char *, int, uint64_t *);
--- a/usr/src/lib/libzfs_core/common/libzfs_core.c
+++ b/usr/src/lib/libzfs_core/common/libzfs_core.c
@@ -625,6 +625,11 @@ recv_read(int fd, void *buf, int ilen)
* flag will cause the target filesystem to be rolled back or destroyed if
* necessary to receive.
*
+ * lzc_receive_basic reads the whole stream while lzc_receive allows the
+ * caller to read the begin record and then to pass it in. That could be
+ * useful if the caller wants to derive, for example, the snapname or the
+ * origin parameters based on the information contained in the begin record.
+ *
* Return 0 on success or an errno on failure.
*
* Note: this interface does not work on dedup'd streams
@@ -632,7 +637,7 @@ recv_read(int fd, void *buf, int ilen)
*/
int
lzc_receive(const char *snapname, nvlist_t *props, const char *origin,
- boolean_t force, int fd)
+ boolean_t force, int fd, const struct drr_begin *begin_record)
{
/*
* The receive ioctl is still legacy, so we need to construct our own
@@ -642,7 +647,6 @@ lzc_receive(const char *snapname, nvlist_t *props, const char *origin,
char *atp;
char *packed = NULL;
size_t size;
- dmu_replay_record_t drr;
int error;
ASSERT3S(g_refcount, >, 0);
@@ -678,10 +682,7 @@ lzc_receive(const char *snapname, nvlist_t *props, const char *origin,
(void) strlcpy(zc.zc_string, origin, sizeof (zc.zc_string));
/* zc_begin_record is non-byteswapped BEGIN record */
- error = recv_read(fd, &drr, sizeof (drr));
- if (error != 0)
- goto out;
- zc.zc_begin_record = drr.drr_u.drr_begin;
+ zc.zc_begin_record = *begin_record;
/* zc_cookie is fd to read from */
zc.zc_cookie = fd;
@@ -703,6 +704,21 @@ out:
return (error);
}
+int
+lzc_receive_basic(const char *snapname, nvlist_t *props, const char *origin,
+ boolean_t force, int fd)
+{
+ dmu_replay_record_t drr;
+ int error;
+
+ error = recv_read(fd, &drr, sizeof (drr));
+ if (error == 0) {
+ error = lzc_receive(snapname, props, origin, force, fd,
+ &drr.drr_u.drr_begin);
+ }
+ return (error);
+}
+
/*
* Roll back this filesystem or volume to its most recent snapshot.
* If snapnamebuf is not NULL, it will be filled in with the name
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer
