Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Illumos 2605 want to resume interrupted zfs send [rebase February 29th] #4396

Closed

Conversation

kernelOfTruth
Copy link
Contributor

Reviewed by: George Wilson [email protected]
Reviewed by: Paul Dagnelie [email protected]
Reviewed by: Richard Elling [email protected]
Reviewed by: Xin Li [email protected]
Reviewed by: Arne Jansen [email protected]
Approved by: Dan McDonald [email protected]

References:
https://www.illumos.org/issues/2605
illumos/illumos-gate@9c3fd12

Porting notes:

The tests from Illumos in usr/src/test/zfs-tests were left out
Further the following files which don't exist in ZoL:
usr/src/cmd/truss/expound.c
usr/src/lib/libzfs/common/mapfile-vers
usr/src/lib/libzfs_core/common/mapfile-vers
usr/src/pkg/manifests/system-test-zfstest.mf

From a readability point of view the changes in [module/zfs/dmu_traverse.c]
from ecfb0b5 Fix misuse of input argument in traverse_visitbp
appear to be the optimum - also there's no functional change,
when going with upstream the compiler would likely complain about ISO C90 errors - so only switching
cdnp = buf->b_data;
to
child_dnp = buf->b_data;

diverged code base from Illumos:

[cmd/zstreamdump/zstreamdump.c]
37f8a88 Illumos 5746 - more checksumming in zfs send

[include/sys/zfs_ioctl.h]
0cee240 Speed up 'zfs list -t snapshot -o name -s name'

[module/zfs/zfs_ioctl.c]
13fe019 Illumos 3464 zfs synctask code needs restructuring

[module/zfs/dmu_send.c]
b58986e Use large stacks when available
044baf0 Use taskq for dump_bytes()
ISO C90 - mixed declarations and code: struct send_block_record *to_data;
struct receive_ign_obj_node *n;
account for 'missing braces around initializer' in 'struct send_thread_arg to_arg ='
ISO C90 - mixed declarations and code: void *payload = NULL;
nvlist_t *nvl = fnvlist_alloc();
uint64_t one = 1; uint64_t zero = 1;
char recvname[ZFS_MAXNAMELEN];
uint64_t val;
uint32_t payloadlen = drc->drc_drr_begin->drr_payloadlen;
size_t payload_len = 0;

[module/zfs/dsl_dataset.c]
Illumos 4185 add new cryptographic checksums to ZFS: SHA-512, Skein, Edon-R
hasn't landed yet, thus
' #include <sys/zio_checksum.h>
is still missing,

' #include <sys/dmu_send.h>
however is needed

further
replaced
fletcher_4_native(compressed, compressed_size, NULL, &cksum);
with
fletcher_4_native(compressed, compressed_size, &cksum);

ctx_template argument was removed (purpose: the template will be reused to make initialization more efficient)

C99 or C11 mode: declare 'int i' at the beginning of the function.

ISO C90 - mixed declarations and code: char buf[256];
zio_cksum_t cksum;
char *propval = kmem_asprintf("%u-%llx-%llx-%s",
char recvname[ZFS_MAXNAMELEN];
dsl_dataset_t *recv_ds;

[libzfs_sendrecv.c]
' #include <sha2.h>
isn't listed in the header list since sha2, sha256 is implemented differently in ZoL.

replace
fletcher_4_native(compressed, len, NULL, &cksum);
with
fletcher_4_native(compressed, len, &cksum);

declare 'int nread, i;' at the beginning of the function

b8864a2 Fix gcc cast warnings

[module/zcommon/zfs_prop.c]
11b9ec2 Add full SELinux support

[module/zfs/dmu_traverse.c]
ecfb0b5 Fix misuse of input argument in traverse_visitbp
79c76d5 Change KM_PUSHPAGE -> KM_SLEEP
a168788 Reduce stack for traverse_visitbp() recursion
b8d06fc Switch KM_SLEEP to KM_PUSHPAGE
47050a8 Fix stack traverse_impl()
ISO C90: place 'uint64_t obj, error;' at the beginning, further replace 'err' with 'error'

[man/man8/zfs.8]
Part of the sections for 'zfs receive' and 'zfs send' was rewritten and reordered to approximate upstream.

Ported-by: kernelOfTruth [email protected]

Reviewed by: George Wilson <[email protected]>
Reviewed by: Paul Dagnelie <[email protected]>
Reviewed by: Richard Elling <[email protected]>
Reviewed by: Xin Li <[email protected]>
Reviewed by: Arne Jansen <[email protected]>
Approved by: Dan McDonald <[email protected]>

References:
https://www.illumos.org/issues/2605
illumos/illumos-gate@9c3fd12

Porting notes:

The tests from Illumos in usr/src/test/zfs-tests were left out
Further the following files which don't exist in ZoL:
usr/src/cmd/truss/expound.c
usr/src/lib/libzfs/common/mapfile-vers
usr/src/lib/libzfs_core/common/mapfile-vers
usr/src/pkg/manifests/system-test-zfstest.mf

From a readability point of view the changes in [module/zfs/dmu_traverse.c]
from openzfs@ecfb0b5 Fix misuse of input argument in traverse_visitbp
appear to be the optimum - also there's no functional change,
when going with upstream the compiler would likely complain about ISO C90 errors - so only switching
cdnp = buf->b_data;
to
child_dnp = buf->b_data;

diverged code base from Illumos:

[cmd/zstreamdump/zstreamdump.c]
openzfs@37f8a88 Illumos 5746 - more checksumming in zfs send

[include/sys/zfs_ioctl.h]
openzfs@0cee240 Speed up 'zfs list -t snapshot -o name -s name'

[module/zfs/zfs_ioctl.c]
openzfs@13fe019 Illumos 3464 zfs synctask code needs restructuring

[module/zfs/dmu_send.c]
openzfs@b58986e Use large stacks when available
openzfs@044baf0 Use taskq for dump_bytes()
ISO C90 - mixed declarations and code: 	struct send_block_record *to_data;
					struct receive_ign_obj_node *n;
account for 'missing braces around initializer' in 'struct send_thread_arg to_arg ='
ISO C90 - mixed declarations and code: 	void *payload = NULL;
					nvlist_t *nvl = fnvlist_alloc();
					uint64_t one = 1; uint64_t zero = 1;
					char recvname[ZFS_MAXNAMELEN];
					uint64_t val;
					uint32_t payloadlen = drc->drc_drr_begin->drr_payloadlen;
					size_t payload_len = 0;

[module/zfs/dsl_dataset.c]
Illumos 4185 add new cryptographic checksums to ZFS: SHA-512, Skein, Edon-R
hasn't landed yet, thus
' #include <sys/zio_checksum.h>
is still missing,

' #include <sys/dmu_send.h>
however is needed

further
replaced
fletcher_4_native(compressed, compressed_size, NULL, &cksum);
with
fletcher_4_native(compressed, compressed_size, &cksum);

ctx_template argument was removed (purpose: the template will be reused to make initialization more efficient)

C99 or C11 mode: declare 'int i' at the beginning of the function.

ISO C90 - mixed declarations and code: 	char buf[256];
					zio_cksum_t cksum;
					char *propval = kmem_asprintf("%u-%llx-%llx-%s",
					char recvname[ZFS_MAXNAMELEN];
					dsl_dataset_t *recv_ds;

[libzfs_sendrecv.c]
' #include <sha2.h>
isn't listed in the header list since sha2, sha256 is implemented differently in ZoL.

replace
fletcher_4_native(compressed, len, NULL, &cksum);
with
fletcher_4_native(compressed, len, &cksum);

declare 'int nread, i;' at the beginning of the function

openzfs@b8864a2 Fix gcc cast warnings

[module/zcommon/zfs_prop.c]
openzfs@11b9ec2 Add full SELinux support

[module/zfs/dmu_traverse.c]
openzfs@ecfb0b5 Fix misuse of input argument in traverse_visitbp
openzfs@79c76d5 Change KM_PUSHPAGE -> KM_SLEEP
openzfs@a168788 Reduce stack for traverse_visitbp() recursion
openzfs@b8d06fc Switch KM_SLEEP to KM_PUSHPAGE
openzfs@47050a8 Fix stack traverse_impl()
ISO C90: place 'uint64_t obj, error;' at the beginning, further replace 'err' with 'error'

[man/man8/zfs.8]
Part of the sections for 'zfs receive' and 'zfs send' was rewritten and reordered to approximate upstream.

Ported-by: kernelOfTruth [email protected]
@kernelOfTruth
Copy link
Contributor Author

referencing: #3896

@kernelOfTruth
Copy link
Contributor Author

Appears like the (TEST) buildbot runs don't like the stress ?

in the last pull-request several of them also passed out - hm ...

I wonder what this has to do with this change

@behlendorf
Copy link
Contributor

@kernelOfTruth thanks for working on this. It appears zfstress is triggering a NULL dereference with this patch. You can see the backtrace in the Amazon TEST console log. This patch is definitely on my list is things to review.

@behlendorf
Copy link
Contributor

Replaced by #4742.

@behlendorf
Copy link
Contributor

@kernelOfTruth @ptx0 the testing failure was caused by a buffer overrun in the original OpenZFS patch. Ironically after I identified the issue I discovered that a fix was applied to OpenZFS last week. I've added the upstream fix to the #4742 patch stack.

@kernelOfTruth
Copy link
Contributor Author

@behlendorf sorry for the long absence from the project, live's currently demanding that I focus solely on totally different things

I'm glad that this issue could be sorted out

Exciting changes are ahead (encryption, more hashes, avx acceleration, etc.) 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants