Re: [Xen-devel] [OSSTEST PATCH 09/22] LVM: Break out lv_create

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-11-11 at 16:30 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 09/22] LVM: Break out
> lv_create"):
> > On Tue, 2015-11-10 at 19:53 +0000, Ian Jackson wrote:
> > > +sub lv_create ($$$$) {
> > > +ÂÂÂÂmy ($ho, $vg, $lv, $mb) = @_;
> > > +ÂÂÂÂmy $lvdev = "/dev/$vg/$lv";
> > 
> > In the original code it was using $gho->{Lvdev}, is this semantic
> > change
> > deliberate or a rebase-o? If the former then I think it warrants a
> > comment
> > in the commit message.
> 
> It was deliberate.ÂÂI have added this to the commit message:
> 
> Âlv_create doesn't (want to) take a $gho, but the $vg and $lv names
> Âdirectly (so that callers can use it when they don't have a suitable
> Â$gho whose $gho->{Lvdev} they want to use).
> 
> ÂIn the one existing call site we pass $gho->{Vg} and $gho->{Lv} so
> Âthat the effect is the same.
> 
> ÂThere is a minor functional change: $gho->{Lvdev} has been put through
> Âlv_dev_mapper.ÂÂBut we don't care about that in lv_create (since the
> ÂLVM operations, and dd, are perfectly happy to use the `real',
> Ânon-/dev/mapper, names).ÂÂSo we can just use /dev/$vg/$lv.

That looks good. The "return $lvdev" from this new function would slightly
encourage using this path in other situations which are not lv* commands
and which might want the mapper version. I think this is a remote enough
possibility to not worry about, although maybe you want to add a comment.

In any case:

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel