Refactor OpticalHelper functionality.

Add helper functions to properly set up the cdrom combo widgets. Separate
the dbus polling from UI editting: eventually we will get some of this info
from libvirt hostdev APIs, and this will help.

src/virtManager/choosecd.py

@@ -23,7 +23,7 @@ import logging
 
 import virtinst
 
-from virtManager.opticalhelper import vmmOpticalDriveHelper
+import virtManager.opticalhelper
 from virtManager.storagebrowse import vmmStorageBrowser
 from virtManager.error import vmmErrorDialog
 
@@ -58,17 +58,7 @@ class vmmChooseCD(gobject.GObject):
 
         self.window.get_widget("iso-image").set_active(True)
 
-        # set up the list for the cd-path widget
+        self.initialize_opt_media()
-        cd_list = self.window.get_widget("cd-path")
-        # Fields are raw device path, volume label, flag indicating
-        # whether volume is present or not, and HAL path
-        cd_model = gtk.ListStore(str, str, bool, str)
-        cd_list.set_model(cd_model)
-        text = gtk.CellRendererText()
-        cd_list.pack_start(text, True)
-        cd_list.add_attribute(text, 'text', 1)
-        cd_list.add_attribute(text, 'sensitive', 2)
-
         self.reset_state()
 
     def close(self,ignore1=None,ignore2=None):
@@ -92,35 +82,37 @@ class vmmChooseCD(gobject.GObject):
         else:
             self.window.get_widget("physical-media").set_sensitive(True)
             self.window.get_widget("iso-file-chooser").set_sensitive(True)
-            self.populate_opt_media()
-            self.window.get_widget("cd-path").set_active(0)
 
     def ok(self,ignore1=None, ignore2=None):
+        path = None
+
         if self.window.get_widget("iso-image").get_active():
             path = self.window.get_widget("iso-path").get_text()
         else:
             cd = self.window.get_widget("cd-path")
+            idx = cd.get_active()
             model = cd.get_model()
-            path = model.get_value(cd.get_active_iter(), 0)
+            if idx != -1:
+                path = model[idx][virtManager.opticalhelper.OPTICAL_PATH]
 
         if path == "" or path == None:
-            return self.err.val_err(_("Invalid Media Path"), \
+            return self.err.val_err(_("Invalid Media Path"),
                                     _("A media path must be specified."))
 
         try:
+            dev=virtinst.VirtualDisk.DEVICE_CDROM
             disk = virtinst.VirtualDisk(path=path,
-                                        device=virtinst.VirtualDisk.DEVICE_CDROM, 
+                                        device=dev,
                                         readOnly=True,
                                         conn=self.conn.vmm)
         except Exception, e:
             return self.err.val_err(_("Invalid Media Path"), str(e))
+
         self.emit("cdrom-chosen", disk.type, disk.path, self.dev_id_info)
         self.cancel()
 
     def media_toggled(self, ignore1=None, ignore2=None):
         if self.window.get_widget("physical-media").get_active():
-            self.populate_opt_media()
-            self.window.get_widget("cd-path").set_active(0)
             self.window.get_widget("cd-path").set_sensitive(True)
             self.window.get_widget("iso-path").set_sensitive(False)
             self.window.get_widget("iso-file-chooser").set_sensitive(False)
@@ -135,10 +127,10 @@ class vmmChooseCD(gobject.GObject):
     def browse_fv_iso_location(self, ignore1=None, ignore2=None):
         self._browse_file(_("Locate ISO Image"))
 
-    def populate_opt_media(self):
+    def initialize_opt_media(self):
         try:
-            optical_helper = vmmOpticalDriveHelper(self.window.get_widget("cd-path"))
+            widget = self.window.get_widget("cd-path")
-            optical_helper.populate_opt_media()
+            virtManager.opticalhelper.init_optical_combo(widget)
             self.window.get_widget("physical-media").set_sensitive(True)
         except Exception, e:
             logging.error("Unable to create optical-helper widget: '%s'", e)

src/virtManager/create.py

@@ -31,11 +31,11 @@ import logging
 import virtinst
 from virtinst import VirtualNetworkInterface
 
+import virtManager.opticalhelper
 from virtManager import util
 from virtManager.error import vmmErrorDialog
 from virtManager.asyncjob import vmmAsyncJob
 from virtManager.createmeter import vmmCreateMeter
-from virtManager.opticalhelper import vmmOpticalDriveHelper
 from virtManager.storagebrowse import vmmStorageBrowser
 
 OS_GENERIC = "generic"
@@ -220,17 +220,10 @@ class vmmCreate(gobject.GObject):
         # Physical CD-ROM model
         cd_list = self.window.get_widget("install-local-cdrom-combo")
         cd_radio = self.window.get_widget("install-local-cdrom")
-        # Fields are raw device path, volume label, flag indicating
+
-        # whether volume is present or not, and HAL path
-        cd_model = gtk.ListStore(str, str, bool, str)
-        cd_list.set_model(cd_model)
-        text = gtk.CellRendererText()
-        cd_list.pack_start(text, True)
-        cd_list.add_attribute(text, 'text', 1)
-        cd_list.add_attribute(text, 'sensitive', 2)
         # FIXME: We should disable all this if on a remote connection
         try:
-            vmmOpticalDriveHelper(cd_list)
+            virtManager.opticalhelper.init_optical_combo(cd_list)
         except Exception, e:
             logging.error("Unable to create optical-helper widget: '%s'", e)
             cd_radio.set_sensitive(False)

src/virtManager/opticalhelper.py

@@ -20,38 +20,140 @@
 import gobject
 import dbus
 import logging
+import gtk
+
+OPTICAL_PATH = 0
+OPTICAL_LABEL = 1
+OPTICAL_IS_MEDIA_PRESENT = 2
+OPTICAL_HAL_PATH = 3
+
+def init_optical_combo(widget, empty_sensitive=False):
+    # These fields should match up with vmmOpticalHelper.device_info
+    model = gtk.ListStore(str, str, bool, str)
+    widget.set_model(model)
+    model.clear()
+
+    text = gtk.CellRendererText()
+    widget.pack_start(text, True)
+    widget.add_attribute(text, 'text', 1)
+    if not empty_sensitive:
+        widget.add_attribute(text, 'sensitive', 2)
+
+    helper = vmmOpticalDriveHelper()
+    helper.connect("optical-added", optical_added, widget)
+    helper.connect("optical-removed", optical_removed, widget)
+    for row in helper.get_device_info():
+        model.append(row)
+
+    widget.set_active(-1)
+    set_default_selection(widget)
+
+def optical_removed(ignore_helper, halpath, widget):
+    model = widget.get_model()
+    active = widget.get_active()
+    idx = 0
+    # Search for the row containing matching HAL volume path
+    # and update (clear) it, de-activating it if its currently
+    # selected
+    for row in model:
+        if row[OPTICAL_HAL_PATH] == halpath:
+            row[OPTICAL_LABEL] = display_label(row[OPTICAL_PATH], None)
+            row[OPTICAL_IS_MEDIA_PRESENT] = False
+            row[OPTICAL_HAL_PATH] = None
+            if idx == active:
+                widget.set_active(-1)
+        idx = idx + 1
+
+    set_default_selection(widget)
+
+def optical_added(ignore_helper, newrow, widget):
+    model = widget.get_model()
+    active = widget.get_active()
+    idx = 0
+
+    # Search for the row with matching device node and
+    # fill in info about inserted media. If model has no current
+    # selection, select the new media.
+    for row in model:
+        if row[OPTICAL_PATH] == newrow[OPTICAL_PATH]:
+            for i in range(0, len(row)):
+                row[i] = newrow[i]
+            if active == -1:
+                widget.set_active(idx)
+            idx = idx + 1
+
+def set_default_selection(widget):
+    # Set the first active cdrom device as selected, otherwise none
+    model = widget.get_model()
+    idx = 0
+    active = widget.get_active()
+
+    if active != -1:
+        # already a selection, don't change it
+        return
+
+    for row in model:
+        if row[OPTICAL_IS_MEDIA_PRESENT] == True:
+            widget.set_active(idx)
+            return
+        idx += 1
+
+    widget.set_active(-1)
+
+def display_label(devnode, media_label):
+    if not media_label:
+        media_label = _("No media present")
+    return "%s (%s)" % (media_label, devnode)
 
 class vmmOpticalDriveHelper(gobject.GObject):
-    __gsignals__ = {}
+    __gsignals__ = {
+        "optical-added"  : (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+                            [object]),
+        "optical-removed": (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+                            [str]),
+    }
 
-    def __init__(self, widget):
+
+    def __init__(self):
         self.__gobject_init__()
-        self.widget = widget
+
-        self.model = self.widget.get_model()
+        self.bus = None
+        self.hal_iface = None
+
+        # List of lists, containing all relevant device info. Sublist is:
+        # [ HAL device name,
+        #   Pretty label to show,
+        #   Is media present?,
+        #   Filesystem path, e.g. /dev/sr0 ]
+        self.device_info = []
+
+        self._dbus_connect()
+        self.populate_opt_media()
+
+    def get_device_info(self):
+        return self.device_info
+
+    def _dbus_connect(self):
         try:
-            # Get a connection to the SYSTEM bus
             self.bus = dbus.SystemBus()
-            # Get a handle to the HAL service
             hal_object = self.bus.get_object('org.freedesktop.Hal',
                                              '/org/freedesktop/Hal/Manager')
             self.hal_iface = dbus.Interface(hal_object,
                                             'org.freedesktop.Hal.Manager')
-            self.populate_opt_media()
         except Exception, e:
             logging.error("Unable to connect to HAL to list cdrom "
                           "volumes: '%s'", e)
-            self.bus = None
-            self.hal_iface = None
             raise
 
-    def populate_opt_media(self):
-        # get a list of optical devices with data discs in, for FV installs
-        volinfo = {}
-        self.model.clear()
         # Track device add/removes so we can detect newly inserted CD media
         self.hal_iface.connect_to_signal("DeviceAdded", self._device_added)
         self.hal_iface.connect_to_signal("DeviceRemoved", self._device_removed)
 
+
+    def populate_opt_media(self):
+        volinfo = {}
+        self.device_info = []
+
         # Find info about all current present media
         for path in self.hal_iface.FindDeviceByCapability("volume"):
             label, devnode = self._fetch_device_info(path)
@@ -75,70 +177,33 @@ class vmmOpticalDriveHelper(gobject.GObject):
                 label, path = None, None
                 present = False
 
-            self.model.append([devnode, self._display_label(devnode, label),
+            row = []
-                               present, path])
+            row.insert(OPTICAL_PATH, str(devnode))
+            row.insert(OPTICAL_LABEL, display_label(devnode, label))
+            row.insert(OPTICAL_IS_MEDIA_PRESENT, present)
+            row.insert(OPTICAL_HAL_PATH, str(path))
 
-        self.set_default_selection()
+            self.device_info.append(row)
-
-    def set_default_selection(self):
-        # Set the first active cdrom device as selected, otherwise none
-        idx = 0
-        active = self.widget.get_active()
-
-        if active != -1:
-            # already a selection, don't change it
-            return
-
-        for row in self.model:
-            if row[2] == True:
-                self.widget.set_active(idx)
-                return
-            idx += 1
-
-        self.widget.set_active(-1)
 
     def _device_added(self, path):
-        active = self.widget.get_active()
-        idx = 0
         label, devnode = self._fetch_device_info(path)
 
         if not devnode:
             # Not an applicable device
             return
 
-        # Search for the row with matching device node and
+        row = []
-        # fill in info about inserted media. If model has no current
+        row.insert(OPTICAL_PATH, str(devnode))
-        # selection, select the new media.
+        row.insert(OPTICAL_LABEL, display_label(devnode, label))
-        for row in self.model:
+        row.insert(OPTICAL_IS_MEDIA_PRESENT, True)
-            if row[0] == devnode:
+        row.insert(OPTICAL_HAL_PATH, str(path))
-                row[1] = self._display_label(devnode, label)
+
-                row[2] = True
+        logging.debug("Optical device added: %s" % row)
-                row[3] = path
+        self.emit("optical-added", row)
-                if active == -1:
-                    self.widget.set_active(idx)
-            idx = idx + 1
 
     def _device_removed(self, path):
-        active = self.widget.get_active()
+        logging.debug("Optical device removed: %s" % path)
-        idx = 0
+        self.emit("optical-removed", path)
-        # Search for the row containing matching HAL volume path
-        # and update (clear) it, de-activating it if its currently
-        # selected
-        for row in self.model:
-            if row[3] == path:
-                row[1] = self._display_label(row[0], None)
-                row[2] = False
-                row[3] = None
-                if idx == active:
-                    self.widget.set_active(-1)
-            idx = idx + 1
-
-        self.set_default_selection()
-
-    def _display_label(self, devnode, label):
-        if not label:
-            label = _("No media present")
-        return "%s (%s)" % (label, devnode)
 
     def _fetch_device_info(self, path):
         label = None