allanjude / zxfer

A continuation of development on zxfer, a popular script for managing ZFS snapshot replication
BSD 2-Clause "Simplified" License
122 stars 40 forks source link

Two issues with zfs user properties: Too many arguments and pipe character in value #14

Open abochem opened 9 years ago

abochem commented 9 years ago

I am using iocage to manage jails, which stores the jail configuration in zfs user properties. The jail command parameters ip4.addr and ip6.addr are stored verbatim in a zfs user property, the value formatted as <interface>|<ip address>/<mask>. Note the pipe character in this string.

Zxfer runs into the following error(s) when trying to replicate the jail dataset to a remote host:

eval: 172.20.7.11/24: not found
too many arguments

Using dry run I could see that zfs property values are not escaped, and indeed there are a load of user properties that make the list of -o arguments very long.

I could make this use case work for me by making two changes to the code:

  1. Put single quotes around the values of zfs properties given via the -o option list to escape the pipe character.
  2. Remove eval from the lines actually executing the zfs create command.

eval would still stumble over the pipe character even with single quotes, and throw the too many arguments error on top of it. Without eval, single quotes are still neccessary, as sh would stumble over the pipe character otherwise.

In dry run I did not see any double $ variable expansion requiring a second parse through eval. But maybe I missed a reason why eval should be there anyway? It breaks my use case, though.

The diff below shows my changes against zxfer 1.1.0. I browsed 1.1.3 in the repository and found the lines in question still the same as in 1.1.0 except on different line numbers.

root@warden:~ # diff /usr/local/sbin/zxfer zxfer
1308c1308
<       override_option_list=$(echo "$new_rmvs_pv" | sed -e 's/,/ -o /g')

---
>       override_option_list=$(echo "$new_rmvs_pv" | tr "," "\n" | sed "s/\(.*\)=\(.*\)/\1=\'\2\'/g" | tr "\n" "," | sed 's/,$//' | sed -e 's/,/ -o /g')
1323c1323
<         eval "$RZFS create $override_option_list $actual_dest" || \

---
>         $RZFS create $override_option_list $actual_dest || \
1341c1341
<       creation_option_list=$(echo "$new_rmvs_pv" | sed -e 's/,/ -o /g')

---
>       creation_option_list=$(echo "$new_rmvs_pv" | tr "," "\n" | sed "s/\(.*\)=\(.*\)/\1=\'\2\'/" | tr "\n" "," | sed 's/,$//' | sed -e 's/,/ -o /g')
1355c1355
<         eval "$RZFS create -p $creation_option_list $actual_dest" || \

---
>         $RZFS create -p $creation_option_list $actual_dest || \
abochem commented 9 years ago

Addendum: With my changes, backup does work, but restore does not. (Without my changes, there is still the error due to the unescaped pipe character.)

However, when I manually c&p the zfs create ... command lines from dry-run output, they work just fine and create the datasets without error.

  1. When restoring the parent dataset (having none of the zfs user properties set by iocage), I get an error: Creating destination filesystem "crypt/iocage/jails" with specified properties. cannot create 'crypt/iocage/jails': bad numeric value ''none''
  2. When restoring the jail dataset itself (holding lots of zfs user properties set by iocage), I still get the too many arguments error.

My current workaround is using dry-run, c&p the zfs create ... commands from the output to manually create the datasets, then run zxfer without dry-run for the actual restore. Luckily, restore doesn't happen that often.

allanjude commented 9 years ago

if you run the script with sh -x, it will show how it is interpreting the command and will likely help to find out what is happening.

The way the code is now manages to handle spaces, but not some other special characters.

I think the 'none' error is for the 'filesystem_limits' property. This is a new property that zxfer hasn't been updated to deal with yet.

allanjude commented 8 years ago

I think this is fixed by the merge of #25

utrenkner commented 7 years ago

I am also having the problem with iocage's use of pipe characters org.freebsd.iocage:ip4_addr lo1|10.0.0.11 breaks zxfer:Creating destination filesystem "backup0/jails/05911cf9-2fdf-11e7-9fcc-3c970e109aff" with specified properties. eval: 10.0.0.11: not found missing filesystem argument While the shell bases iocage is going away, maybe you could still roll the fix into a new release of zxfer? For now, I will give the fix a try...