[rhel7/master] Multiple loops shouldn't be fatal (#980510)

[Brian C. Lane]

Backing files can have multiple loops associated with them without
problems. Raising an error here causes a crash when using an iso for
both stage2 and for the repo (eg. boot with
inst.repo=hd:/dev/sdb1:/path-to-dvd.iso).

Instead just use the first loop device in the list as the name.

Resolves: rhbz#980510
---
 blivet/devicelibs/loop.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/blivet/devicelibs/loop.py b/blivet/devicelibs/loop.py
index 0b374a2..1600ed8 100644
--- a/blivet/devicelibs/loop.py
+++ b/blivet/devicelibs/loop.py
@@ -55,8 +55,9 @@ def get_loop_name(path):
     args = ["-j", path]
     buf = losetup(args, capture=True)
     if len(buf.splitlines()) > 1:
-        # there should never be more than one loop device listed
-        raise LoopError("multiple loops associated with %s" % path)
+        # If there are multiple loop devices use the first one
+        buf = buf.splitlines()[0]
+        log.warning("multiple loops associated with %s. Using %s" % (path, buf))

name = os.path.basename(buf.split(":")[0])
     return name

[Vratislav Podzimek]

On Tue, 2015-01-06 at 10:25 -0800, Brian C. Lane wrote:
> Backing files can have multiple loops associated with them without
> problems. Raising an error here causes a crash when using an iso for
> both stage2 and for the repo (eg. boot with
> inst.repo=hd:/dev/sdb1:/path-to-dvd.iso).
>
> Instead just use the first loop device in the list as the name.
> Resolves: rhbz#980510
>
> blivet/devicelibs/loop.py | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/blivet/devicelibs/loop.py b/blivet/devicelibs/loop.py
> index 0b374a2..1600ed8 100644
> --- a/blivet/devicelibs/loop.py
> +++ b/blivet/devicelibs/loop.py
> @@ -55,8 +55,9 @@ def get_loop_name(path):
>      args = ["-j", path]
>      buf = losetup(args, capture=True)
>      if len(buf.splitlines()) > 1:
>    # there should never be more than one loop device listed
>    raise LoopError("multiple loops associated with %s" % path)
>    # If there are multiple loop devices use the first one
>    buf = buf.splitlines()[0]
>    log.warning("multiple loops associated with %s. Using %s" % (path, buf))

It would be nicer to do splitlines() only once and then check+use the
result. But the change genrally looks good to me.

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: "anaconda patch review" 
> Sent: Tuesday, January 6, 2015 1:38:23 PM
> Subject: Re: [rhel7/master] Multiple loops shouldn't be fatal (#980510)
>
> On Tue, 2015-01-06 at 10:25 -0800, Brian C. Lane wrote:
> > Backing files can have multiple loops associated with them without
> > problems. Raising an error here causes a crash when using an iso for
> > both stage2 and for the repo (eg. boot with
> > inst.repo=hd:/dev/sdb1:/path-to-dvd.iso).
> >
> > Instead just use the first loop device in the list as the name.
> > Resolves: rhbz#980510
> >
> > blivet/devicelibs/loop.py | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/blivet/devicelibs/loop.py b/blivet/devicelibs/loop.py
> > index 0b374a2..1600ed8 100644
> > --- a/blivet/devicelibs/loop.py
> > +++ b/blivet/devicelibs/loop.py
> > @@ -55,8 +55,9 @@ def get_loop_name(path):
> >      args = ["-j", path]
> >      buf = losetup(args, capture=True)
> >      if len(buf.splitlines()) > 1:
> >    # there should never be more than one loop device listed
> >    raise LoopError("multiple loops associated with %s" % path)
> >    # If there are multiple loop devices use the first one
> >    buf = buf.splitlines()[0]
> >    log.warning("multiple loops associated with %s. Using %s" % (path,
> >
> > buf))
>
> It would be nicer to do splitlines() only once and then check+use the
> result. But the change genrally looks good to me.

I was a bit confused for a while because buf ends up meaning so many things.

Would be more readable with different names for different things, like:

buf = losetup(args, capture=True)
      entries = buf.splitlines()
      first_entry = entries[0]
      if len(entries) > 1:
        log.warning("multiple loops associated with %s. Using %s" % (path, first_entry))
      name = os.path.basename(first_entry.split(":")[0])
      ...
> --
> Vratislav Podzimek
>
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

- mulhern

[Brian C. Lane]

On Wed, Jan 07, 2015 at 08:44:55AM -0500, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" 
> > To: "anaconda patch review" 
> > Sent: Tuesday, January 6, 2015 1:38:23 PM
> > Subject: Re: [rhel7/master] Multiple loops shouldn't be fatal (#980510)
> >
> > On Tue, 2015-01-06 at 10:25 -0800, Brian C. Lane wrote:
> > > Backing files can have multiple loops associated with them without
> > > problems. Raising an error here causes a crash when using an iso for
> > > both stage2 and for the repo (eg. boot with
> > > inst.repo=hd:/dev/sdb1:/path-to-dvd.iso).
> > >
> > > Instead just use the first loop device in the list as the name.
> > > Resolves: rhbz#980510
> > >
> > > blivet/devicelibs/loop.py | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/blivet/devicelibs/loop.py b/blivet/devicelibs/loop.py
> > > index 0b374a2..1600ed8 100644
> > > --- a/blivet/devicelibs/loop.py
> > > +++ b/blivet/devicelibs/loop.py
> > > @@ -55,8 +55,9 @@ def get_loop_name(path):
> > >      args = ["-j", path]
> > >      buf = losetup(args, capture=True)
> > >      if len(buf.splitlines()) > 1:
> > >    # there should never be more than one loop device listed
> > >    raise LoopError("multiple loops associated with %s" % path)
> > >    # If there are multiple loop devices use the first one
> > >    buf = buf.splitlines()[0]
> > >    log.warning("multiple loops associated with %s. Using %s" % (path,
> > >
> > > buf))
> >
> > It would be nicer to do splitlines() only once and then check+use the
> > result. But the change genrally looks good to me.
>
> I was a bit confused for a while because buf ends up meaning so many things.
>
> Would be more readable with different names for different things, like:
>   buf = losetup(args, capture=True)
>   entries = buf.splitlines()
>   first_entry = entries[0]
>   if len(entries) > 1:
>     log.warning("multiple loops associated with %s. Using %s" % (path, first_entry))
>   name = os.path.basename(first_entry.split(":")[0])
>   ...

Yeah, it doesn't hurt to just go ahead and allways do the splitlines

[v2] Multiple loops shouldn't be fatal (#980510)

[Brian C. Lane]

Backing files can have multiple loops associated with them without
problems. Raising an error here causes a crash when using an iso for
both stage2 and for the repo (eg. boot with
inst.repo=hd:/dev/sdb1:/path-to-dvd.iso).

Instead just use the first loop device in the list as the name.

Resolves: rhbz#980510
---
 blivet/devicelibs/loop.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/blivet/devicelibs/loop.py b/blivet/devicelibs/loop.py
index 0b374a2..661a32e 100644
--- a/blivet/devicelibs/loop.py
+++ b/blivet/devicelibs/loop.py
@@ -54,11 +54,17 @@ def get_backing_file(name):
 def get_loop_name(path):
     args = ["-j", path]
     buf = losetup(args, capture=True)
-    if len(buf.splitlines()) > 1:
-        # there should never be more than one loop device listed
-        raise LoopError("multiple loops associated with %s" % path)

-    name = os.path.basename(buf.split(":")[0])
+    entries = buf.splitlines()
+    if not entries:
+        raise LoopError("No loop associated with %s" % (path))
+
+    first_entry = entries[0]
+    if len(entries) > 1:
+        # If there are multiple loop devices use the first one
+        log.warning("multiple loops associated with %s. Using %s", path, first_entry)
+
+    name = os.path.basename(first_entry.split(":")[0])
     return name

def loop_setup(path):

[v2] Multiple loops shouldn't be fatal (#980510)

[Anne Mulhern]

----- Original Message -----
> From: "Brian C. Lane" 
> To: [e-mail]
> Sent: Wednesday, January 7, 2015 12:45:34 PM
> Subject: [v2] Multiple loops shouldn't be fatal (#980510)
>
> Backing files can have multiple loops associated with them without
> problems. Raising an error here causes a crash when using an iso for
> both stage2 and for the repo (eg. boot with
> inst.repo=hd:/dev/sdb1:/path-to-dvd.iso).
>
> Instead just use the first loop device in the list as the name.
> Resolves: rhbz#980510
>
> blivet/devicelibs/loop.py | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/blivet/devicelibs/loop.py b/blivet/devicelibs/loop.py
> index 0b374a2..661a32e 100644
> --- a/blivet/devicelibs/loop.py
> +++ b/blivet/devicelibs/loop.py
> @@ -54,11 +54,17 @@ def get_backing_file(name):
>  def get_loop_name(path):
>      args = ["-j", path]
>      buf = losetup(args, capture=True)
>
> if len(buf.splitlines()) > 1:
>    # there should never be more than one loop device listed
>    raise LoopError("multiple loops associated with %s" % path)
>
> name = os.path.basename(buf.split(":")[0])
> entries = buf.splitlines()
> if not entries:
>    raise LoopError("No loop associated with %s" % (path))
> first_entry = entries[0]
> if len(entries) > 1:
>    # If there are multiple loop devices use the first one
>    log.warning("multiple loops associated with %s. Using %s", path,
>
> first_entry)
> name = os.path.basename(first_entry.split(":")[0])
> return name
> def loop_setup(path):
>
> 1.9.3
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

Ack.

- mulhern