Re: [Xen-devel] block-iscsi with Xen 4.5 / 4.6

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, May 04, 2016 at 06:41:26PM +1000, Steven Haigh wrote:
> On 4/05/2016 5:34 PM, Roger Pau Monné wrote:
> > On Wed, May 04, 2016 at 03:06:23PM +1000, Steven Haigh wrote:
> > It is important for us to use the '-e' in order to make sure all the 
> > failure 
> > points are correctly handled, without the '-e' some command might fail and 
> > the script wouldn't realize.
> 
> I honestly think this is pretty nasty. While it may not be true of all
> scripts, the block-iscsi script can only really fail in a couple of
> places - yet we have this set of procedures called:
> 
> parse_target -> check_tools -> prepare -> add -> attach -> find_device
> -> write_dev.
> 
> At least check_tools, prepare, add, attach, find_device could all be
> rolled into a single function - as the majority of the rest is 1-4 lines
> of code.

No, check_tools is used by both the attach and the detach path, so it cannot 
be rolled into a single function together with the other ones, and the same 
applies to mostly all other functions (find_device is also shared between 
the add and remove functions).

IMHO, I think the current code is fine because each function has a small 
logical task to accomplish, so it's easy to make sure each function does 
what it's supposed to do, nothing more and nothing less. Batching everything 
into one big function would make this harder.

That doesn't mean that I'm not open to improving it, so if you think it 
would be better/easier using some other logical organization patches are 
welcome :).

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel