Ticket #2267 (closed enhancement: fixed)

Opened 8 years ago

Last modified 8 years ago

Implement resuming file downloads (reget) in FISH VFS

Reported by: zaytsev Owned by: angel_il
Priority: major Milestone: 4.7.4
Component: mc-vfs Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: Votes for changeset: committed-master

Description

Hi!

Currently when the connection is dropped during the download, we get an incomplete file on the recipient drive. The framework for resuming file downloads is already present in the VFS layer, all it takes is to implement its support in FISH.

Thanks!

Change History

comment:1 Changed 8 years ago by angel_il

2 zaytsev: try

diff --git a/lib/vfs/mc-vfs/fish.c b/lib/vfs/mc-vfs/fish.c
index 3abe293..ec5f121 100644
--- a/lib/vfs/mc-vfs/fish.c
+++ b/lib/vfs/mc-vfs/fish.c
@@ -866,8 +866,6 @@ fish_linear_start (struct vfs_class *me, struct vfs_s_fh *fh, off_t offset)
     struct vfs_s_super *super = FH_SUPER;
     char *name;
     char *quoted_name;
-    if (offset)
-        ERRNOR (E_NOTSUPP, 0);
     name = vfs_s_fullpath (me, fh->ino);
     if (name == NULL)
         return 0;
@@ -882,8 +880,8 @@ fish_linear_start (struct vfs_class *me, struct vfs_s_fh *fh, off_t offset)
      * standard output (i.e. over the network).
      */
.
-    shell_commands = g_strconcat (SUP.scr_env, "FISH_FILENAME=%s;\n", SUP.scr_get, (char *) NULL);
-    offset = fish_command (me, super, WANT_STRING, shell_commands, quoted_name);
+    shell_commands = g_strconcat (SUP.scr_env, "FISH_FILENAME=%s FISH_OFFSET=%lli;\n", SUP.scr_get, (char *) NULL);
+    offset = fish_command (me, super, WANT_STRING, shell_commands, quoted_name, offset);
     g_free (shell_commands);
     g_free (quoted_name);
     if (offset != PRELIM)
diff --git a/lib/vfs/mc-vfs/fish/get b/lib/vfs/mc-vfs/fish/get
index 76b5b36..47d43d2 100755
--- a/lib/vfs/mc-vfs/fish/get
+++ b/lib/vfs/mc-vfs/fish/get
@@ -1,13 +1,23 @@
-#RETR $FISH_FILENAME
+#RETR $FISH_FILENAME $FISH_OFFSET
 FILENAME="/${FISH_FILENAME}"
 export LC_TIME=C
 if dd if="${FILENAME}" of=/dev/null bs=1 count=1 2>/dev/null ; then
-    ls -ln "${FILENAME}" 2>/dev/null | (
+    file_size=`ls -ln "${FILENAME}" 2>/dev/null | {
        read p l u g s r
        echo $s
-    )
+    }`
+    file_size=`expr $file_size - $FISH_OFFSET`
+    if [ $file_size -gt 0 ]; then
+        echo $file_size
+    else
+        echo 0
+    fi
     echo "### 100"
-    cat "${FILENAME}"
+    if [ $FISH_OFFSET -gt 0 ]; then
+        dd skip=$FISH_OFFSET ibs=1 if="${FILENAME}" 2>/dev/null
+    else
+        cat "${FILENAME}"
+    fi
     echo "### 200"
 else
     echo "### 500"

comment:2 follow-up: ↓ 3 Changed 8 years ago by ossi

specifying ibs=1 will make dd actually read one byte at a time. you will get a terrible throughput and still 100% cpu usage.
to get some efficiency, you'd have to use bigger blocks (10k or more) and round down the offset to the next multiple of the block size. the transfer of the extra bytes shouldn't be much of a problem ...

alternatively you could try 'tail -c +$FISH_OFFSET ...' - i don't know how portable this is, so you should look up the man pages of some non-gnu systems.

comment:3 in reply to: ↑ 2 Changed 8 years ago by angel_il

Replying to ossi:

alternatively you could try 'tail -c +$FISH_OFFSET ...' - i don't know how portable this is, so you should look up the man pages of some non-gnu systems.

Unfortunately, many embedded systems with busybox do not support 'tail -c'...
if the host supports perl, I'll try to use it...

comment:4 Changed 8 years ago by angel_il

branch: 2267_fish_resume_download
changeset: e2f94f73ae29f3a8a88d4567bcd1a900f6edd7cb

comment:5 Changed 8 years ago by angel_il

  • severity changed from no branch to on review

comment:6 Changed 8 years ago by slavazanko

  • Votes for changeset set to slavazanko

comment:7 Changed 8 years ago by slavazanko

  • Status changed from new to assigned

comment:8 Changed 8 years ago by zaytsev

1) Please rename fish_get_ -> fish_got_

2) Perl version: what if the device doesn't have enough memory to read everything in $content? Reget is mostly useful on huge files, like few gigabytes in size and this will most likely kill even desktop PCs not even speaking about embedded junk.

Am I missing something?

comment:9 Changed 8 years ago by angel_il

fish_get_perl fixed, please review

comment:10 Changed 8 years ago by angel_il

comment:11 Changed 8 years ago by zaytsev

1) Please rename fish_get_ -> fish_got_ (asked already)

2) Looks good. Does it make any sense to use $blksize instead of 4096? I don't know.

comment:12 Changed 8 years ago by angel_il

1) Please rename fish_get_ -> fish_got_ (asked already)

No, wherefore? in this case, 'get' is not verb is a name of method...

2) Looks good. Does it make any sense to use $blksize instead of 4096? I don't know.

I do not think it's a good idea to increase the size of the script, just to make the script look nicer...

comment:13 Changed 8 years ago by zaytsev

No, wherefore? in this case, 'get' is not verb is a name of method...

OK, my bad.

I do not think it's a good idea to increase the size of the script, just to make the script look nicer...

To decrease the size of the script, you could have named the return values of stat $a, $b, $c, $d etc. (at least for those, that are unused). My question is whether reading by block size instead of hardcoded 4096 will be faster or not. If it will, I think that has to be done.

comment:14 Changed 8 years ago by andrew_b

  • Votes for changeset changed from slavazanko to slavazanko andrew_b
  • severity changed from on review to approved

comment:15 Changed 8 years ago by angel_il

  • Status changed from assigned to testing
  • Votes for changeset changed from slavazanko andrew_b to committed-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:16 Changed 8 years ago by angel_il

  • Status changed from testing to closed

comment:17 Changed 8 years ago by andrew_b

  • Keywords vfs fish removed

Implemented in 4.7.0-stable in #2367.

Note: See TracTickets for help on using tickets.