8:06 a.m.
My thought was to combine patch 2 and 3 unless it was felt that disallowing
more than one Multipath pool to defined at one time (patch 3) was unnecessary
For patch 1, I did some searching and came across the following to describe
why /dev/mpath is no longer valid:
http://comments.gmane.org/gmane.linux.kernel.device-mapper.devel/13864
I kept /dev/mpath as a valid option only just in case there's some strange
back-compat issue or some platform where /dev/mpath is preferred/used
instead of /dev/mapper.
John Ferlan (3):
mpath: Update path in CheckPool function
docs: Clarify the Multipath target path value usage
mpath: Don't allow more than one mpath pool at a time
docs/formatstorage.html.in | 2 ++
docs/storage.html.in | 3 ++-
src/conf/storage_conf.c | 3 +++
src/storage/storage_backend_mpath.c | 3 ++-
4 files changed, 9 insertions(+), 2 deletions(-)
--
2.1.0
8:06 a.m.
New subject: [libvirt] [PATCH 1/3] mpath: Update path in CheckPool function
https://bugzilla.redhat.com/show_bug.cgi?id=1230664
Per the devmapper docs, use "/dev/mapper" or "/dev/dm-n" in order to
determine if a device is under control of DM Multipath.
So add "/dev/mapper" to the virFileExists, leaving the "/dev/mpath"
as a "legacy" option since it appears for a while it was the preferred
mechanism, but is no longer maintained
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_mpath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 971408a..ca9a62f 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -248,7 +248,8 @@ static int
virStorageBackendMpathCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
bool *isActive)
{
- *isActive = virFileExists("/dev/mpath");
+ *isActive = virFileExists("/dev/mapper") ||
+ virFileExists("/dev/mpath");
return 0;
}
--
2.1.0
8:06 a.m.
New subject: [libvirt] [PATCH 2/3] docs: Clarify the Multipath target path value usage
https://bugzilla.redhat.com/show_bug.cgi?id=1232606
As described in the storage.html.in, a Multipath pool ignores the target
element in favor of the default target mapping of /dev/mapper. Indicate so
for formatstorage.html.in as well.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatstorage.html.in | 2 ++
1 file changed, 2 insertions(+)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index b6f4361..a60e05e 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -401,6 +401,8 @@
guaranteed stable across reboots, since they are allocated on
demand. It is preferable to use a stable location such as one
of the <code>/dev/disk/by-{path|id|uuid|label}</code> locations.
+ For a Multipath pool (type <code>mpath</code>), the provided
+ value is ignored and the default value of "/dev/mapper" is used.
<span class="since">Since 0.4.1</span>
</dd>
<dt><code>permissions</code></dt>
--
2.1.0
8:06 a.m.
New subject: [libvirt] [PATCH 3/3] mpath: Don't allow more than one mpath pool at a time
https://bugzilla.redhat.com/show_bug.cgi?id=1232606
Since an mpath pool contains all the Multipath devices on a host, allowing
more than one defined on a host at a time should be disallowed under the
policy of disallowing duplicate source pools for the host.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/storage.html.in | 3 ++-
src/conf/storage_conf.c | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 0b467d5..6c8222a 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -505,7 +505,8 @@
<h2><a name="StorageBackendMultipath">Multipath
pools</a></h2>
<p>
This provides a pool that contains all the multipath devices on the
- host. Volume creating is not supported via the libvirt APIs.
+ host. Therefore, only one Multipath pool may be configured per host.
+ Volume creating is not supported via the libvirt APIs.
The target element is actually ignored, but one is required to appease
the libvirt XML parser.<br/>
<br/>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 4bbed4f..971f5c1 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2560,6 +2560,9 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn,
matchpool = pool;
break;
case VIR_STORAGE_POOL_MPATH:
+ /* Only one mpath pool is valid per host */
+ matchpool = pool;
+ break;
case VIR_STORAGE_POOL_RBD:
case VIR_STORAGE_POOL_LAST:
break;
--
2.1.0
9:27 a.m.
On 06/24/2015 09:06 AM, John Ferlan wrote:
My thought was to combine patch 2 and 3 unless it was felt that
disallowing
more than one Multipath pool to defined at one time (patch 3) was unnecessary
For patch 1, I did some searching and came across the following to describe
why /dev/mpath is no longer valid:
http://comments.gmane.org/gmane.linux.kernel.device-mapper.devel/13864
I kept /dev/mpath as a valid option only just in case there's some strange
back-compat issue or some platform where /dev/mpath is preferred/used
instead of /dev/mapper.
John Ferlan (3):
mpath: Update path in CheckPool function
docs: Clarify the Multipath target path value usage
mpath: Don't allow more than one mpath pool at a time
docs/formatstorage.html.in | 2 ++
docs/storage.html.in | 3 ++-
src/conf/storage_conf.c | 3 +++
src/storage/storage_backend_mpath.c | 3 ++-
4 files changed, 9 insertions(+), 2 deletions(-)
ping... another simple pair
Tks -
John
9:49 a.m.
On 24.06.2015 15:06, John Ferlan wrote:
My thought was to combine patch 2 and 3 unless it was felt that
disallowing
more than one Multipath pool to defined at one time (patch 3) was unnecessary
For patch 1, I did some searching and came across the following to describe
why /dev/mpath is no longer valid:
http://comments.gmane.org/gmane.linux.kernel.device-mapper.devel/13864
I kept /dev/mpath as a valid option only just in case there's some strange
back-compat issue or some platform where /dev/mpath is preferred/used
instead of /dev/mapper.
John Ferlan (3):
mpath: Update path in CheckPool function
docs: Clarify the Multipath target path value usage
mpath: Don't allow more than one mpath pool at a time
docs/formatstorage.html.in | 2 ++
docs/storage.html.in | 3 ++-
src/conf/storage_conf.c | 3 +++
src/storage/storage_backend_mpath.c | 3 ++-
4 files changed, 9 insertions(+), 2 deletions(-)
ACK and safe for the freeze.
Michal
3433
days inactive
3439
days old
5 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik