simh / simh

The Computer History Simulation Project
http://simh.trailing-edge.com
Other
1.66k stars 304 forks source link

ISO image file modified by SIMH and then unreadable #1094

Open pghardy opened 2 years ago

pghardy commented 2 years ago

sim> sh rz RZ 7 units RZ0 426MB, attached to lamps1_disk0.img, write enabled RZ25, autosize, RAW format RZ4 593MB, not attached, write locked RRD40, autosize, AUTO detect format RZ5 593MB, attached to empty.tape, write enabled TZ30, autosize, SIMH format sim> diskinfo VMS552H4.iso Container: /home/pi/VMS/LSL/LAMPS1/VMS552H4.iso Simulator: VAXstation 3100 M38 (KA42-B) DriveType: RRD40 SectorSize: 512 SectorCount: 401384 TransferElementSize: 1 AccessFormat: RAW CreationTime: Mon Jul 27 16:35:39 2020 Container Size: 205,508,608 bytes sim>

eschaton commented 2 years ago

This is a severe flaw in SIMH’s disk system that really needs to be addressed. It should not be appending arbitrary data other tools don’t understand to disk images. If it wants to maintain its own metadata, it should do so in a sidecar file. Just treating CD-ROM devices as read-only is grossly insufficient since a disk image can be used by physical hardware and more than one emulator.

The overall change to modify disk images outside their boundaries really needs to be reverted, and re-added after being made to use a sidecar file.

drovak commented 2 years ago

On Tue, Dec 28, 2021, 17:42 Chris Hanson @.***> wrote:

This is a severe flaw in SIMH’s disk system that really needs to be addressed.

I find it very disheartening that the ego of one individual is preventing this from being appropriately addressed.

ghost commented 2 years ago

There is another issue open on the same topic. I offer three possible solutions.

  1. Revert to SimH 3.X. I maintain this stream because I find 4.X's behavior unacceptable.
  2. If there is a "no modification" mode, make it the default.
  3. If there isn't such a mode, add it, and make it the default.

Then, those who like the invisible addition of metadata can have that done, by adding an enable to their .ini files; and those who don't won't see it anymore.

In general, the addition of incompatible behavior should be under control of a user accessible mode switch, and the default should be prior behavior.

al20878 commented 2 years ago

@pghardy Welcome to the club! See #1059

markpizz commented 2 years ago

The problem as described was indeed an oversight since CDROM devices were not attached (under the covers with as read only). That problem was fixed on 11/7/2021.

A ISO image that had been modified (adding the aforementioned meta-data) can be cleaned with:

sim> ZAP VMS552H4.iso

Thanks for finding this bug.

Note that the behavior which did not force a read-only attach was inherited from the pdp11_rq.c some 12 years ago. The 3.x code still does not force -R for CDROM devices at attach time.

al20878 commented 2 years ago

The 3.x code still does not force -R for CDROM devices at attach time.

Right, but even having opened a container in the update mode, since no write ops were coming from the guest OS for a CD-ROM device, the simulator would never have written anything back to the CD-ROM image! So the open mode for the container was actually irrelevant (but surely, opening read-only is a better choice in this case, in general). It was 4.x's behavior to judge if the container was "updateable" and immediately mark it so with a "signature" -- all stems from the fact of #1059.

al20878 commented 2 years ago

ZAP VMS552H4.iso

As a side note, I personally would have never looked at "ZAP" in help (until it was pointed out to me) while searching for a command to remove the signature, because the meaning of the verb suggests destroying an entire object, rather than stripping just a part of it. IDK why "zap" was a verb of choice for this kind of action.

ghost commented 2 years ago

For crying out loud... doesn't anyone read the source code anymore? Or understand it?

Note that the behavior which did not force a read-only attach was inherited from the pdp11_rq.c some 12 years ago. The 3.x code still does not force -R for CDROM devices at attach time.

The RRD40 (CDROM) is typed as READ ONLY in the pdp11_rq.c simulator code. Software CANNOT WRITE TO IT. Attaching it read-only is unnecessary and counterproductive.

Data files are attached read/write BY DEFAULT so that users can examine and modify them through the simulator interface, if they need to. This is true of read-only devices like paper-tape readers and card readers (and CDROMs) and write-only devices like paper-tape punches and line printers. This behavior has been present since day 0 of SimH.

The simulator interface can do lots of things that real hardware can't, like randomly access sequential devices, write to read-only devices, and read from write-only devices. These powerful capabilities are present to facilitate debugging, and I have used them on numerous occasions. What it never did, though, was modify a data file EXCEPT under EXPLICIT user direction.

hbent commented 2 years ago

I just ran into a variant of this. I had an ISO image mounted on a simulated RRD42. Then, since metadata had been written to the ISO image marking it as a specifically RRD42 image, I was unable to mount it on a simulated RRD40:

sim> set rq1 cdrom sim> att rq1 NetBSD-9.2-vax.iso %SIM-INFO: RQ1: Unit is read only %SIM-ERROR: RQ device: Non-existent parameter - RRD42 %SIM-ERROR: RQ1: Cannot set to drive type RRD42

Shouldn't the RRD40 accept RRD42 images, and vice versa? Certainly this would have worked on real hardware.

markpizz commented 2 years ago

I just ran into a variant of this. I had an ISO image mounted on a simulated RRD42. Then, since metadata had been written to the ISO image marking it as a specifically RRD42 image, I was unable to mount it on a simulated RRD40:

Were you using the latest code from the master branch?

When had you attached the NetBSD-9.2-vax.iso to a SCSI device set as RRD42, I suspect that you'd done it quite a while back.

If, on the original simulator with the RRD42 drive (using current version), you: sim> zap NetBSD-9.2-vax.iso sim> set rz1 RRD42 sim> attach rz1 NetBSD-9.2-vax.iso sim> detach rz1 sim> diskinfo NetBSD-9.2-vax.iso You'll notice no metadata.

You can then attach that same ISO to a (current version) Qbus/Unibus simulator without issue.

This problem was fixed with 5015d6a

More robust/reliable/clean support for ISO 9660 images is coming.