[Mondo-devel] Woohoo got the tape verify working ...

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Mondo-devel] Woohoo got the tape verify working ...

David C. Partridge
I initially tried code like:

  if ((length_of_incoming_file <= 0)  &&
     !(BLK_START_OF_TAPE == controlchar &&
        BLK_START_OF_BACKUP == controlchar))      
 {
                return(1);
        }removed the

but that didn't work as  got a lot of header block type comparison failures
before it all fell over in a heap.

Just removing the following lines in libmondo-stream.c at line 1764
   if (length_of_incoming_file <= 0) {
                return(1);
        }

wrote a tape that verified successfully!

Dave



------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Mondo-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/mondo-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Mondo-devel] Woohoo got the tape verify working ...

Bruno Cornec-4
Hello David,

First of all , you need to know that I'm the maintainer of MondoRescue, not the original author. While I've modified a lot the code since 2005 when I took over the maintenance, I'm still not aware of all the code. In particular, I've not written the tape management part (except OBDR support). So sometimes, I have no much more clue than yourself ! You'll have to bear with this :-)

So now, looking at your findings, I'm happy that you found a way to solve your issue. But we need to investigate more before being able to adopt your solution I think.

First svn blame helps seeing where it comes from: (enlarge your window to see that correctly)

     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005) /**
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * Write a header block to the opened stream (CD or tape).
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * @param length_of_incoming_file The length to store in the header block.
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * Usually matters only if this is a file-related header, in which case it should
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * be the length of the file that will follow.
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * @param filename The filename to store in the header block. Usually matters
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * only if this is a file-related header, in which case this should be the name
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * if the file that will follow.
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * @param control_char The type of header block this is (start-file, end-file, start-tape, ...)
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  * @return 0 for success, nonzero for failure.
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)  */
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005) int
   684    bcornec 2006-06-26 13:12:48 +0200 (lun. 26 juin 2006) write_header_block_to_stream(off_t length_of_incoming_file,
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)                                                          char *filename, int control_char)
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005) {
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)         /*@ buffers **************************************************** */
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)         char tempblock[TAPE_BLOCK_SIZE];
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)         char *p;
  3611      bruno 2016-11-10 21:09:54 +0100 (jeu. 10 nov. 2016)         char *tmp = NULL;
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)         /*@ int ******************************************************** */
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)         int i;
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)
   684    bcornec 2006-06-26 13:12:48 +0200 (lun. 26 juin 2006)         off_t olen;
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)
   128    bcornec 2005-11-19 02:27:41 +0100 (sam. 19 nov. 2005)         /*@ end vars *************************************************** */
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)
  3296      bruno 2014-06-06 05:44:16 +0200 (ven. 06 juin 2014)         if (length_of_incoming_file <= 0) {
  3296      bruno 2014-06-06 05:44:16 +0200 (ven. 06 juin 2014)                 return(1);
  3296      bruno 2014-06-06 05:44:16 +0200 (ven. 06 juin 2014)         }
     1    bcornec 2005-09-06 12:20:07 +0200 (mar. 06 sept. 2005)

So most of that code is from the origins of the project !!
I've added the block you mention which is creating an issue in 2014.
The reason is given in the svn log:

------------------------------------------------------------------------
r3296 | bruno | 2014-06-06 05:44:16 +0200 (ven. 06 juin 2014) | 1 ligne
Chemins modifiés :
   M /branches/3.0/mondo/src/common/libmondo-stream.c
   M /branches/3.2/mondo/src/common/libmondo-stream.c

Fix #747 by returning early enough to avoid using a bad value for file length
------------------------------------------------------------------------

You can see it better at http://trac.mondorescue.org/changeset/3296

That bug 747 http://trac.mondorescue.org/ticket/747 was something I encountered and fixed that way.
So it seems it has at the same time broken the verify phase :-(

So we need to find a way to both not break the verify phase, and to avoid crashing when a big file is removed during archiving.


David C. Partridge said on Thu, Dec 22, 2016 at 11:25:16AM +0000:
>Just removing the following lines in libmondo-stream.c at line 1764
>   if (length_of_incoming_file <= 0) {
> return(1);
> }

So from the embedded doc (which I didn't write):

 * @param length_of_incoming_file The length to store in the header block.
 * Usually matters only if this is a file-related header, in which case it should
 * be the length of the file that will follow.

If I go back to the first commit, there was commented code that maybe in fact relevant:

  memcpy (tempblock + 7001, (char *) &olen, sizeof (long long));
/*  if (length_of_incoming_file) {memcpy(tempblock+7001,(char*)&length_of_incoming_file,sizeof(long long));} */

So maybe we should change back all that and indeed remove the block I added as it returns too early in the function so doesn't write all the headers needed, but still not do the memcpy when length_of_incoming_file <= 0.

So my proposal would be now this one:

Index: mondo/src/common/libmondo-stream.c
===================================================================
--- mondo/src/common/libmondo-stream.c  (révision 3639)
+++ mondo/src/common/libmondo-stream.c  (copie de travail)
@@ -1760,10 +1760,6 @@
 
        /*@ end vars *************************************************** */
 
-       if (length_of_incoming_file <= 0) {
-               return(1);
-       }
-
        olen = length_of_incoming_file;
        p = strrchr(filename, '/');     /* Make 'em go, "Unnnh!" Oh wait, that was _Master_ P... */
        if (!p) {
@@ -1780,7 +1776,9 @@
        }
        sprintf(tempblock + 6000 + control_char, STR_HEADER);
        tempblock[7000] = control_char;
-       memcpy(tempblock + 7001, (char *) &olen, sizeof(off_t));
+       if (length_of_incoming_file > 0) {
+               memcpy(tempblock + 7001, (char *) &olen, sizeof(off_t));
+       }
        strcpy(tempblock + 1000, filename);
        g_tape_posK += fwrite(tempblock, 1, (size_t) TAPE_BLOCK_SIZE, g_tape_stream) / 1024;
        tmp = marker_to_string(control_char);


Could you try to make this additional change (around line 1780) as you already have the first part, and see whether that still works for you, as I think it would also cover #747 as well.

Anyway, many thanks for passing so much time digging into it, and founding the root issue of the problem. Hopefully we'll be able to fix it for good soon.
If you prefer to use direcly my latest versions then use the files at ftp://ftp.mondorescue.org/test/ubuntu/16.04/
That version should also fix ther other problems reported by you.

Best regards,
Bruno.
--
Open Source Profession, WW Linux Community Lead     http://www.hpintelco.net
HPE EMEA EG FLOSS Technology Strategist http://www.hpe.com/engage/opensource
FLOSS projects:    http://mondorescue.org         http://project-builder.org
Musique ancienne?   http://www.musique-ancienne.org  http://www.medieval.org

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Mondo-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/mondo-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Mondo-devel] Woohoo got the tape verify working ...

David C. Partridge
Perhaps an even simpler change to the code that requests the writing of the
header blocks would allow the <=0 test to be re-instated.

I'm working on the basis that the length field is only used for file related
headers (those followed by file data).

What if any problem would be caused if the "non-file-related" headers
specified a length of 1?  So long as no-one actually tried to use it ...

Dave
-----Original Message-----
From: Bruno Cornec [mailto:[hidden email]]
Sent: 23 December 2016 15:28
To: Mondo mailing list
Subject: Re: [Mondo-devel] Woohoo got the tape verify working ...

Hello David,

First of all , you need to know that I'm the maintainer of MondoRescue, not
the original author. While I've modified a lot the code since 2005 when I
took over the maintenance, I'm still not aware of all the code. In
particular, I've not written the tape management part (except OBDR support).
So sometimes, I have no much more clue than yourself ! You'll have to bear
with this :-)

So now, looking at your findings, I'm happy that you found a way to solve
your issue. But we need to investigate more before being able to adopt your
solution I think.

First svn blame helps seeing where it comes from: (enlarge your window to
see that correctly)

:
: stuff deleted
:

Best regards,
Bruno.


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Mondo-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/mondo-devel
Loading...