[mythtv-commits] Ticket #12602: lossless transcode that fills the disk loses the tail of the recording

[MythTV]

#12602: lossless transcode that fills the disk loses the tail of the recording
-------------------------------------+-------------------------------------
     Reporter:  Timothy D Witham     |      Owner:  jyavenard
  <twitham@…>                        |
         Type:  Bug Report -         |     Status:  new
  General                            |
     Priority:  minor                |  Milestone:  unknown
    Component:  MythTV -             |    Version:  0.27.5
  Mythtranscode                      |   Keywords:  disk full transcode
     Severity:  medium               |  truncate
Ticket locked:  0                    |
-------------------------------------+-------------------------------------
 Backup your test recording first!  Or use a worthless recording.

 Fill your disk so that free space exists but is less than the size of the
 large test recording you are about to transcode.  Now queue a lossless
 transcode to permanently remove a cutlist.  When disk gets completely full
 during the transcode, this error is not detected, and the rename of .tmp
 file is performed anyway, overwriting the original with the failed file.
 Of course this permanently loses the end of the recording.  I see no
 evidence of the full disk or related error in mythtranscode.log, but it
 does clearly log the final rename.

 I see in some of the code where the write is carefully checked for failure
 but the close is not:


 {{{
  1163     int ret = write(fh, data, size);
  1164     if (ret < 0)
  1165         LOG(VB_GENERAL, LOG_ERR, QString("write failed
 %1").arg(filename) +
  1166                 ENO);
  1167     close(fh);

 }}}

 The problem could be that the write silently succeeds and the ENOSPC error
 would actually be reported by the close instead.  From write(2) and
 close(2):

 {{{
 NOTES
      A successful return from write() does not make any guarantee
      that  data has  been committed  to disk.   In fact,  on some
      buggy implementations, it does not even guarantee that space
      has successfully  been reserved for the data.

 NOTES
      Not checking  the return  value of close()  is a  common but
      nevertheless  serious   programming  error.   It   is  quite
      possible that  errors on  a previous write(2)  operation are
      first  reported  at the  final  close().   Not checking  the
      return value when  closing the file may lead  to silent loss
      of data.
 }}}

 If this is indeed the root cause, then the fix is easy: check the return
 value of the close just like you did the write previous to it and return
 an unsuccessful exit that will prevent the rename.  I see this in at least
 mpeg2fix.cpp but it could be a good idea anywhere that writes are done to
 disk.

 Of course my workaround is to check disk headroom before doing a
 transcode, or to maintain headroom larger than largest file I will
 transcode.  But this requires more headroom than maximum recording rate
 requires.  I think auto-expire happens only during recording, so it is not
 making more space during a transcode.  Transcode of 4 hours of FHD could
 need 20GB or more of free space.

--
Ticket URL: <https://code.mythtv.org/trac/ticket/12602>
MythTV <http://www.mythtv.org>
MythTV Media Center