Re: [Libguestfs] [PATCH 19/27] daemon: Reimplement ‘list_filesystems’ API in the daemon, in OCaml.

2017-07-21 Thread Richard W.M. Jones
On Thu, Jul 20, 2017 at 05:13:34PM +0200, Pino Toscano wrote:
> > +  (* Use vfs-type to check for filesystems on partitions, but
> > +   * ignore MBR partition type 42 used by LDM.
> > +   *)
> > +  let ret =
> > +ret @
> > +  filter_map (
> > +fun part ->
> > +  if not has_ldm || not (is_mbr_partition_type_42 part) then
> > +check_with_vfs_type part
> > +  else
> > +None(* ignore type 42 *)
> > +  ) partitions in
> 
> Now this will run is_mbr_partition_type_42 on devices as well, in case
> ldm is available -- I guess it should not be an issue?

Can you explain a bit more?  I don't understand.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 19/27] daemon: Reimplement ‘list_filesystems’ API in the daemon, in OCaml.

2017-07-20 Thread Pino Toscano
On Friday, 14 July 2017 15:39:27 CEST Richard W.M. Jones wrote:
> +let rec list_filesystems () =
> +  let has_lvm2 = Lvm.available () in
> +  let has_ldm = Ldm.available () in
> +
> +  let devices = Devsparts.list_devices () in
> +  let partitions = Devsparts.list_partitions () in
> +  let mds = Md.list_md_devices () in
> +
> +  (* Look to see if any devices directly contain filesystems
> +   * (RHBZ#590167).  However vfs-type will fail to tell us anything
> +   * useful about devices which just contain partitions, so we also
> +   * get the list of partitions and exclude the corresponding devices
> +   * by using part-to-dev.
> +   *)
> +  let devices = List.fold_left (
> +fun devices part ->
> +  let d = Devsparts.part_to_dev part in
> +  List.filter ((<>) d) devices
> +  ) devices partitions in

Hm it took me a couple of reading to get it -- what would you think
about the following (untested):

  module StringSet = Set.Make (String)
  ...
  let devices_of_partitions = List.fold_left (
fun set part ->
  StringSet.add part set
  ) partitions StringSet.empty in
  (* Remove  *)
  let devices = List.filter (
fun dev ->
  not (StringSet.mem dev devices_of_partitions)
  ) devices in
  let devices = devices @ partitions in

> +  (* Use vfs-type to check for filesystems on devices. *)
> +  let ret = filter_map check_with_vfs_type devices in
> +
> +  (* Use vfs-type to check for filesystems on partitions, but
> +   * ignore MBR partition type 42 used by LDM.
> +   *)
> +  let ret =
> +ret @
> +  filter_map (
> +fun part ->
> +  if not has_ldm || not (is_mbr_partition_type_42 part) then
> +check_with_vfs_type part
> +  else
> +None(* ignore type 42 *)
> +  ) partitions in

Now this will run is_mbr_partition_type_42 on devices as well, in case
ldm is available -- I guess it should not be an issue?

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs