[blivet][master] Get rid of the has_lvm function

Anne Mulhern amulhern at redhat.com
Fri Jan 23 19:24:22 UTC 2015


The general plan is obviously something we need to coordinate on,
as blivet will then have to use libblockdev interface to make it's
own decisions and communicate w/ client code about them. This
general problem is something I am currently working on, as you know.

Can you give me a pointer to existing implementation or something
regarding this function that you mention?
I can not locate it in current version of libblockdev
repo.

Also, please keep in mind that the tests themselves should be
aware of the availability of supporting applications, and skipped if the
applications being tested are not available. So, has_lvm() or its equivalent
should really be being used in a skipUnless() call, rather than being tested,
per se.

Of course, it isn't actually the case that this principle is followed
for the devicelibs tests. But something similar to what happens for the formats
tests should occur. If you are able to make this happen while porting blivet to libblockdev
that would be a bonus.

Regarding this patch itself, though, ack.

- mulhern


----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Friday, January 23, 2015 3:49:45 AM
> Subject: Re: [blivet][master] Get rid of the has_lvm function
> 
> On Thu, 2015-01-22 at 09:45 -0500, Anne Mulhern wrote:
> > 
> > ----- Original Message -----
> > > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > > To: anaconda-patches at lists.fedorahosted.org
> > > Sent: Wednesday, January 21, 2015 8:49:28 AM
> > > Subject: [blivet][master] Get rid of the has_lvm function
> > > 
> > > It's only used in the tests and since blivet requires lvm tools to be
> > > installed
> > > I doubt any (user) code uses it.
> > > 
> > > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > > ---
> > >  blivet/devicelibs/lvm.py          | 8 --------
> > >  tests/devicelibs_test/lvm_test.py | 9 ---------
> > >  2 files changed, 17 deletions(-)
> > > 
> > > diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
> > > index 7e2dbef..290e64d 100644
> > > --- a/blivet/devicelibs/lvm.py
> > > +++ b/blivet/devicelibs/lvm.py
> > > @@ -50,14 +50,6 @@ ThPoolProfile = namedtuple("ThPoolProfile", ["name",
> > > "desc"])
> > >  KNOWN_THPOOL_PROFILES = (ThPoolProfile("thin-generic", N_("Generic")),
> > >                           ThPoolProfile("thin-performance",
> > >                           N_("Performance")))
> > >  
> > > -def has_lvm():
> > > -    if util.find_program_in_path("lvm"):
> > > -        for line in open("/proc/devices").readlines():
> > > -            if "device-mapper" in line.split():
> > > -                return True
> > > -
> > > -    return False
> > > -
> > >  # Start config_args handling code
> > >  #
> > >  # Theoretically we can handle all that can be handled with the LVM
> > >  --config
> > > diff --git a/tests/devicelibs_test/lvm_test.py
> > > b/tests/devicelibs_test/lvm_test.py
> > > index 8f776b2..d88c451 100755
> > > --- a/tests/devicelibs_test/lvm_test.py
> > > +++ b/tests/devicelibs_test/lvm_test.py
> > > @@ -254,15 +254,6 @@ class LVMAsRootTestCase(LVMAsRootTestCaseBase):
> > >          self.assertEqual(lvm.lvs(vg_name="wrong-vg-name"), {})
> > >  
> > >          ##
> > > -        ## has_lvm
> > > -        ##
> > > -        # pass
> > > -        self.assertEqual(lvm.has_lvm(), True)
> > > -
> > > -        # fail
> > > -        # TODO
> > > -
> > > -        ##
> > >          ## lvremove
> > >          ##
> > >          # pass
> > > --
> > > 2.1.0
> > > 
> > > _______________________________________________
> > > anaconda-patches mailing list
> > > anaconda-patches at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > > 
> > 
> > The application availability branch I'm working on now has a purpose
> > of dynamically detecting availability of tools. Right now the work I
> > have done is all in formats, but we expect to wrap around devicelibs
> > stuff as well. At that point we should be able to drop the Requires
> > lvm tools to a Suggests.
> I think you don't take libblockdev in mind here. Once we port blivet to
> use libblockdev, which should be by the end of January, there will be no
> invocations of lvm tools in blivet and no Requires/Suggests.
> 
> > 
> > It might be that has_lvm() is wrong, but it's possibly still the best
> > reference for how to detect availability of lvm tools, so I'ld feel
> > better about leaving it in, if possible.
> It will be available in git history forever, so I don't see a reason for
> keeping it in the sources. And with libblockdev the function will be
> reduced to:
> 
> "lvm" in BlockDev.get_available_plugin_names()
> 
> --
> Vratislav Podzimek
> 
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list