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

Added in --sendoptions=OPTIONS and --recvoptions=OPTIONS to inject OP… #295

Merged
merged 1 commit into from Jan 22, 2019

Conversation

Projects
None yet
7 participants
@clinta
Copy link
Contributor

clinta commented Nov 29, 2018

This is a cleaned up rebase of @mattaw's PR #273.

@jimsalterjrs

This comment has been minimized.

Copy link
Owner

jimsalterjrs commented Dec 4, 2018

@phreaker0 @clinta needs rebase and review afterward.

@clinta

This comment has been minimized.

Copy link
Contributor Author

clinta commented Dec 5, 2018

@jimsalterjrs @phreaker0 rebased again.

@mattaw

This comment has been minimized.

Copy link
Contributor

mattaw commented Dec 7, 2018

Thankyou so much, I got buried by the end of our semester and was planning to tackle it again near Christmas.

@jimsalterjrs

This comment has been minimized.

Copy link
Owner

jimsalterjrs commented Dec 13, 2018

Still have conflicts that need to be resolved.

@TerraTech TerraTech referenced this pull request Dec 18, 2018

Open

Add option --no-mount #255

@TerraTech

This comment has been minimized.

Copy link
Contributor

TerraTech commented Dec 23, 2018

I believe this needs a bit more fine tuning where some options need to be masked depending on the dataset being transferred.

Case in point is applying: --recvoptions="-o canmount=noauto"
which will work fine for filesystem types, but will throw an error if a volume type is being processed.

I'm thinking that a possible solution is to extend the options to provide specificity, e.g.

  • --recvoptions=
  • --recvoptions-filesystem=
  • --recvoptions-volume=

or if you don't want to balloon the number of options, then a additive building parser can be done:

  • --recvoptions=""
  • --recvoptions=":filesystem:-o canmount=noauto"

where the applied options is scoped to the embedded :identifier: (I'm open to alternative delimiters)

The internal hash would be a HOL:

%recvoptions = (
  common => (...),
  filesystem => (...),
  volume => (...)
);

Then the correct options could be applied via introspection:

my $fstype = getzfsvalue($sourcehost,$sourcefs,$sourceisroot,'type');
if ($fstype eq 'volume') {
  $receiveextraargs = join(' ', @recvoptions{common,volume});
}
elsif ($fstype eq 'filesystem') {
  $receiveextraargs = join(' ', @recvoptions{common,filesystem});
}
else {
  $receiveextraargs = join(' ', $recvoptions{common});
}
Added in --sendoptions=OPTIONS and --recvoptions=OPTIONS to inject OP…
…TIONS enabling things like syncoid --sendoptions=-Lcep and syncoid --recvoptions="-x property"

Co-authored-by: Clint Armstrong <clint@clintarmstrong.net>

@clinta clinta force-pushed the clinta:zfs_send_recv_options branch from 3674585 to bd4eb49 Dec 23, 2018

@clinta

This comment has been minimized.

Copy link
Contributor Author

clinta commented Dec 23, 2018

Rebased again. Perhaps volume and filesystem specific options can be added in another another PR.

@clinta

This comment has been minimized.

Copy link
Contributor Author

clinta commented Jan 7, 2019

@jimsalterjrs @phreaker0

Would it be possible to get this reviewed and merged before it falls out of sync and needs another rebase?

@efinley1272

This comment has been minimized.

Copy link

efinley1272 commented Jan 7, 2019

@phreaker0
Copy link
Collaborator

phreaker0 left a comment

I tested the PR and it works for me. I still don't like the fact that it will probably break stuff like resume able replication and such, but it's marked as danger/power option so I am fine with it. I'm thinking of an follow up PR with an whitelist of parameters which can be used in all the all the send/recv cases and automatically removing invalid options from the provided ones.

@mailinglists35

This comment has been minimized.

Copy link

mailinglists35 commented Jan 30, 2019

for my use case of "send -Lc" and "receive -vu" it does not break resumable replication

@mailinglists35

This comment has been minimized.

Copy link

mailinglists35 commented Jan 30, 2019

I see it correctly gets a resume token and if interrupted it correctly picks up the token and completes the send.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment