On 09/16/11 18:05, Jack Schwartz wrote:
Hi everyone.
During the review period I came to find a flaw in the code, and had to
revise some of it. Please see the bug report for details. This review
accounts for this plus all other relevant code review comments
previously received.
Hopefully this is the last update to this code review. I have retested
this code and it now works for the cases it failed before, as well as
for the ones which passed. See below for testing info. It is also 15
lines simpler than before.
webrev vs slim_source:
https://cr.opensolaris.org/action/browse/caiman/schwartz/7090604_3
webrev vs previous webrev:
https://cr.opensolaris.org/action/browse/caiman/schwartz/7090604_3_2
I would like to ask Ethan and Darren to re-review this code as they are
most familiar with it at this point, and for Dave to bless it so that it
gets in before 175 closes.
Approved from my end.
Dave
Thanks,
Jack
P.S. Note: due to curtailment of new messages at this point, I could
only reuse the error messages I already had. The new method in
process_dtd.py could use a better one, and I will be filing a bug to fix
this later once this wad is approved for push.
P.P.S. New passing "aimanifest add" cases with the following paths:
disk
- Errs due to ambiguity of possible parent paths
disk/disk_name@name
- Errs due to ambiguity of possible parent paths
target
- Succeeds when manifest doesn't already have a target.
- Errs when manifest already has a target.
target/disk/disk_name@name
- succeeds. Adds new disk each time
zvol@action
- succeeds. Adds a new zvol to the target/logical each time
badname
- Errs out
disk/badname
- Errs out
I also regression tested other cases which used to work, including:
- simple absolute cases such as:
/auto_install/ai_instance/software/source/publisher/origin
- non-simple absolute cases such as:
/auto_install/ai_instance[@name=default]/software/software_data[@action=install]/name
- non-simple, non-absolute cases such as:
ai_instance[@name=default]/software/software_data[@action=install]/name
software/software_data[@action=install]/name
On 09/14/11 07:59 PM, Jack Schwartz wrote:
Hi everyone.
Seeking two reviews by Thursday COB and Dave's approval please. I'd
like to request Ethan be one of the reviewers.
Here is a code review that fixes a hole in aimanifest path processing.
Without this bugfix,
aimanifest add diskname@name c0t0d0s0
or other references to non-absolute paths which have no values in them
fails.
webrev:
https://cr.opensolaris.org/action/browse/caiman/schwartz/7090604_1
bug report:
http://monaco.us.oracle.com/detail.jsf?cr=7090604
While it introduces a new method to find the path leading to the
referenced element, all code introduced by this fix affects only the
failing cases; codepaths of working cases are not affected.*
* While in there, I added some additional args checking for set(),
get() and add(), which I have tested but can remove if it makes the
difference between being allowed in or not.
Thanks for your time,
Jack
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
