[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [OSSTEST PATCH 09/22] LVM: Break out lv_create
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |