[anaconda][PATCH 2/2] Use a temporary directory for verifying ISO media

Vratislav Podzimek vpodzime at redhat.com
Tue Sep 24 14:10:55 UTC 2013


On Tue, 2013-09-24 at 09:55 -0400, David Shea wrote:
> On 09/24/2013 09:48 AM, Vratislav Podzimek wrote:
> > On Tue, 2013-09-24 at 09:37 -0400, David Shea wrote:
> >> None of the callers of opticalInstallMedia use the mountpoint parameter,
> >> so removed it. Changed to using a temporary directory instead of
> >> /run/install/source to mount and verify potential optical media, since
> >> /run/install/source may already be in use by media specified in
> >> inst.repo.
> >> ---
> >>   pyanaconda/image.py | 23 ++++++++++++++---------
> >>   1 file changed, 14 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/pyanaconda/image.py b/pyanaconda/image.py
> >> index 9049be9..82f3e4f 100644
> >> --- a/pyanaconda/image.py
> >> +++ b/pyanaconda/image.py
> >> @@ -18,8 +18,8 @@
> >>   #
> >>   
> >>   import isys
> >> -import os, os.path, stat
> >> -from constants import INSTALL_TREE, ISO_DIR
> >> +import os, os.path, stat, tempfile
> >> +from constants import ISO_DIR
> >>   
> >>   from errors import errorHandler, ERROR_RAISE, InvalidImageSizeError, MediaMountError, MediaUnmountError, MissingImageError
> >>   
> >> @@ -185,7 +185,7 @@ def mountImage(isodir, tree):
> >>   
> >>   # Return the first Device instance containing valid optical install media
> >>   # for this product.
> >> -def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE):
> >> +def opticalInstallMedia(devicetree):
> >>       retval = None
> >>   
> >>       # Search for devices identified as cdrom along with any other
> >> @@ -201,16 +201,21 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE):
> >>               # no mountable media
> >>               continue
> >>   
> >> +        mountpoint = tempfile.mkdtemp()
> >>           try:
> >> -            dev.format.mount(mountpoint=mountpoint)
> >> -        except FSError:
> >> -            continue
> >> +            try:
> >> +                dev.format.mount(mountpoint=mountpoint)
> >> +            except FSError:
> >> +                continue
> >> +
> >> +            if not verifyMedia(mountpoint):
> >> +                dev.format.unmount()
> >> +                continue
> >>   
> >> -        if not verifyMedia(mountpoint):
> >>               dev.format.unmount()
> >> -            continue
> >> +        finally:
> >> +            os.rmdir(mountpoint)
> >>   
> >> -        dev.format.unmount()
> > If verifyMedia returns True, the format won't get umounted and the
> > finally block will try to remove its contents. Or am I missing
> > something?
> >
> It looks funny because I indented a block and one of the original 
> unmounts ended up in the context of the diff. I just removed the 
> parameter from the function definition, added the mountpoint = 
> tempfile.mkdtemp(), and wrapped everything in this block:
> 
>          try:
>              dev.format.mount(mountpoint=mountpoint)
>          except FSError:
>              continue
> 
>          if not verifyMedia(mountpoint):
>              dev.format.unmount()
>              continue
> 
>          dev.format.unmount()
> 
> 
> In a try-finally. There's still two unmount() lines.
Oh, I've overlooked the one context line between two removed lines.
These both look good to me, then.

-- 
Vratislav Podzimek

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



More information about the anaconda-patches mailing list