9:23 p.m.
Only a v3 because the first two series "Addition host name check for
network storage pools" were not well received. If someone wants to
pick up/use the the hostname checking code from the v2, then have at it.
Patch 1, 2, 7, & 8 use the virSocketAddr code.
This series just focuses on the iSCSI duplicate source checks and in
particular the removal of the host name checks to be replaced primarily
by the duplicate source device path (or IQN) check. For iSCSI devices,
checking the resolved host name was not feasible. Instead, if the source
device path (IQN) of the new definition is the same as a running pool,
then the new definition will be rejected. The secondary check is for an
undocumented <inititor> element to define the initiator iqn (see bz 488142).
The first patch is "new" - it's something I discovered while doing some
extra testing with the v2 of the series.
The second patch provides the change and modifies the documentation. It
resolves two bz's as listed in the patch description.
v2 here:
http://www.redhat.com/archives/libvir-list/2015-April/msg01197.html
Much discussion took place in v1 though in 7/7:
http://www.redhat.com/archives/libvir-list/2015-April/msg00880.html
John Ferlan (2):
conf: Adjust duplicate source host port check
conf: Remove source host name check for iSCSI
docs/formatstorage.html.in | 18 ++++++++++++++----
docs/storage.html.in | 8 +++++++-
src/conf/storage_conf.c | 7 +++----
3 files changed, 24 insertions(+), 9 deletions(-)
--
2.1.0
9:23 p.m.
New subject: [libvirt] [v3 1/2] conf: Adjust duplicate source host port check
Only perform the port number check if the incoming definition actually
provides it. Since the port number is optional we could erroneously pass
a duplicate source host check since some storage pool backends which fill
in the default port number (e.g., iSCSI and sheepdog) for the started pool.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/storage_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 4852dfb..61365b9 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2412,7 +2412,8 @@ virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr
poolsrc,
if (poolsrc->nhost != 1 && defsrc->nhost != 1)
return false;
- if (poolsrc->hosts[0].port != defsrc->hosts[0].port)
+ if (defsrc->hosts[0].port &&
+ poolsrc->hosts[0].port != defsrc->hosts[0].port)
return false;
return STREQ(poolsrc->hosts[0].name, defsrc->hosts[0].name);
--
2.1.0
9:23 p.m.
New subject: [libvirt] [v3 2/2] conf: Remove source host name check for iSCSI
https://bugzilla.redhat.com/show_bug.cgi?id=1171984
https://bugzilla.redhat.com/show_bug.cgi?id=1188463
Remove the check for the source host name for iSCSI source XML processing
declaring duplicate sources when the source device path and if present the
initiator of a proposed storage pool matches an existing storage pool.
The backend iSCSI storage driver uses 'iscsiadm --mode session' to query
available iscsid target sessions. The output displayed is the IP address
and the IQN (target path) of known targets. The displayed IP address
is a resolved address based on the session --login. Additionally, iscsid
keeps track of the various ways to define the host name (IPv4 Address,
IPv6 Address, /etc/hosts, etc.) for that IQN (see output of an 'iscsiadm
--mode node'). If an incoming IQN matches and the host name provided by
libvirt is resolved to the existing IQN, then iscsid will "reuse" the
session. Although libvirt could do the same name resolution, if there
is a difference, iscsid could still declare two seemingly different sources
to be the same and not create a new session which means libvirt now has
two storage pools looking at the same source. Thus to avoid any strange
host name resolution issues, just rely on iscsid for that and do not
allow multiple pools on the same host to use the same device path (IQN).
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatstorage.html.in | 18 ++++++++++++++----
docs/storage.html.in | 8 +++++++-
src/conf/storage_conf.c | 4 +---
3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 479e73c..474abd6 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -78,7 +78,7 @@
...
<source>
<host name="iscsi.example.com"/>
- <device path="demo-target"/>
+ <device path="iqn.2013-06.com.example:iscsi-pool"/>
<auth type='chap' username='myname'>
<secret usage='mycluster_myname'/>
</auth>
@@ -118,8 +118,10 @@
(pool types <code>fs</code>, <code>logical</code>,
<code>disk</code>,
<code>iscsi</code>, <code>zfs</code>).
May be repeated multiple times depending on backend driver. Contains
- a single attribute <code>path</code> which is the fully qualified
- path to the block device node. <span class="since">Since
0.4.1</span></dd>
+ a single attribute <code>path</code> which is either the fully
+ qualified path to the block device node or for <code>iscsi</code>
+ the iSCSI Qualified Name (IQN).
+ <span class="since">Since 0.4.1</span></dd>
<dt><code>dir</code></dt>
<dd>Provides the source for pools backed by directories (pool
type <code>dir</code>), or optionally to select a subdirectory
@@ -292,7 +294,15 @@
or <code>device</code> element. Contains an attribute
<code>name</code>
which is the hostname or IP address of the server. May optionally
contain a <code>port</code> attribute for the protocol specific
- port number. <span class="since">Since
0.4.1</span></dd>
+ port number. Duplicate storage pool definition checks may perform
+ a cursory check that the same host name by string comparison in the
+ new pool does not match an existing pool's source host name when
+ combined with the <code>directory</code> or
<code>device</code>
+ element. Name resolution of the provided hostname or IP address
+ is left to the storage driver backend interactions with the remote
+ server. See the <a href="storage.html">storage driver
page</a> for
+ any restrictions for specific storage backends.
+ <span class="since">Since 0.4.1</span></dd>
<dt><code>auth</code></dt>
<dd>If present, the <code>auth</code> element provides the
authentication credentials needed to access the source by the
diff --git a/docs/storage.html.in b/docs/storage.html.in
index b9b503d..92e9ae7 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -439,6 +439,12 @@
to use <code>/dev/disk/by-path</code> or
<code>/dev/disk/by-id</code>
for the target path. These provide persistent stable naming for LUNs
</p>
+ <p>
+ The libvirt iSCSI storage backend does not resolve the provided
+ host name or IP address when finding the available target IQN's
+ on the host; therefore, defining two pools to use the same IQN
+ on the same host will fail the duplicate source pool checks.
+ </p>
<h3>Example pool input</h3>
<pre>
@@ -446,7 +452,7 @@
<name>virtimages</name>
<source>
<host name="iscsi.example.com"/>
- <device path="demo-target"/>
+ <device path="iqn.2013-06.com.example:iscsi-pool"/>
</source>
<target>
<path>/dev/disk/by-path</path>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 61365b9..65b2704 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2427,9 +2427,7 @@ virStoragePoolSourceISCSIMatch(virStoragePoolObjPtr matchpool,
virStoragePoolSourcePtr poolsrc = &matchpool->def->source;
virStoragePoolSourcePtr defsrc = &def->source;
- if (!virStoragePoolSourceMatchSingleHost(poolsrc, defsrc))
- return false;
-
+ /* NB: Do not check the source host name */
if (STRNEQ_NULLABLE(poolsrc->initiator.iqn, defsrc->initiator.iqn))
return false;
--
2.1.0
10:05 a.m.
On 12.05.2015 04:23, John Ferlan wrote:
Only a v3 because the first two series "Addition host name check
for
network storage pools" were not well received. If someone wants to
pick up/use the the hostname checking code from the v2, then have at it.
Patch 1, 2, 7, & 8 use the virSocketAddr code.
This series just focuses on the iSCSI duplicate source checks and in
particular the removal of the host name checks to be replaced primarily
by the duplicate source device path (or IQN) check. For iSCSI devices,
checking the resolved host name was not feasible. Instead, if the source
device path (IQN) of the new definition is the same as a running pool,
then the new definition will be rejected. The secondary check is for an
undocumented <inititor> element to define the initiator iqn (see bz 488142).
The first patch is "new" - it's something I discovered while doing some
extra testing with the v2 of the series.
The second patch provides the change and modifies the documentation. It
resolves two bz's as listed in the patch description.
v2 here:
http://www.redhat.com/archives/libvir-list/2015-April/msg01197.html
Much discussion took place in v1 though in 7/7:
http://www.redhat.com/archives/libvir-list/2015-April/msg00880.html
John Ferlan (2):
conf: Adjust duplicate source host port check
conf: Remove source host name check for iSCSI
docs/formatstorage.html.in | 18 ++++++++++++++----
docs/storage.html.in | 8 +++++++-
src/conf/storage_conf.c | 7 +++----
3 files changed, 24 insertions(+), 9 deletions(-)
ACK series.
Michal
3482
days inactive
3482
days old
3 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik