[lvm-devel] [PATCH] (1/2) new library interface for vg_read

[Dave Wysochanski]

On Fri, 2008-12-12 at 10:20 +0100, Petr Rockai wrote:
> Dave Wysochanski <dwysocha at redhat.com> writes:
> > On Tue, 2008-12-09 at 17:44 +0100, Petr Rockai wrote:
> >>  /* FIXME: remove redundant vg_name */
> >>  int vg_remove_single(struct cmd_context *cmd, const char *vg_name,
> >> -                    struct volume_group *vg, int consistent,
> >> +                    struct volume_group *vg,
> >>                      force_t force __attribute((unused)))
> >>  {
> >>         struct physical_volume *pv;
> >> @@ -340,7 +340,7 @@ int vg_remove_single(struct cmd_context 
> >>         unsigned lv_count;
> >>         int ret = 1;
> >>  
> >> -       if (!vg || !consistent || vg_missing_pv_count(vg)) {
> >> +       if (vg_read_error(vg) || vg_missing_pv_count(vg)) {
> >>                 log_error("Volume group \"%s\" not found, is
> >> inconsistent "
> >>                           "or has PVs missing.", vg_name);
> >>                 log_error("Consider vgreduce --removemissing if
> >> metadata "
> >
> > Did you mean to put this in the second patch?
> Actually, hard to tell. It, after all, is part of the library, for a reason or
> other. It belongs equally well to the first as to the second patch. What do you
> propose?
> 

Well, doesn't this cause a bug in this path though for the !consistent
case?  First patch drops the consistent variable in this vgremove path,
but things like _process_one_vg() do not use the new APIs (which sets
the proper error codes) until the second patch.  You mention in your
patch intro this first patch should apply and make a completely workable
LVM so I'm assuming you mean no bugs.

I'm still digesting the main changes in vg_lock_and_read().