On 08/02/2010 09:58 PM, Daniel Veillard wrote:
Hi Aurelien,
Hi Daniel,
Sorry for coming back so late, I accumulated a bit of backlog and
since this clearly wasn't for 0.8.3 it ended up on the backburner...
no worries, everyone has priorities.
Context : Some days ago I have decided to use infiniband for ISCSI
streams. Infiniband adds a new wonderful transport protocol : ISER.
This protocols is added to the well known the default TCP. (NB: ISER
= ISCSI + RDMA). I could not see any ISCSI transport protocol
modification support in libivirt (the default protocol tcp is even
hardcoded in the regex)
What i have done :
- did the shell testing for the iscsiadm
- the attached patch that corrects 2 typos in the original code and
switches completely the iscsi transport protocol from tcp to iser
(which is not ideal at all)
The typo should be applied separately, no need to wait for them
Cool, i have noticed they were included in my last git clone.
What should be done (imho):
- add iscsi transport protocol support (using my patch as a basis)
- add a new XML property/whatever_fits_the_projects_policy to the
storagepool object that allows user to pick the iscsi transport
protocol (default is tcp)
I was thinking of having something like :
<pool type="iscsi">
<name>volumename</name>
<source>
<host name="1.2.3.4"/>
<device path="IQNOFTARGET"/>
<transport protocol="iser"/>
</source>
<target>
<path>/dev/disk/by-path</path>
</target>
</pool>
Any comment on this ? Any help on the XML part ?
the XML addition of a transport element with a protocol attribute
looks fine to me. Values would for now be 'tcp' by default and
optionally 'iser'
This would be perfect
Best regards,
Aurélien
NB: the current iscsi transport protocols available are :
tcp(default), iser, qla4xxx, bnx2, and icxgb3i.
what about the other protocols ? Can you detect availability ? Can you
automatically set things up from libvirt ?
Yes :
root@mdc-lab-twin01-d:/sys/module/libiscsi/holders# ls
ib_iser iscsi_tcp
Meaning mdc-lab-twin01-d handles tcp and iser.
PS: i'm still doing extensive testing of my patch
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
index f34123f..7c21ad7 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -113,11 +113,13 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool,
* # iscsiadm --mode session
* tcp: [1] 192.168.122.170:3260,1 demo-tgt-b
* tcp: [2] 192.168.122.170:3260,1 demo-tgt-a
+ * iser: [5] 10.111.140.1:3260,1 demo-tgt-d
+
*
* Pull out 2nd and 4th fields
*/
const char *regexes[] = {
- "^tcp:\\s+\\[(\\S+)\\]\\s+\\S+\\s+(\\S+)\\s*$"
+ "^iser:\\s+\\[(\\S+)\\]\\s+\\S+\\s+(\\S+)\\s*$"
};
Since regexes are a list of allowed regexps you should just add
"^iser:\\s+\\[(\\S+)\\]\\s+\\S+\\s+(\\S+)\\s*$"
as the second element in the array
Absolutely true !
int vars[] = {
2,
@@ -230,7 +232,7 @@ virStorageBackendIQNFound(virStoragePoolObjPtr pool,
out:
if (ret == IQN_MISSING) {
- VIR_DEBUG("Could not find interface witn IQN '%s'", iqn);
+ VIR_DEBUG("Could not find interface with IQN '%s'", iqn);
}
VIR_FREE(line);
@@ -293,7 +295,24 @@ virStorageBackendCreateIfaceIQN(virStoragePoolObjPtr pool,
if (virRun(cmdargv2,&exitstatus)< 0) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to run command '%s' to update iscsi interface with IQN '%s'"),
- cmdargv1[0], pool->def->source.initiator.iqn);
+ cmdargv2[0], pool->def->source.initiator.iqn);
Those 2 typo need to be fixed, I pushed this to git, no need to wait
+ goto out;
+ }
+
+ /* This part switches iscsi transport protocol from default (tcp) to iser
+ * TODO: add some XML variable variable for enabling/disabling this block*/
+ const char *const cmdargv3[] = {
+ ISCSIADM, "--mode", "iface", "--interface",&temp_ifacename[0],
+ "--op", "update", "--name iface.transport_name", "--value iser", NULL
+ };
+
+ /* Note that we ignore the exitstatus. Older versions of iscsiadm tools
+ * returned an exit status of> 0, even if they succeeded. We will just
+ * rely on whether iface file got updated properly. */
+ if (virRun(cmdargv3,&exitstatus)< 0) {
+ virStorageReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to run command '%s' to update iscsi interface with IQN '%s'"),
+ cmdargv3[0], pool->def->source.initiator.iqn);
goto out;
}
Well if you run this here you prevent normal tcp operations it seems.
True, but if we modifiy just a bit making the code replacing the value
(which is iser in this patch) with the xml attribute we discussed before
it would work for any transport protocol.
The big problem is to know when you need to do this, and either this can
be found from the environment settings on the node (possible ?) or
fallback to the user indicating this.
cf last answer
Doing the reading from configuration should not be hard look at the
existing conf code doing it, if needed grab me on IRC for help on the
XML parsing/saving code,
If i'm not mistaking to add the whole feature we just need to:
- add row(s) to the regexes table, or even widen the current regex to
accept \w instead of a given string (such as tcp:// or iser://)
- add the transport_protocol attribute
- replace the hardcoded "iser" string by the value of this new attribute
- add an availability check of the transport protocol using the
/sys/module/libiscsi/holders/ folder
Then everything should be ok. I'll contact you right away about the XML
attribute thingy, i should be able to deal with the rest.
Daniel
Aurélien
!DSPAM:4c5809c690974020910970!
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list