Re: [PATCH] Parallelize sysupgrade downloads

[Jarkko Oranen]

On 5/7/19 10:39 PM, Ville Valkonen wrote:
Hello,
thanks for the great new tool, sysupgrade. Works like a charm.

While on it, I came with this patch to speed up the downloading.
It uses xargs -P to parallelize downloads (max 6, chosen from top of my head).

Also, removed trailing spaces in two lines.

Without parallel patch:
     4m20.91s real     0m00.44s user     0m24.26s system
With parallel patch:
     3m14.14s real     0m00.32s user     0m13.80s system

I wonder if saving a minute is worth the trouble. However, there seems 
to be a more important problem with the patch. See below


<snip>
diff --git usr.sbin/sysupgrade/sysupgrade.sh usr.sbin/sysupgrade/sysupgrade.sh
index 87de9ccf432..635b48186f5 100644
--- usr.sbin/sysupgrade/sysupgrade.sh
+++ usr.sbin/sysupgrade/sysupgrade.sh
@@ -44,7 +44,7 @@ unpriv()
          _file=$2
          shift 2
      fi
-     if [[ -n ${_file} ]]; then
+    if [[ -n ${_file} ]]; then
          >${_file}
          chown "${_user}" "${_file}"
      fi
@@ -69,6 +69,16 @@ rmel() {
      echo -n "$_c"
  }

+generate_urls()
+{
+    OFS=$IFS
+    IFS=$'\n'
+    printf "${2}\n" | while read -r f; do
+        printf "%s%s\n" "${1}" "${f}"
+    done
+    IFS=$OFS
+}
+
  RELEASE=false
  SNAP=false
  FORCE=false
@@ -122,7 +132,7 @@ if [[ -e ${SETSDIR} ]]; then
           ug_err "${SETSDIR} needs to be owned by root:wheel"
      [[ $st_gid -eq 0 ]] ||
           ug_err "${SETSDIR} needs to be owned by root:wheel"
-    [[ $st_mode -eq 040755 ]] ||
+    [[ $st_mode -eq 040755 ]] ||
          ug_err "${SETSDIR} is not a directory with permissions 0755"
  else
      mkdir -p ${SETSDIR}
@@ -167,9 +177,13 @@ for f in $SETS; do
  done

  [[ -n ${OLD_FILES} ]] && rm ${OLD_FILES}
+cd ${SETSDIR}
+generate_urls $URL "$(echo ${DL} |tr ' ' '\n')" \
+    | xargs -P 6 -n 1 ftp -VCm

Doesn't this download the files as root without using unpriv to drop the 
downloader's privileges? This doesn't seem correct.

  for f in ${DL}; do
-    unpriv -f $f ftp -Vmo ${f} ${URL}${f}
+    unpriv -f $f

Does unpriv do anything if you don't give it a command to run? These are 
probably no-ops.

  done
+cd -

  echo Verifying sets.
  [[ -n ${DL} ]] && unpriv cksum -qC SHA256 ${DL}
</snip>

--
Kind regards,
Ville Valkonen


--
Jarkko