[linux-lvm] pv_move_pe() error again :/

[Ragnar Kjørstad]

On Mon, Sep 10, 2001 at 01:51:37AM +0200, FEJF wrote:
> Ragnar Kjørstad, on Montag, 12. September 2001 00:16 wrote:
> > OK, this is the patch.
> 
> thx, for your help, but meanwhile i got one from Holger Grothe.
> sorry, but i haven't had time to post it earlier. i do it now because it has 
> some advantages...
> 
> tools/lib/pv_move.c:
> 
> replace:
>             fprintf ( stderr, "%s -- ERROR reading input "
>                      "physical volume \"%s\" (still %d bytes to read)\n       
>                       cmd, vg->pv[src_pv_index]->pv_name, size);
>             pe_unlock ( vg->vg_name);
>             ret = -LVM_EPV_MOVE_PE_READ_IN;
>             goto pv_move_pe_end;
> with:
>            fprintf ( stderr, "read: %ld, to_read %ld\n", red, to_read);
>            memset(buffer,170,to_read);
>            red=to_read;
> 
> with 170 u can chosse with which chars the bad block should be replaced with. 
> (you can search filez for them later if u want - and have enough time ;)

It's better than my patch, but still not "correct":
* In my case pe_lock() failed, so it had to be "removed" as well. Was
  that not the case for you?
* ret==-1 and ret<to_read should probably be handled differently.
  * For ret>0 && ret<to_read the regular execution path could be followed.
  * When red==-1 there should be a seek on pv[src_pv_index], so the
    possition of the filehandle is set correctly. (now it's undefined)
* maybe red=SECTOR_SIZE; would be better? see next comment.

> tools/pvmove.c:
> 
> replace:
> 	int buffer_size = 64*1024;
> with:   
> 	int buffer_size = 512;
> 
> this is an advantage to your patch, because pvmove then copys only 512 
> byte-blocks and if there's only one block damaged u don't loose 64kb data. 
> this has one disadvantage: it's SLOW... and slower than that ;)

This is not needed if the read loop is allowed to continue.


It would be great if someone took this patch, made the proposed changes,
and integrated it into the standard tools with an "ingore-read-errors"
flag... hint hint.



-- 
Ragnar Kjørstad
Big Storage