Hi William,

William Schumann wrote:
> Jan,
>
> auto_install.c:
> 756: iSCSI boot target might be identified and mounted, but 807 
> auto_select_install_target() does not automatically select it.

Hmm, looking at the code, the logic around selecting the iSCSI disk was
not changed - could you please elaborate more on why it should not work ?

If there is an iSCSI target disk selected for the installation, my 
understanding
is that iSCSI 'forces' subsequent code to select that target disk by
pretending that <target_device_name> was specified in AI manifest ? 
Might it be correct ?

If this is the case then it is made sure (by AI schema) that this is the 
only
criterion available and auto_select_install_target() has the only choice -
select the disk specified by ctd name.

I think that the correct action would be basically skip 
auto_select_install_target()
if iSCSI target disk was picked up for the installation. However, 
looking at that
function, it seems it does additional stuff after disk is selected, so 
we perhaps
can't completely bypass it.

> Possible actions:
> - moving 753-802 into auto_select_install_target() and if 
> iscsi_devnam[0] != '\0', take iscsi_devnam as the install disk and set 
> target_disk_identified = B_TRUE, or
> - passing in iscsi_devnam to auto_select_install_target() do the same 
> if iscsi_devnam[0] != '\0'
>
> I would have a slight preference in the latter, since the iSCSI mount 
> call would remain in the mainline of auto_install.c and not hidden 
> away in auto_select_install_target().
>
> auto_td.c:
> 214 suggest relaxing case for AIM_TARGET_DEVICE_BOOT_DISK using 
> strcasecmp(3c).  There is potential justification for insisting on 
> case matches for device names, since the subsequent routines aren't as 
> lenient, but there is no added value in insisting on a case match for 
> "boot_disk".

You are right - changed.

> 218 "but the boot disk was not found", since "none" implies that there 
> could be more than one.

Changed.

>
> The changes making the manifest elements mutually exclusive warrants a 
> release note, since it will break manifests, formerly passing 
> validation, now failing with a confusing error message. The most 
> likely case of this is combining the device name tag with the group 2 
> tags.  This point in the doc should be updated, too.

Agreed. These are actions which I think would help to mitigate that problem:

* behavior is documented in AI schema
* Flag day will be sent out to make people aware of this behavior
* I will follow up with Barbara on documentation part

Also, I am wondering that in case validation fails, if it might be useful
to capture output of xmllint(1) (delivered by SUNWlxml package ) in 
/tmp/install_log -
the output looks like

# xmllint --noout --relaxng /tmp/ai_manifest.rng /tmp/ai_manifest.xml
Relax-NG validity error : Extra element ai_target_device in interleave
/tmp/ai_manifest.xml:2: element ai_target_device: Relax-NG validity 
error : Element ai_manifest failed to validate content
/tmp/ai_manifest.xml fails to validate

This would provide user with initial clue what section of AI manifest
is the problematic one and what the potential issue is.

Please let me know what you think.

Thank you,
Jan


Reply via email to