From e7bde8d319d86305ee542e554118139f06239094 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Mon, 9 May 2016 14:57:17 -0400 Subject: [PATCH] storage: Fix algorithm generating path names for devmapper https://bugzilla.redhat.com/show_bug.cgi?id=1265694 Commit id '020135dc' didn't quite get the algorithm correct when a device mapper source ended with a non numeric value (e.g. ends with an alphabet value). This patch modifies the 'part_separator' logic to add the "p" separator to the attempted target path name only when specified as part_separator='yes'. For a source name that already ends with a number, the logic doesn't change as the part separator would need to be there. For a source name that ends with something other than a number, this allows the possibility that a "p" separator can be added. The default for one of these source devices is to not add the separator. The key for device mapper and the need for a partition separator "p" is the presence of a number in the last character of the device name link in /dev/mapper. A name such as "/dev/mapper/mpatha1" would generate a "/dev/mapper/mpatha1p1" partition, while "/dev/mapper/mpatha" would generate partition "/dev/mapper/mpatha1". Similarly for a device mapper entry not using friendly names or an alias, a device such as "/dev/mapper/3600a0b80005b10ca00005ad656fd8d93" would generate a paritition "/dev/mapper/3600a0b80005b10ca00005ad656fd8d93p1", while a device such as "/dev/mapper/3600a0b80005b10ca00005e115729093f" would generate a partition "/dev/mapper/3600a0b80005b10ca00005e115729093f1". The long number is the WWID of the device. It's also possible to assign an alias for a device mapper entry, that alias follows the same rules with respect to ending with a number or not when adding a "p" to create the target device path. --- docs/formatstorage.html.in | 18 +++++------------- src/storage/parthelper.c | 11 ++++++----- src/storage/storage_backend_disk.c | 9 +++++---- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 0356182127..94277a15ec 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -134,19 +134,11 @@ path may be supplied. Valid values for the attribute may be either "yes" or "no". This attribute is to be used for a disk pool type using a path 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 device path 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. + device mapper multipath device. Setting the attribute to "yes" + causes libvirt to attempt to generate and find target volume path's + using a "p" separator. The default algorithm used by device mapper + is to add the "p" separator only when the source device path ends + with a number. Since 1.3.1

dir
Provides the source for pools backed by directories (pool diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c index 6695f23896..d81b1779b6 100644 --- a/src/storage/parthelper.c +++ b/src/storage/parthelper.c @@ -69,7 +69,7 @@ int main(int argc, char **argv) const char *path; char *canonical_path; const char *partsep; - bool devmap_nopartsep = false; + bool devmap_partsep = false; if (virGettextInitialize() < 0) exit(EXIT_FAILURE); @@ -77,7 +77,7 @@ 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; + devmap_partsep = true; } else if (argc != 2) { fprintf(stderr, _("syntax: %s DEVICE [-g]|[-p]\n"), argv[0]); return 1; @@ -85,10 +85,11 @@ int main(int argc, char **argv) path = argv[1]; if (virIsDevMapperDevice(path)) { - /* The 'devmap_nopartsep' option will not append the partsep of "p" - * onto the name unless the 'path' ends in a number + /* If the path ends with a number or we explicitly request it for + * path, then append the "p" partition separator. Otherwise, if + * the path ends with a letter already, then no need for a separator. */ - if (c_isdigit(path[strlen(path)-1]) || !devmap_nopartsep) + if (c_isdigit(path[strlen(path)-1]) || devmap_partsep) partsep = "p"; else partsep = ""; diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 3e0395dec9..eadf8a37cf 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -325,13 +325,14 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, pool->def->source.devices[0].path, NULL); - /* Check for the presence of the part_separator='no'. Pass this + /* Check for the presence of the part_separator='yes'. 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. + * libvirt_parthelper to append the "p" partition separator to + * the generated device name for a source device which ends with + * a non-numeric value (e.g. mpatha would generate mpathap#). */ if (pool->def->source.devices[0].part_separator == - VIR_TRISTATE_BOOL_NO) + VIR_TRISTATE_BOOL_YES) virCommandAddArg(cmd, "-p"); /* If a volume is passed, virStorageBackendDiskMakeVol only updates the