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

[Heinz J . Mauelshagen]

Ragnar,

typically a dying source device will cause read to fail (~line 520
in pv_move_pe.c). This could be easily addressed by an 'ignore read errors'
option and a fallback to BLOCK_SIZEed I/O in order to avoid as much data losses
as possible.

But I wonder, why in your case the locking of the PE failed. Are you able to 
reporduce your case and provide the error code?

Regards,
Heinz    -- The LVM Guy --


On Mon, Sep 10, 2001 at 10:39:52AM +0200, Ragnar Kjørstad wrote:
> 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
> 
> _______________________________________________
> linux-lvm mailing list
> linux-lvm at sistina.com
> http://lists.sistina.com/mailman/listinfo/linux-lvm
> read the LVM HOW-TO at http://www.sistina.com/lvm/Pages/howto.html

*** Software bugs are stupid.
    Nevertheless it needs not so stupid people to solve them ***

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Heinz Mauelshagen                                 Sistina Software Inc.
Senior Consultant/Developer                       Am Sonnenhang 11
                                                  56242 Marienrachdorf
                                                  Germany
Mauelshagen at Sistina.com                           +49 2626 141200
                                                       FAX 924446
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-