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.campb...@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
