Improve the logic and data structure for SCSI ID management in Web UI (#893)

- Have the get_scsi_ids() utility method return a dict, while adding occupied_ids. Leverage this to improve the logic for detecting which IDs are available to be reserved in the Web UI. (Which fixes a recent regression bug that's causing no IDs to be detected as available to be reserved.)
- Improve /scsi/attach endpoint logic to capture dynamic parameter fields now prefixed with "param_" (previous it scanned for any arbitrary field, which wasn't very accurate or secure)
- Added Product string to the block_size:512 CD-ROM device, so that it's obvious when it's being used.
- Tweaked test data for attach_device tests
This commit is contained in:
Daniel Markstedt 2022-10-06 10:00:57 -07:00 committed by GitHub
parent a30438279e
commit 52ebb3a2ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 53 additions and 40 deletions

View File

@ -434,7 +434,7 @@
{
"device_type": "SCCD",
"vendor": null,
"product": null,
"product": "SCSI CD-ROM 512",
"revision": null,
"block_size": 512,
"size": null,

View File

@ -107,7 +107,7 @@
{% endif %}
</td>
<td style="text-align:center">
{% if device.device_type != "-" %}
{% if device.id in scsi_ids["occupied_ids"] %}
{% if device.device_type in REMOVABLE_DEVICE_TYPES and "No Media" not in device.status %}
<form action="/scsi/eject" method="post" onsubmit="return confirm('{{ _("Eject Disk? WARNING: On Mac OS, eject the Disk in the Finder instead!") }}')">
<input name="scsi_id" type="hidden" value="{{ device.id }}">
@ -277,8 +277,8 @@
<input name="file_size" type="hidden" value="{{ file['size'] }}">
<label for="id">{{ _("ID") }}</label>
<select name="scsi_id">
{% for id in scsi_ids %}
<option name="id" value="{{id}}"{% if id == recommended_id %} selected{% endif %}>
{% for id in scsi_ids["valid_ids"] %}
<option name="id" value="{{id}}"{% if id == scsi_ids["recommended_id"] %} selected{% endif %}>
{{ id }}
</option>
{% endfor %}
@ -373,11 +373,11 @@
<form action="/scsi/attach_device" method="post">
<input name="type" type="hidden" value="{{ type }}">
{% for key, value in device_types[type]["params"] | dictsort %}
<label for="{{ key }}">{{ key }}:</label>
<label for="param_{{ key }}">{{ key }}:</label>
{% if value.isnumeric() %}
<input name="{{ key }}" type="number" value="{{ value }}">
<input name="param_{{ key }}" id="param_{{ key }}" type="number" value="{{ value }}">
{% elif key == "interface" %}
<select name="interface">
<select name="param_{{ key }}" id="param_{{ key }}">
{% for if in netinfo["ifs"] %}
<option value="{{ if }}">
{{ if }}
@ -385,7 +385,7 @@
{% endfor %}
</select>
{% else %}
<input name="{{ key }}" type="text" size="{{ value|length }}" placeholder="{{ value }}">
<input name="param_{{ key }}" id="param_{{ key }}" type="text" size="{{ value|length }}" placeholder="{{ value }}">
{% endif %}
{% endfor %}
{% if type in REMOVABLE_DEVICE_TYPES %}
@ -419,8 +419,8 @@
{% endif %}
<label for="scsi_id">{{ _("SCSI ID:") }}</label>
<select name="scsi_id">
{% for id in scsi_ids %}
<option value="{{ id }}"{% if id == recommended_id %} selected{% endif %}>
{% for id in scsi_ids["valid_ids"] %}
<option value="{{ id }}"{% if id == scsi_ids["recommended_id"] %} selected{% endif %}>
{{ id }}
</option>
{% endfor %}
@ -537,8 +537,8 @@
<label for="scsi_id">{{ _("SCSI ID:") }}</label>
<form action="/files/download_to_iso" method="post">
<select name="scsi_id">
{% for id in scsi_ids %}
<option value="{{ id }}"{% if id == recommended_id %} selected{% endif %}>
{% for id in scsi_ids["valid_ids"] %}
<option value="{{ id }}"{% if id == scsi_ids["recommended_id"] %} selected{% endif %}>
{{ id }}
</option>
{% endfor %}

View File

@ -196,7 +196,7 @@ def index():
units += int(device["unit"])
reserved_scsi_ids = server_info["reserved_ids"]
scsi_ids, recommended_id = get_valid_scsi_ids(devices["device_list"], reserved_scsi_ids)
scsi_ids = get_valid_scsi_ids(devices["device_list"], reserved_scsi_ids)
formatted_devices = sort_and_format_devices(devices["device_list"])
image_suffixes_to_create = map_image_file_descriptions(
@ -243,7 +243,6 @@ def index():
CFG_DIR=CFG_DIR,
AFP_DIR=AFP_DIR,
scsi_ids=scsi_ids,
recommended_id=recommended_id,
attached_images=attached_images,
units=units,
reserved_scsi_ids=reserved_scsi_ids,
@ -586,25 +585,30 @@ def attach_device():
"""
Attaches a peripheral device that doesn't take an image file as argument
"""
params = {}
scsi_id = request.form.get("scsi_id")
unit = request.form.get("unit")
device_type = request.form.get("type")
drive_name = request.form.get("drive_name")
if not scsi_id:
return response(error=True, message=_("No SCSI ID specified"))
# Attempt to fetch the drive properties based on drive name
drive_props = None
if drive_name:
for drive in APP.config["RASCSI_DRIVE_PROPERTIES"]:
if drive["name"] == drive_name:
drive_props = drive
break
# Collect device parameters into a dictionary
PARAM_PREFIX = "param_"
params = {}
for item in request.form:
if item == "scsi_id":
scsi_id = request.form.get(item)
elif item == "unit":
unit = request.form.get(item)
elif item == "type":
device_type = request.form.get(item)
elif item == "drive_name":
drive_name = request.form.get(item)
for drive in APP.config["RASCSI_DRIVE_PROPERTIES"]:
if drive["name"] == drive_name:
drive_props = drive
break
else:
if item.startswith(PARAM_PREFIX):
param = request.form.get(item)
if param:
params.update({item: param})
params.update({item.replace(PARAM_PREFIX, ""): param})
error_url = "https://github.com/akuker/RASCSI/wiki/Dayna-Port-SCSI-Link"
error_msg = _("Please follow the instructions at %(url)s", url=error_url)
@ -653,6 +657,8 @@ def attach_image():
unit = request.form.get("unit")
device_type = request.form.get("type")
if not scsi_id:
return response(error=True, message=_("No SCSI ID specified"))
if not file_name:
return response(error=True, message=_("No image file to insert"))

View File

@ -18,6 +18,7 @@ def get_valid_scsi_ids(devices, reserved_ids):
Takes a list of (dict)s devices, and list of (int)s reserved_ids.
Returns:
- (list) of (int)s valid_ids, which are the SCSI ids that are not reserved
- (list) of (int)s occupied_ids, which are the SCSI ids were a device is attached
- (int) recommended_id, which is the id that the Web UI should default to recommend
"""
occupied_ids = []
@ -33,11 +34,15 @@ def get_valid_scsi_ids(devices, reserved_ids):
recommended_id = unoccupied_ids[-1]
else:
if occupied_ids:
recommended_id = occupied_ids.pop(0)
recommended_id = occupied_ids[0]
else:
recommended_id = 0
return valid_ids, recommended_id
return {
"valid_ids": valid_ids,
"occupied_ids": occupied_ids,
"recommended_id": recommended_id,
}
def sort_and_format_devices(devices):

View File

@ -42,8 +42,8 @@ def test_attach_image(http_client, create_test_image, detach_devices):
{
"type": "SCRM",
"drive_props": {
"vendor": "VENDOR",
"product": "PRODUCT",
"vendor": "HD VENDOR",
"product": "HD PRODUCT",
"revision": "0123",
"block_size": "512",
},
@ -54,8 +54,8 @@ def test_attach_image(http_client, create_test_image, detach_devices):
{
"type": "SCMO",
"drive_props": {
"vendor": "VENDOR",
"product": "PRODUCT",
"vendor": "MO VENDOR",
"product": "MO PRODUCT",
"revision": "0123",
"block_size": "512",
},
@ -66,17 +66,19 @@ def test_attach_image(http_client, create_test_image, detach_devices):
{
"type": "SCCD",
"drive_props": {
"vendor": "VENDOR",
"product": "PRODUCT",
"vendor": "CD VENDOR",
"product": "CD PRODUCT",
"revision": "0123",
"block_size": "512",
},
},
),
("Host Bridge", {"type": "SCBR", "interface": "eth0", "inet": "10.10.20.1/24"}),
("Ethernet Adapter", {"type": "SCDP", "interface": "eth0", "inet": "10.10.20.1/24"}),
# TODO: Find a portable way to detect network interfaces for testing
("Host Bridge", {"type": "SCBR", "param_inet": "192.168.0.1/24"}),
# TODO: Find a portable way to detect network interfaces for testing
("Ethernet Adapter", {"type": "SCDP", "param_inet": "192.168.0.1/24"}),
("Host Services", {"type": "SCHS"}),
("Printer", {"type": "SCLP", "timeout": 30, "cmd": "lp -oraw %f"}),
("Printer", {"type": "SCLP", "param_timeout": 60, "param_cmd": "lp -fart %f"}),
],
)
def test_attach_device(http_client, detach_devices, device_name, device_config):