Code review comment for lp:~justin-fathomdb/nova/justinsb-hpsan2

If we're going to do something special with the return value of driver.create_volume and driver.update_volume, those should be documented as such, probably in nova/volume/driver.py#VolumeDriver (since all the others inherit from that).

I'm not well versed in iscsi, but I'd hope there would be a way to get more specific matches in _do_iscsi_discovery, maybe by making sure a field is surrounded by spaces, etc. The line I'm referencing is "if FLAGS.iscsi_ip_prefix in target and volume_name in target:". I'd hate for a target on 10.1.2.30 to match when we're looking for 10.1.2.3 just because one is a substring of the other.

It would also be great to document what fields should be in iscsi_properties, since this is a class that is inherited other places. A central point would be better than having to look it up in _run_iscsiadmin, discover_volume, etc, getting just a few of the fields each time.

I also see in HpSanISCSIDriver you're using CHAP, but in ISCSIDriver._do_iscsi_discovery you have a note that discovery doesn't work with CHAP. Can you let me know what I'm missing?

Also, I think it is strange for the volume layer to have knowledge of the database schema, and have to know column names and have a variable it returns called 'db_update'. Maybe just renaming it 'state_changes' or something would assuage my fear, and make me feel that if there was a schema change that would be okay because we don't actually have to pass the return value to database calls.

All in all a very nice contribution. ☺