Ryan Barry has uploaded a new change for review. Change subject: Validate "Other Device" against live device ......................................................................
Validate "Other Device" against live device Previously, we didn't check whether or not "Other Device" input matched against a known live device or not (USB boot, etc). Do so. As part of this, slight rearrangement of storage.Device to improve responsiveness when looking for udev devices (we're assuming udev will not change during the install), and get the new validation workflow to properly invalid pages if a plugin's on_change throws an exception. Change-Id: I44b87876876965418765cbb6a0c0030bf143f156 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=885017 Signed-off-by: Ryan Barry <[email protected]> --- M src/ovirt/node/installer/core/boot_device_page.py M src/ovirt/node/installer/core/installation_device_page.py M src/ovirt/node/plugins.py M src/ovirt/node/utils/storage.py 4 files changed, 39 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/79/19879/1 diff --git a/src/ovirt/node/installer/core/boot_device_page.py b/src/ovirt/node/installer/core/boot_device_page.py index 8e03c52..8d47a63 100644 --- a/src/ovirt/node/installer/core/boot_device_page.py +++ b/src/ovirt/node/installer/core/boot_device_page.py @@ -18,7 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, # MA 02110-1301, USA. A copy of the GNU General Public License is # also available at http://www.gnu.org/copyleft/gpl.html. -from ovirt.node import plugins, ui, utils, valid +from ovirt.node import exceptions, plugins, ui, utils, valid import threading """ @@ -97,7 +97,12 @@ self.widgets["label.details"].set_device(device) if changes.contains_any(["boot.device.custom"]): - self._model.update(changes) + if self.storage_discovery.devices.live_disk_name() == \ + self.storage_discovery.devices.translate_device_name( + changes["boot.device.custom"]).path: + raise exceptions.InvalidData("Can't be the same as the live device") + else: + self._model.update(changes) def on_merge(self, effective_changes): changes = self.pending_changes(False) @@ -134,6 +139,7 @@ """ _all_devices = None do_fake = False + devices = None def __init__(self, do_fake): super(StorageDiscovery, self).__init__() @@ -141,6 +147,7 @@ def run(self): devices = utils.storage.Devices(fake=self.do_fake) + self.devices = devices self._all_devices = devices.get_all() def all_devices(self): diff --git a/src/ovirt/node/installer/core/installation_device_page.py b/src/ovirt/node/installer/core/installation_device_page.py index f793cec..cd88eed 100644 --- a/src/ovirt/node/installer/core/installation_device_page.py +++ b/src/ovirt/node/installer/core/installation_device_page.py @@ -125,9 +125,13 @@ self.logger.debug("selected devices: %s" % selected_devices) changes["installation.devices"] = selected_devices self._model.update(changes) - if changes.contains_any(["installation.device.custom"]): - self._model.update(changes) + if self.storage_discovery.devices.live_disk_name() == \ + self.storage_discovery.devices.translate_device_name( + changes["installation.device.custom"]).path: + raise exceptions.InvalidData("Can't be the same as the live device") + else: + self._model.update(changes) def on_merge(self, effective_changes): changes = self.pending_changes(False) diff --git a/src/ovirt/node/plugins.py b/src/ovirt/node/plugins.py index 747e6d9..bb8c192 100644 --- a/src/ovirt/node/plugins.py +++ b/src/ovirt/node/plugins.py @@ -296,6 +296,10 @@ self.__changes.update(change) except exceptions.InvalidData as e: + # If we caught it somewhere here, we failed custom validation + # in the plugin -- assume all the changes are invalid + self.__invalid_changes.update(dict((k, v) for (k,v) in + change.iteritems())) msg = e.message except Exception as e: diff --git a/src/ovirt/node/utils/storage.py b/src/ovirt/node/utils/storage.py index a050090..c83598e 100644 --- a/src/ovirt/node/utils/storage.py +++ b/src/ovirt/node/utils/storage.py @@ -85,33 +85,44 @@ self._fake_devices[args[1]] = Device(*tuple(args)) else: import ovirtnode.storage + from ovirtnode.ovirtfunctions import translate_multipath_device + from ovirtnode.ovirtfunctions import get_live_disk + self.translate_multipath_device = translate_multipath_device + self.get_live_disk = get_live_disk self._storage = ovirtnode.storage.Storage() def live_disk_name(self): """get the device name of the live-media we are booting from """ - from ovirtnode.ovirtfunctions import get_live_disk - name = get_live_disk() + name = self.get_live_disk() if not "/dev/mapper" in name: # FIXME explain ... name = "/dev/%s" % name.rstrip('0123456789') return name + + def translate_device_name(self, _dev): + """Discover whether a device can be mapped to a devicemapper name so + we can match arbitrary /dev/sd? names, such as flash drives + """ + dev = self.translate_multipath_device(_dev) + info = self.disk_dict[dev].split(",", 5) + device = Device(dev, *info) + return device def get_all(self): """Get all available storage devices attached to this system """ if self._fake_devices: return self._fake_devices - from ovirtnode.ovirtfunctions import translate_multipath_device - dev_names, disk_dict = self._storage.get_udev_devices() + self.dev_names, self.disk_dict = self._storage.get_udev_devices() devices = {} - for _dev in dev_names: - dev = translate_multipath_device(_dev) + for _dev in self.dev_names: + dev = self.translate_multipath_device(_dev) self.logger.debug("Checking device %s (%s)" % (dev, _dev)) if dev in devices: self.logger.warning("Device is already in dict: %s" % dev) continue - if dev not in disk_dict: + if dev not in self.disk_dict: self.logger.warning("Device in names but not in dict: " + "%s" % dev) continue @@ -119,10 +130,10 @@ self.logger.info("Ignoring device " + "%s it's the live media" % dev) continue - infos = disk_dict[dev].split(",", 5) + infos = self.disk_dict[dev].split(",", 5) device = Device(dev, *infos) device.name = os.path.basename(device.name).replace(" ", "") - device.name = translate_multipath_device(device.name) + device.name = self.translate_multipath_device(device.name) if device.name in devices: self.logger.debug("Device with same name already " + "exists: %s" % device.name) -- To view, visit http://gerrit.ovirt.org/19879 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I44b87876876965418765cbb6a0c0030bf143f156 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: Ryan Barry <[email protected]> _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
