When using logical pools, we had to trust the target->path
provided.
This parameter, however, can be completely ommited and we can get the
correct path using 'lvdisplay' command. In order not to omit the
target.path completely, we rather default it to '/dev/<source.name>'
which is used to check the pool anyway.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=952973
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Notes:
v3:
- just rebase
v2:
- don't drop target.path, but fix it instead to
'/dev/<source.name>'
Thanks to the change from v1, we can now safely drop all the hunks
from logical backend and even the dependency on lvdisplay command.
There might be some systems where the path is different and the part
of this patch using lvdisplay command would make most of them work.
However, checkPool() still depends on '/dev/<source.name>' and I
haven't found any other way how to fix that, so feel free to ACK just
the {conf,docs,tests}/ part in case you think that we shouldn't try to
support anything else than '/dev/<source.name>'.
configure.ac | 4 +
docs/schemas/storagepool.rng | 13 +++-
src/conf/storage_conf.c | 19 +++--
src/storage/storage_backend_logical.c | 88 ++++++++++++++--------
src/storage/storage_driver.c | 2 +-
tests/storagepoolxml2xmlin/pool-logical-create.xml | 2 +-
tests/storagepoolxml2xmlin/pool-logical-nopath.xml | 18 +++++
.../storagepoolxml2xmlout/pool-logical-nopath.xml | 19 +++++
tests/storagepoolxml2xmltest.c | 1 +
9 files changed, 125 insertions(+), 41 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-logical-nopath.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-logical-nopath.xml
diff --git a/configure.ac b/configure.ac
index b5af0d3..967e70a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1601,6 +1601,7 @@ if test "$with_storage_lvm" = "yes" || test
"$with_storage_lvm" = "check"; then
AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin])
AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin])
AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin])
+ AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin])
+
+ virCommandPtr cmd = virCommandNewArgList(LVDISPLAY,
+ "--columns",
+ "--options",
"lv_path",
+ "--noheadings",
+ "--unbuffered",
+ NULL);
Why are you introducing use of 'lvdisplay', when our existing code uses
'lvs' ? 'lvs' can tell you everything that 'lvdisplay' can and
handles
the case we have here
# lvdisplay --columns --options lv_path --noheadings --unbuffered vg_t500wlan/lv_root
/dev/vg_t500wlan/lv_root
# lvs --options lv_path --noheadings --unbuffered vg_t500wlan/lv_root
/dev/vg_t500wlan/lv_root
Regards,
Daniel
--
|: