Hi Drew,
Thanks for fixing that up. Looks good to me.
Cheers,
Niall
On 21/02/2012 03:07, Drew Fisher wrote:
Niall,
After thinking about it for a few seconds, I realized you were right.
I've got a webrev for just this one change:
http://mox.us.oracle.com/code/iscsi_fcoe_u1-work/webrev/
Can you take a look?
-Drew
On 2/19/12 10:45 PM, Niall Power wrote:
On 20/02/2012 16:32, Drew Fisher wrote:
I'll let Dermot handle the GUI specific ones, but you commented on
one other issue
On 2/19/12 8:56 PM, Niall Power wrote:
Hi Drew & Dermot,
Here are my comments for the GUI changes. I'll try to squeeze in
time to look at the target libraries also.
47 & 296: cache_update() It looks wrong to me for the GUI (or
other client) to be trying to manipulate libdiskmgt, which is just
an implementation detail of target_discovery. It sorta violates
that whole MVC thing :) Why can't target discovery take care of
this itself when doing iSCSI discovery?
Remember how discovery and physical.py did all that song and dance
about calling fork() so we could lay down a label and gather
geometry data? We had to do this because libdiskmgt had no external
function that we could use to force a rebuild of the cache.
Instead, the library rebuilt the cache every 60 seconds.
When we call cache_update(), it's because we want to force
libdiskmgt to find the newly mapped iSCSI LUN rather than waiting
for a maximum of 62-65 seconds (60 for the cache rebuild timer + 2-5
seconds for the cache refresh). Putting it directly in the hands of
the GUI (and TI - see disk_selection.py) lets the GUI control when
it wants to rebuild the cache AND to block while that cache rebuilds.
The GUI doesn't "want" to rebuild the cache, it just currently has
to. It would ideally rather not know about the cache :-)
We'll know for certain that the LUN is mapped and libdiskmgt knows
about it when the call returns.
Immediately after the GUI calls update_cache() it runs the target
discovery checkpoint for iSCSI:
298 logger.debug("Starting TD(iSCSI) with search_name: %s", iscsi.ctd)
299 kwargs = {"search_type": "disk", "search_name": iscsi.ctd}
300 # Either re-run an already-registered TD(iSCSI) checkpoint
301 # or register one for the first time
302 for checkpoint in engine._checkpoints:
303 if checkpoint.name == ISCSI_TARGET_DISCOVERY:
304 logger.debug("Re-using existing %s checkpoint" %
305 ISCSI_TARGET_DISCOVERY)
306 checkpoint.kwargs = kwargs
307 break
308 else:
309 # register a new iSCSI target discovery checkpoint
310 engine.register_checkpoint(ISCSI_TARGET_DISCOVERY,
311 "solaris_install/target/discovery",
312 "TargetDiscovery",
313 kwargs=kwargs,
314 insert_before=TRANSFER_PREP)
315 errsvc.clear_error_list()
316 engine.execute_checkpoints(start_from=ISCSI_TARGET_DISCOVERY,
317 pause_before=TRANSFER_PREP,
318 callback=td_iscsi_callback)
So why can't the checkpoint deal with calling cache_update() in its execute()
method?
Niall
-Drew
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
