We claim to return a list, but since we never unmount after
finding valid media, we can never return a list of more than
a single device.
---
pyanaconda/image.py | 9 +++++----
pyanaconda/ui/gui/spokes/source.py | 3 ++-
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/pyanaconda/image.py b/pyanaconda/image.py
index 01e01cd..7bfdb52 100644
--- a/pyanaconda/image.py
+++ b/pyanaconda/image.py
@@ -176,10 +176,10 @@ def mountImage(isodir, tree, messageWindow):
else:
break

-# Return a list of Device instances containing valid optical install media
+# Return the first Device instance containing valid optical install media
# for this product.
-def opticalInstallMedia(devicetree, mountpoint="/tmp/mnt"):
- retval = []
+def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE):
+ retval = None

for dev in devicetree.getDevicesByType("cdrom"):
devicetree.updateDeviceFormat(dev)
@@ -192,7 +192,8 @@ def opticalInstallMedia(devicetree, mountpoint="/tmp/mnt"):
dev.format.unmount()
continue

- retval.append(dev)
+ retval = dev
+ break

return retval

diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index b5e88c8..cfe5fec 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -284,7 +284,8 @@ class SourceSpoke(NormalSpoke):
# If we found any optical install media, display a selector for each
# of those.
added = False
- for cdrom in opticalInstallMedia(self.devicetree, mountpoint=MOUNTPOINT):
+ cdrom = opticalInstallMedia(self.devicetree, mountpoint=MOUNTPOINT)
+ if cdrom
selector = AnacondaWidgets.DiskOverview(cdrom.format.label or "", "drive-removable-media", "")
selector.path = cdrom.path
self._autodetectMediaBox.pack_start(selector, False, False, 0)
--
1.7.9.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

> diff --git a/pyanaconda/image.py b/pyanaconda/image.py
> index 01e01cd..7bfdb52 100644
> --- a/pyanaconda/image.py
> +++ b/pyanaconda/image.py
> @@ -176,10 +176,10 @@ def mountImage(isodir, tree, messageWindow):
> else:
> break
>
> -# Return a list of Device instances containing valid optical install media
> +# Return the first Device instance containing valid optical install media
> # for this product.
> -def opticalInstallMedia(devicetree, mountpoint="/tmp/mnt"):
> - retval = []
> +def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE):
> + retval = None
>
> for dev in devicetree.getDevicesByType("cdrom"):
> devicetree.updateDeviceFormat(dev)
> @@ -192,7 +192,8 @@ def opticalInstallMedia(devicetree, mountpoint="/tmp/mnt"):
> dev.format.unmount()
> continue
>
> - retval.append(dev)
> + retval = dev
> + break
>
> return retval
>

I've got a patch locally that also unmounts the device after finding it.
I'm not yet sure whether we need to do that or not.

Also, I'm debating getting rid of the verifyMedia check in
opticalInstallMedia. It's leading to some weirdness with the
installation source spoke.

> diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
> index b5e88c8..cfe5fec 100644
> --- a/pyanaconda/ui/gui/spokes/source.py
> +++ b/pyanaconda/ui/gui/spokes/source.py
> @@ -284,7 +284,8 @@ class SourceSpoke(NormalSpoke):
> # If we found any optical install media, display a selector for each
> # of those.
> added = False
> - for cdrom in opticalInstallMedia(self.devicetree, mountpoint=MOUNTPOINT):
> + cdrom = opticalInstallMedia(self.devicetree, mountpoint=MOUNTPOINT)
> + if cdrom

Missing a colon here.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On Tue, 2012-03-06 at 11:07 -0500, Chris Lumens wrote:
> > diff --git a/pyanaconda/image.py b/pyanaconda/image.py
> > index 01e01cd..7bfdb52 100644
> > --- a/pyanaconda/image.py
> > +++ b/pyanaconda/image.py
> > @@ -176,10 +176,10 @@ def mountImage(isodir, tree, messageWindow):
> > else:
> > break
> >
> > -# Return a list of Device instances containing valid optical install media
> > +# Return the first Device instance containing valid optical install media
> > # for this product.
> > -def opticalInstallMedia(devicetree, mountpoint="/tmp/mnt"):
> > - retval = []
> > +def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE):
> > + retval = None
> >
> > for dev in devicetree.getDevicesByType("cdrom"):
> > devicetree.updateDeviceFormat(dev)
> > @@ -192,7 +192,8 @@ def opticalInstallMedia(devicetree, mountpoint="/tmp/mnt"):
> > dev.format.unmount()
> > continue
> >
> > - retval.append(dev)
> > + retval = dev
> > + break
> >
> > return retval
> >
>
> I've got a patch locally that also unmounts the device after finding it.
> I'm not yet sure whether we need to do that or not.

In patch 2 I've removed that calls that mount the device after finding
it, just to save some thrashing, but if we're also using
opticalInstallMedia in ways that do not warrant leaving the device
mounted I can certainly add the umount in opticalInstallMedia and add
the mount code back into patch 2.

>
> Also, I'm debating getting rid of the verifyMedia check in
> opticalInstallMedia. It's leading to some weirdness with the
> installation source spoke.
>
> > diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
> > index b5e88c8..cfe5fec 100644
> > --- a/pyanaconda/ui/gui/spokes/source.py
> > +++ b/pyanaconda/ui/gui/spokes/source.py
> > @@ -284,7 +284,8 @@ class SourceSpoke(NormalSpoke):
> > # If we found any optical install media, display a selector for each
> > # of those.
> > added = False
> > - for cdrom in opticalInstallMedia(self.devicetree, mountpoint=MOUNTPOINT):
> > + cdrom = opticalInstallMedia(self.devicetree, mountpoint=MOUNTPOINT)
> > + if cdrom
>
> Missing a colon here.

And that's why I value the review process.

>
> - Chris
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@redhat.com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

> In patch 2 I've removed that calls that mount the device after finding
> it, just to save some thrashing, but if we're also using
> opticalInstallMedia in ways that do not warrant leaving the device
> mounted I can certainly add the umount in opticalInstallMedia and add
> the mount code back into patch 2.

This is fine for now. It's easy to unmount later if we discover it's
causing problems.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list