[sheepdog] [PATCH 4/4] dog: make repairing vdi optional

[Liu Yuan]

On Thu, Aug 22, 2013 at 11:18:07PM +0900, MORITA Kazutaka wrote:
> At Thu, 22 Aug 2013 16:13:49 +0800,
> Liu Yuan wrote:
> > 
> > On Thu, Aug 22, 2013 at 05:06:20PM +0900, MORITA Kazutaka wrote:
> > > At Thu, 22 Aug 2013 15:53:30 +0800,
> > > Liu Yuan wrote:
> > > > 
> > > > On Thu, Aug 22, 2013 at 04:49:18PM +0900, MORITA Kazutaka wrote:
> > > > > At Thu, 22 Aug 2013 15:37:44 +0800,
> > > > > Liu Yuan wrote:
> > > > > > 
> > > > > > On Thu, Aug 22, 2013 at 09:30:05AM +0200, Valerio Pachera wrote:
> > > > > > > Sorry guys if I intrude in this list.
> > > > > > > 
> > > > > > > In my opinion, form user point of view, it would be nice to have a
> > > > > > > "read only" check that could be run with a running vm.
> > > > > > > In case problems are reported, then the user could shutdown the guest
> > > > > > > when he can, and run the check "read write" (auto repair).
> > > > > > > 
> > > > > > > Similar to
> > > > > > > xfs_check -n <device>  (no repair)
> > > > > > > xfs_check <device> (repair)
> > > > > > > 
> > > > > > 
> > > > > > This looks reasonable to me.
> > > > > 
> > > > > Does it means that it's okay for you if auto-repar is disabled by
> > > > > default?  If we have a way to do online check, either is okay to me.
> > > > 
> > > > I think default to be disabled (safest option) looks fine to me. Can we simply
> > > > call it 'repair' instead of 'auto-repair'? I have no clue what 'auto' means when
> > > > I see it.
> > > 
> > > Ah, I may see your point.
> > > With my patches,
> > > 
> > >  - vdi check: shows a confirm message to repair the vdi when an error
> > >               is found.
> > >  - vdi check -A: repair the vdi without asking (I said this as a auto-repair)
> > > 
> > > but you mean 
> > > 
> > >  - vdi check: check only
> > >  - vdi check -r: check and repiar
> > > 
> > > right?
> > 
> > Yes
> > 
> > >
> > > Then how about creating another command 'vdi repair' to fix the vdi?
> > > I noticed that xfs has xfs_check and xfs_repair.
> > 
> > vdi check # check only?
> > vdi repair # check and repair?
> > 
> > But we'll duplicate many advanced commands for check and repair. So I think
> > 
> > vdi check [ -r -o {consistency,inode...} ] as a bundle is better.
> 
> I still think -o {optlist} is not a good idea because
> 
>  - Users cannot know which option is enabled by default and which
>    option they should turn on.

Actually I think users should not know any options at all. Anyone tries to run
vdi check, is just to check the health of the targeted vdi.

>  - It's difficult to maintain bash_completion_dog and dog.8 if we
>    allow the option list.
> 
> How about adding only one option "-o, --online", which filters unsafe
> check/repair when the vdi is in use?
> 
> E.g.
> 
> $ vdi check                   # This tries full check.
> $ vdi check --online          # This filters consistency check since it
>                               # doesn't work if the vdi is in use
> 
> $ vdi check --repair          # This tries full check and repair
> $ vdi check --repair --online # This does only existence check since
>                               # the other tests are not safe against online vdi.
> 

>From the view of ordinary user, I think concept like existence, inode, consistency
won't make too much difference to them. If I were the user, I'd expect

$ vdi check # this will check the health and report the problem if any
$ vdi check -r # if problems reported, users can decide to repair, whatever problem is

That is all. For advanced users,
$ vdi check -o {option list}
or
$ vdi check -opt1 -opt2 -opt2 # for easier bash_completion_dog generation.

Thanks
Yuan