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

Vratislav Podzimek vpodzime at redhat.com
Mon Jan 26 08:37:28 UTC 2015


On Fri, 2015-01-23 at 14:24 -0500, Anne Mulhern wrote:
> 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.
So far there's no code like this in libblockdev. The library tries to
load all plugins in the init() call and the whole concept (right now)
relies on package management system to pull in all the dependencies (the
liblockdev-lvm package providing the lvm plugin 'Requires: lvm2'). Let's
discuss possible improvements to that in a separate mail discussion so
that we don't spam anaconda-patches with it.

> 
> 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.
I'll keep that in mind, but to tell the truth, I'd like to port blivet
to libblockdev ASAP so this might be a follow-up, separate, step.

> 
> Regarding this patch itself, though, ack.
Thanks!

> 
> - 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
> > 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list