SDL-Hercules-390 / hyperion

The SDL Hercules 4.x Hyperion version of the System/370, ESA/390, and z/Architecture Emulator
Other
240 stars 90 forks source link

Option to fail gracefully if any CUU file(s) not found in subset defined via $(CCUU) statement #604

Closed littlejackal closed 8 months ago

littlejackal commented 10 months ago

This may be too niche to justify any enhancement efforts, but I propose the following:

CONTEXT:

When defining a large group of DASD, I often find it preferable to use a CCUU.## statement with generic CCKD filenames.

Example would be:

0100.cckd
0101.cckd
0102.cckd
0103.cckd
0104.cckd
0105.cckd
0106.cckd

Rather than adding 7 discrete 3390 statements in hercules.conf, I think condensing these into one is preferable:

0100.7 3390 $(CCUU).cckd

PROBLEM:

This works well if all 7 are present, but if you are missing any files from that subset then the DASD definition and load seems to end with the missing file.

Example I've observed -- If I have:

0100.cckd
0101.cckd
(0102.cckd is missing)
0103.cckd
0104.cckd
0105.cckd
0106.cckd

0100.7 3390 $(CCUU).cckd will load the first two but then stop loading any subsequent units from this statement after the missing file.

So in Hercules I only have 0100, 0101 defined. Not only are 0103 through 0106 not loaded but they're not even defined.

My example of seven files is simplistic because seven statements are easy to write, but when you start to go into double digits I think the value proposition for group-defining DASDs with formulaic file names presents itself.

SUGGESTION:

My own real world example:

HHC01413I Hercules version 4.6.0.10941-SDL-g65c97fd6

..

HHC00414I 0:00A0 CCKD file /opt/hercules/guests/MVS/dasd/by-unit/00A0: model 3390-3 cyls 3339 heads 15 tracks 50085 trklen 56832
HHC00401E 0:00A1 CKD file /opt/hercules/guests/MVS/dasd/by-unit/00A1: open error: not found
HHC00007I Previous message from function 'ckd_dasd_init_handler' at ckddasd.c(251)
HHC01463E 0:00A1 device initialization failed
HHC00007I Previous message from function 'attach_device' at config.c(1354)
HHC00100I Thread id 00007fcc0887e640, prio 2, name 'rubato_thread' started

The offending statement:

00A0.16 3390    $(DASD)/$(CCUU)         sf=$(SHDW)/$(CCUU)._.shadow

Files on disk:

hercules:~/guests/MVS/dasd/by-unit$ ls
00A0  00A2  00A3  00A4  00A5  00A6  00A7  00A8  00A9  00AA 
Fish-Git commented 8 months ago

This may be too niche to justify any enhancement efforts,

Not at all! What you propose seems quite reasonable to me. IMO it's actually a bona fide bug that should be fixed. An error on any given device definition should not prevent further processing of subsequent device definitions. The bug you describe is no different than doing:

0100.cckd
0101.cckd
(0102.cckd is missing)
0103.cckd
0104.cckd
0105.cckd
0106.cckd

and Hercules failing to define devices 0103-0106 simply because 0102 encountered an error. I think you'll agree that that would be wrong. The same can therefore be said of Hercules's handling of 0100.7 3390 $(CCUU).cckd: the simple act of the definition for device 0102 failing should not cause Hercules to not define devices 0103-0107. The situation is no different IMO.

Suffice to say the bug has now been identified and fixed by commit b7fd573935c9013f8bba8ff265ba11638d63ec96.

If you're interested in testing it (I already have myself, but you might want to confirm it for yourself), checkout the 'develop' branch and build that.

Thanks for reporting it, @littlejackal! You did a good job.  :)

Fish-Git commented 8 months ago

p.s. Your "fail=soft" idea is IMO overkill. It's not needed. As explained in my previous comment above, the behavior you described is actually incorrect behavior (i.e. a bona fide bug), so no new option is necessary.

littlejackal commented 8 months ago

Excellent, thank you for the feedback! The softfail option was only suggested because I wasn't sure if there was a legitimate reason for the "bug" behaviour that I just wasn't seeing, so I'm happy to go without.

Thanks for remediating; I'll definitely be giving this a test! Much appreciated.