Re: [PATCH] Parallelize sysupgrade downloads

2019-05-08 Thread Christian Weisgerber 
On 2019-05-09, Christian Weisgerber  wrote:

>> Does unpriv do anything if you don't give it a command to run?
>
> It should return an error code and abort the script.

And actually that's what it does.
(Sorry, I confused myself there for a moment.)

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: [PATCH] Parallelize sysupgrade downloads

2019-05-08 Thread Christian Weisgerber 
On 2019-05-08, Jarkko Oranen  wrote:

>> -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?

It should return an error code and abort the script.  I need to
investigate why that doesn't happen.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: [PATCH] Parallelize sysupgrade downloads

2019-05-08 Thread 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





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}


--
Kind regards,
Ville Valkonen



--
Jarkko

[PATCH] Parallelize sysupgrade downloads

2019-05-07 Thread Ville Valkonen 
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


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
 for f in ${DL}; do
-unpriv -f $f ftp -Vmo ${f} ${URL}${f}
+unpriv -f $f
 done
+cd -

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


--
Kind regards,
Ville Valkonen