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

[Vratislav Podzimek]

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 
---
 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

[Martin Kolman]

On Wed, 2015-01-21 at 14:49 +0100, Vratislav Podzimek wrote:
> 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
>
> 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

Looks good to me, ACK!

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: [e-mail]
> 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
>
> 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
> [e-mail]
> 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.

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.

- mulhern

[Vratislav Podzimek]

On Thu, 2015-01-22 at 09:45 -0500, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" 
> > To: [e-mail]
> > 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
> >
> > 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
> > [e-mail]
> > 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()

[Anne Mulhern]

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" 
> To: "anaconda patch review" 
> 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" 
> > > To: [e-mail]
> > > 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
> > >
> > > 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
> > > [e-mail]
> > > 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
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

[Vratislav Podzimek]

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" 
> > To: "anaconda patch review" 
> > 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" 
> > > > To: [e-mail]
> > > > 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
> > > >
> > > > 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
> > > > [e-mail]
> > > > 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
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches