On Tue, Apr 10, 2018 at 12:42:52PM +0200, Pino Toscano wrote: > This way the Lvm module contains only the OCaml implementations of LVM > daemon APIs. > > This is simple refactoring, with no functional changes.
Looks like simple code motion as you say, so ACK. Rich. > daemon/Makefile.am | 2 ++ > daemon/findfs.ml | 2 +- > daemon/inspect_fs_unix_fstab.ml | 2 +- > daemon/lvm.ml | 27 ----------------------- > daemon/lvm.mli | 10 --------- > daemon/lvm_utils.ml | 48 > +++++++++++++++++++++++++++++++++++++++++ > daemon/lvm_utils.mli | 27 +++++++++++++++++++++++ > 7 files changed, 79 insertions(+), 39 deletions(-) > create mode 100644 daemon/lvm_utils.ml > create mode 100644 daemon/lvm_utils.mli > > diff --git a/daemon/Makefile.am b/daemon/Makefile.am > index 9dbd375f5..9cd34ff75 100644 > --- a/daemon/Makefile.am > +++ b/daemon/Makefile.am > @@ -268,6 +268,7 @@ SOURCES_MLI = \ > link.mli \ > listfs.mli \ > lvm.mli \ > + lvm_utils.mli \ > md.mli \ > mount.mli \ > mountable.mli \ > @@ -296,6 +297,7 @@ SOURCES_ML = \ > ldm.ml \ > link.ml \ > lvm.ml \ > + lvm_utils.ml \ > findfs.ml \ > md.ml \ > mount.ml \ > diff --git a/daemon/findfs.ml b/daemon/findfs.ml > index f613015f0..c24a194e3 100644 > --- a/daemon/findfs.ml > +++ b/daemon/findfs.ml > @@ -44,7 +44,7 @@ and findfs tag str = > > if String.is_prefix out "/dev/mapper/" || > String.is_prefix out "/dev/dm-" then ( > - match Lvm.lv_canonical out with > + match Lvm_utils.lv_canonical out with > | None -> > (* Ignore the case where 'out' doesn't appear to be an LV. > * The best we can do is return the original as-is. > diff --git a/daemon/inspect_fs_unix_fstab.ml b/daemon/inspect_fs_unix_fstab.ml > index 43cec2323..edb797e3f 100644 > --- a/daemon/inspect_fs_unix_fstab.ml > +++ b/daemon/inspect_fs_unix_fstab.ml > @@ -327,7 +327,7 @@ and resolve_fstab_device spec md_map os_type = > * we have implemented lvm_canonical_lv_name in the daemon. > *) > try > - match Lvm.lv_canonical spec with > + match Lvm_utils.lv_canonical spec with > | None -> Mountable.of_device spec > | Some device -> Mountable.of_device device > with > diff --git a/daemon/lvm.ml b/daemon/lvm.ml > index ed4ed7462..ef45ed4bc 100644 > --- a/daemon/lvm.ml > +++ b/daemon/lvm.ml > @@ -97,30 +97,3 @@ and filter_convert_old_lvs_output out = > ) lines in > > List.sort compare lines > - > -(* Convert a non-canonical LV path like /dev/mapper/vg-lv or /dev/dm-0 > - * to a canonical one. > - * > - * This is harder than it should be. A LV device like /dev/VG/LV is > - * really a symlink to a device-mapper device like /dev/dm-0. However > - * at the device-mapper (kernel) level, nothing is really known about > - * LVM (a userspace concept). Therefore we use a convoluted method to > - * determine this, by listing out known LVs and checking whether the > - * rdev (major/minor) of the device we are passed matches any of them. > - * > - * Note use of 'stat' instead of 'lstat' so that symlinks are fully > - * resolved. > - *) > -let lv_canonical device = > - let stat1 = stat device in > - let lvs = lvs () in > - try > - Some ( > - List.find ( > - fun lv -> > - let stat2 = stat lv in > - stat1.st_rdev = stat2.st_rdev > - ) lvs > - ) > - with > - | Not_found -> None > diff --git a/daemon/lvm.mli b/daemon/lvm.mli > index e9a6faeca..592168433 100644 > --- a/daemon/lvm.mli > +++ b/daemon/lvm.mli > @@ -17,13 +17,3 @@ > *) > > val lvs : unit -> string list > - > -val lv_canonical : string -> string option > -(** Convert a non-canonical LV path like /dev/mapper/vg-lv or /dev/dm-0 > - to a canonical one. > - > - On error this raises an exception. There are two possible non-error > - return cases: > - > - Some lv = conversion was successful, returning the canonical LV > - None = input path was not an LV, it could not be made canonical *) > diff --git a/daemon/lvm_utils.ml b/daemon/lvm_utils.ml > new file mode 100644 > index 000000000..a891193df > --- /dev/null > +++ b/daemon/lvm_utils.ml > @@ -0,0 +1,48 @@ > +(* guestfs-inspection > + * Copyright (C) 2009-2018 Red Hat Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + *) > + > +open Unix > + > +open Utils > + > +(* Convert a non-canonical LV path like /dev/mapper/vg-lv or /dev/dm-0 > + * to a canonical one. > + * > + * This is harder than it should be. A LV device like /dev/VG/LV is > + * really a symlink to a device-mapper device like /dev/dm-0. However > + * at the device-mapper (kernel) level, nothing is really known about > + * LVM (a userspace concept). Therefore we use a convoluted method to > + * determine this, by listing out known LVs and checking whether the > + * rdev (major/minor) of the device we are passed matches any of them. > + * > + * Note use of 'stat' instead of 'lstat' so that symlinks are fully > + * resolved. > + *) > +let lv_canonical device = > + let stat1 = stat device in > + let lvs = Lvm.lvs () in > + try > + Some ( > + List.find ( > + fun lv -> > + let stat2 = stat lv in > + stat1.st_rdev = stat2.st_rdev > + ) lvs > + ) > + with > + | Not_found -> None > diff --git a/daemon/lvm_utils.mli b/daemon/lvm_utils.mli > new file mode 100644 > index 000000000..b25a9a706 > --- /dev/null > +++ b/daemon/lvm_utils.mli > @@ -0,0 +1,27 @@ > +(* guestfs-inspection > + * Copyright (C) 2009-2018 Red Hat Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + *) > + > +val lv_canonical : string -> string option > +(** Convert a non-canonical LV path like /dev/mapper/vg-lv or /dev/dm-0 > + to a canonical one. > + > + On error this raises an exception. There are two possible non-error > + return cases: > + > + Some lv = conversion was successful, returning the canonical LV > + None = input path was not an LV, it could not be made canonical *) > -- > 2.14.3 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs