6:09 p.m.
Originally posted as an RFC:
http://www.redhat.com/archives/libvir-list/2015-December/msg00200.html
I've made further adjustments/changes and actually tested in a multipath
environment configured with the issue.
For these patches, rather than making a "general" flag for the pool,
I chose to add an attribute to the <source> 'device'. This way it was
much more clear what the attribute was affecting.
Additionally, it turned out that the issue was showing up for both
'user_friendly_names' and for custom 'alias' name entries in the
/etc/multipath.conf file so rather than being a user_friendly_name
type flag - I just chose to specific state what it was - a way to
not have the partition separator flag "p" added to the generated name.
Also it was a bit more difficult to describe the use of the flag
especially when it was so specific to one backing store type.
John Ferlan (2):
conf: Add storage pool device attribute part_separator
storage: Add new flag for libvirt_parthelper
docs/formatstorage.html.in | 30 ++++++++++++++++++++--
docs/schemas/storagepool.rng | 5 ++++
src/conf/storage_conf.c | 26 +++++++++++++++++--
src/conf/storage_conf.h | 3 ++-
src/storage/parthelper.c | 15 ++++++++---
src/storage/storage_backend_disk.c | 11 +++++++-
.../pool-disk-device-nopartsep.xml | 14 ++++++++++
.../pool-disk-device-nopartsep.xml | 14 ++++++++++
tests/storagepoolxml2xmltest.c | 1 +
9 files changed, 110 insertions(+), 9 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
--
2.5.0
6:09 p.m.
New subject: [libvirt] [PATCH 1/2] conf: Add storage pool device attribute part_separator
Add a new storage pool source device attribute 'part_separator=[yes|no]'
in order to allow a 'disk' storage pool using a device mapper multipath
device to not add the "p" partition separator to the generated device
name when libvirt_parthelper is run.
This will allow libvirt to find device mapper multipath devices which were
configured in /etc/multipath.conf to use 'user_friendly_names' or custom
'alias' names for the LUN.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatstorage.html.in | 30 ++++++++++++++++++++--
docs/schemas/storagepool.rng | 5 ++++
src/conf/storage_conf.c | 26 +++++++++++++++++--
src/conf/storage_conf.h | 3 ++-
.../pool-disk-device-nopartsep.xml | 14 ++++++++++
.../pool-disk-device-nopartsep.xml | 14 ++++++++++
tests/storagepoolxml2xmltest.c | 1 +
7 files changed, 88 insertions(+), 5 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index a60e05e..4965a4c 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -90,6 +90,14 @@
<pre>
...
<source>
+ <device path='/dev/mapper/mpatha'
part_separator='no'/>
+ <format type='gpt'/>
+ </source>
+ ...</pre>
+
+ <pre>
+ ...
+ <source>
<adapter type='scsi_host' name='scsi_host1'/>
</source>
...</pre>
@@ -118,10 +126,28 @@
(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 either the fully
+ a required 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>
+ <span class="since">Since 0.4.1</span>
+ <p>An optional attribute <code>part_separator</code> for each
+ <code>path</code> may be supplied. Valid values for the attribute
+ may be either "yes" or "no". This attribute is to be used for
a
+ <code>disk</code> pool type using a <code>path</code> to
a
+ device mapper multipath device configured to utilize either
+ 'user_friendly_names' or a custom 'alias' name in the
+ /etc/multipath.conf. The attribute directs libvirt to not
+ generate device volume names with the partition character "p".
+ By default, when libvirt generates the partition names for
+ device mapper multipath devices it will add a "p" path separator
+ to the device name before adding the partition number. For example,
+ a <code>device path</code> of '/dev/mapper/mpatha' libvirt
would
+ generate partition names of '/dev/mapper/mpathap1',
+ '/dev/mapper/mpathap2', etc. for each partition found. With
+ this attribute set to "no", libvirt will not append the "p"
to
+ the name unless it ends with a number thus generating names
+ of '/dev/mapper/mpatha1', '/dev/mapper/mpatha2', etc.
+ <span class="since">Since
1.3.1</span></p></dd>
<dt><code>dir</code></dt>
<dd>Provides the source for pools backed by directories (pool
types <code>dir</code>, <code>netfs</code>,
<code>gluster</code>),
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 3a61f04..49d212f 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -278,6 +278,11 @@
<empty/>
<ref name='devextents'/>
</choice>
+ <optional>
+ <attribute name="part_separator">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
</element>
</define>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 7c81bab..3657dfd 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1,7 +1,7 @@
/*
* storage_conf.c: config handling for storage driver
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -528,6 +528,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
for (i = 0; i < nsource; i++) {
+ char *partsep;
virStoragePoolSourceDevice dev = { .path = NULL };
dev.path = virXMLPropString(nodeset[i], "path");
@@ -537,10 +538,25 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
+ partsep = virXMLPropString(nodeset[i], "part_separator");
+ if (partsep) {
+ dev.part_separator = virTristateBoolTypeFromString(partsep);
+ if (dev.part_separator <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("invalid part_separator setting
'%s'"),
+ partsep);
+ virStoragePoolSourceDeviceClear(&dev);
+ VIR_FREE(partsep);
+ goto cleanup;
+ }
+ VIR_FREE(partsep);
+ }
+
if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) {
virStoragePoolSourceDeviceClear(&dev);
goto cleanup;
}
+
}
source->dir = virXPathString("string(./dir/@path)", ctxt);
@@ -1051,8 +1067,14 @@ virStoragePoolSourceFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</device>\n");
} else {
- virBufferEscapeString(buf, "<device
path='%s'/>\n",
+ virBufferEscapeString(buf, "<device path='%s'",
src->devices[i].path);
+ if (src->devices[i].part_separator !=
+ VIR_TRISTATE_SWITCH_ABSENT) {
+ virBufferAsprintf(buf, " part_separator='%s'",
+
virTristateBoolTypeToString(src->devices[i].part_separator));
+ }
+ virBufferAddLit(buf, "/>\n");
}
}
}
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index ec59c17..f1dc62b 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -1,7 +1,7 @@
/*
* storage_conf.h: config handling for storage driver
*
- * Copyright (C) 2006-2008, 2010-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2008, 2010-2016 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -157,6 +157,7 @@ struct _virStoragePoolSourceDevice {
virStoragePoolSourceDeviceExtentPtr freeExtents;
char *path;
int format; /* Pool specific source format */
+ int part_separator; /* enum virTristateSwitch */
/* When the source device is a physical disk,
* the geometry data is needed
diff --git a/tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
b/tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
new file mode 100644
index 0000000..71b381e
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
@@ -0,0 +1,14 @@
+<pool type='disk'>
+ <name>multipath</name>
+ <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid>
+ <capacity unit='bytes'>0</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <available unit='bytes'>0</available>
+ <source>
+ <device path='/dev/mapper/mpatha' part_separator='no'/>
+ <format type='gpt'/>
+ </source>
+ <target>
+ <path>/dev</path>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
b/tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
new file mode 100644
index 0000000..71b381e
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
@@ -0,0 +1,14 @@
+<pool type='disk'>
+ <name>multipath</name>
+ <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid>
+ <capacity unit='bytes'>0</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <available unit='bytes'>0</available>
+ <source>
+ <device path='/dev/mapper/mpatha' part_separator='no'/>
+ <format type='gpt'/>
+ </source>
+ <target>
+ <path>/dev</path>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index b03c4b0..41d6987 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -80,6 +80,7 @@ mymain(void)
DO_TEST("pool-logical-nopath");
DO_TEST("pool-logical-create");
DO_TEST("pool-disk");
+ DO_TEST("pool-disk-device-nopartsep");
DO_TEST("pool-iscsi");
DO_TEST("pool-iscsi-auth");
DO_TEST("pool-netfs");
--
2.5.0
6:09 p.m.
New subject: [libvirt] [PATCH 2/2] storage: Add new flag for libvirt_parthelper
https://bugzilla.redhat.com/show_bug.cgi?id=1265694
In order to be able to process disk storage pool's using a multipath
device to handle the partitions, libvirt_parthelper will need a way to
not automatically add a partition separator "p" to the generated device
name for each partition found. This is designed to mimic the multipath
features known as 'user_friendly_names' and custom 'alias' name.
If the part_separator attribute is set to "no", then generation of the
multipath partition name will not include the "p" partition separator
unless the source device path name ends with a number. The generated
partition names that get passed back to libvirt are processed in order
to find the device mapper multipath (dm-#) path device.
For example, device path "/dev/mapper/mpatha" would create partitions
"/dev/mapper/mpatha1", "/dev/mapper/mpatha2", etc. instead of
"/dev/mapper/mpathap1", "/dev/mapper/mpathap2", etc. If the device
path ends with a number "/dev/mapper/mpatha1", then the algorithm
to generate names "/dev/mapper/mpatha1p1", "/dev/mapper/mpatha1p2",
etc.
would be utilized.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/parthelper.c | 15 ++++++++++++---
src/storage/storage_backend_disk.c | 11 ++++++++++-
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c
index 8de32fd..d1df068 100644
--- a/src/storage/parthelper.c
+++ b/src/storage/parthelper.c
@@ -10,7 +10,7 @@
* in a reliable fashion if merely after a list of partitions & sizes,
* though it is fine for creating partitions.
*
- * Copyright (C) 2007-2008, 2010, 2013 Red Hat, Inc.
+ * Copyright (C) 2007-2008, 2010, 2013, 2016 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -70,6 +70,7 @@ int main(int argc, char **argv)
const char *path;
char *canonical_path;
const char *partsep;
+ bool devmap_nopartsep = false;
if (setlocale(LC_ALL, "") == NULL ||
bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
@@ -80,14 +81,22 @@ int main(int argc, char **argv)
if (argc == 3 && STREQ(argv[2], "-g")) {
cmd = DISK_GEOMETRY;
+ } else if (argc == 3 && STREQ(argv[2], "-p")) {
+ devmap_nopartsep = true;
} else if (argc != 2) {
- fprintf(stderr, _("syntax: %s DEVICE [-g]\n"), argv[0]);
+ fprintf(stderr, _("syntax: %s DEVICE [-g]|[-p]\n"), argv[0]);
return 1;
}
path = argv[1];
if (virIsDevMapperDevice(path)) {
- partsep = "p";
+ /* The 'devmap_nopartsep' option will not append the partsep of
"p"
+ * onto the name unless the 'path' ends in a number
+ */
+ if (c_isdigit(path[strlen(path)-1]) || !devmap_nopartsep)
+ partsep = "p";
+ else
+ partsep = "";
if (VIR_STRDUP_QUIET(canonical_path, path) < 0)
return 2;
} else {
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index a83e340..3e0395d 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -1,7 +1,7 @@
/*
* storage_backend_disk.c: storage backend for disk handling
*
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2016 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -325,6 +325,15 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool,
pool->def->source.devices[0].path,
NULL);
+ /* Check for the presence of the part_separator='no'. Pass this
+ * along to the libvirt_parthelper as option '-p'. This will cause
+ * libvirt_parthelper to not append the "p" partition separator to
+ * the generated device name, unless the name ends with a number.
+ */
+ if (pool->def->source.devices[0].part_separator ==
+ VIR_TRISTATE_BOOL_NO)
+ virCommandAddArg(cmd, "-p");
+
/* If a volume is passed, virStorageBackendDiskMakeVol only updates the
* pool allocation for that single volume.
*/
--
2.5.0
4:06 p.m.
On 08.01.2016 18:09, John Ferlan wrote:
Originally posted as an RFC:
http://www.redhat.com/archives/libvir-list/2015-December/msg00200.html
It hasn't gotten much attention, has it? Sorry.
I've made further adjustments/changes and actually tested in a multipath
environment configured with the issue.
For these patches, rather than making a "general" flag for the pool,
I chose to add an attribute to the <source> 'device'. This way it was
much more clear what the attribute was affecting.
Additionally, it turned out that the issue was showing up for both
'user_friendly_names' and for custom 'alias' name entries in the
/etc/multipath.conf file so rather than being a user_friendly_name
type flag - I just chose to specific state what it was - a way to
not have the partition separator flag "p" added to the generated name.
Also it was a bit more difficult to describe the use of the flag
especially when it was so specific to one backing store type.
John Ferlan (2):
conf: Add storage pool device attribute part_separator
storage: Add new flag for libvirt_parthelper
docs/formatstorage.html.in | 30 ++++++++++++++++++++--
docs/schemas/storagepool.rng | 5 ++++
src/conf/storage_conf.c | 26 +++++++++++++++++--
src/conf/storage_conf.h | 3 ++-
src/storage/parthelper.c | 15 ++++++++---
src/storage/storage_backend_disk.c | 11 +++++++-
.../pool-disk-device-nopartsep.xml | 14 ++++++++++
.../pool-disk-device-nopartsep.xml | 14 ++++++++++
tests/storagepoolxml2xmltest.c | 1 +
9 files changed, 110 insertions(+), 9 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
ACK to both. But I'd push it after the release.
Michal
7:37 p.m.
On 01/15/2016 10:06 AM, Michal Privoznik wrote:
On 08.01.2016 18:09, John Ferlan wrote:
> Originally posted as an RFC:
>
> http://www.redhat.com/archives/libvir-list/2015-December/msg00200.html
It hasn't gotten much attention, has it? Sorry.
>
> I've made further adjustments/changes and actually tested in a multipath
> environment configured with the issue.
>
> For these patches, rather than making a "general" flag for the pool,
> I chose to add an attribute to the <source> 'device'. This way it was
> much more clear what the attribute was affecting.
>
> Additionally, it turned out that the issue was showing up for both
> 'user_friendly_names' and for custom 'alias' name entries in the
> /etc/multipath.conf file so rather than being a user_friendly_name
> type flag - I just chose to specific state what it was - a way to
> not have the partition separator flag "p" added to the generated name.
> Also it was a bit more difficult to describe the use of the flag
> especially when it was so specific to one backing store type.
>
> John Ferlan (2):
> conf: Add storage pool device attribute part_separator
> storage: Add new flag for libvirt_parthelper
>
> docs/formatstorage.html.in | 30 ++++++++++++++++++++--
> docs/schemas/storagepool.rng | 5 ++++
> src/conf/storage_conf.c | 26 +++++++++++++++++--
> src/conf/storage_conf.h | 3 ++-
> src/storage/parthelper.c | 15 ++++++++---
> src/storage/storage_backend_disk.c | 11 +++++++-
> .../pool-disk-device-nopartsep.xml | 14 ++++++++++
> .../pool-disk-device-nopartsep.xml | 14 ++++++++++
> tests/storagepoolxml2xmltest.c | 1 +
> 9 files changed, 110 insertions(+), 9 deletions(-)
> create mode 100644 tests/storagepoolxml2xmlin/pool-disk-device-nopartsep.xml
> create mode 100644 tests/storagepoolxml2xmlout/pool-disk-device-nopartsep.xml
>
ACK to both. But I'd push it after the release.
Now pushed - thanks.
John
3168
days inactive
3179
days old
4 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik