Libguestfs - Jan 2018 - Re: [RFC PATCH v1 3/3] daemon: list-filesystems: Don't list partitioned md devices

On Tue, Jan 23, 2018 at 12:44:13AM +0200, Mykola Ivanets
wrote:
> Filter partitioned md devices out the same way as partitioned physical
devices are filtered out
> ---
>  daemon/listfs.ml | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/daemon/listfs.ml b/daemon/listfs.ml
> index 370ffb4..dc424f5 100644
> --- a/daemon/listfs.ml
> +++ b/daemon/listfs.ml
> @@ -24,24 +24,20 @@ 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
> +   * (RHBZ#590167). However vfs-type will fail to tell us anything
This whitespace change is wrong.
>     * 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.
> +   * get the list of partitions and exclude the corresponding devices.
>     *)
> -  let devices_containing_partitions = List.fold_left (
> -    fun set part ->
> -      StringSet.add (Devsparts.part_to_dev part) set
> -  ) StringSet.empty partitions in
> -  let devices = List.filter (
> -    fun dev ->
> -      not (StringSet.mem dev devices_containing_partitions)
> -  ) devices in
> +  let device_without_partitions device > +    not
(Devsparts.is_partitioned_device device) in
> +
> +  let devices = Devsparts.list_devices () in
> +  let devices = List.filter device_without_partitions devices in
> +  let mds = Md.list_md_devices () in
> +  let mds = List.filter device_without_partitions mds in
Yes, this is better than the previous approach with sets.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

2018-01-23 12:16 GMT+02:00 Richard W.M. Jones
<rjones@redhat.com>:
> On Tue, Jan 23, 2018 at 12:44:13AM +0200, Mykola Ivanets wrote:
>> Filter partitioned md devices out the same way as partitioned physical
devices are filtered out
>> ---
>>  daemon/listfs.ml | 22 +++++++++-------------
>>  1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/daemon/listfs.ml b/daemon/listfs.ml
>> index 370ffb4..dc424f5 100644
>> --- a/daemon/listfs.ml
>> +++ b/daemon/listfs.ml
>> @@ -24,24 +24,20 @@ 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
>> +   * (RHBZ#590167). However vfs-type will fail to tell us anything
>
> This whitespace change is wrong.
Ah, I didn't realize double-whitespace is used intentionally.
Seems sentences should be separated with double-whitespace?
What is the reason (just curious)?
>>     * 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.
>> +   * get the list of partitions and exclude the corresponding devices.
>>     *)
>> -  let devices_containing_partitions = List.fold_left (
>> -    fun set part ->
>> -      StringSet.add (Devsparts.part_to_dev part) set
>> -  ) StringSet.empty partitions in
>> -  let devices = List.filter (
>> -    fun dev ->
>> -      not (StringSet.mem dev devices_containing_partitions)
>> -  ) devices in
>> +  let device_without_partitions device >> +    not
(Devsparts.is_partitioned_device device) in
>> +
>> +  let devices = Devsparts.list_devices () in
>> +  let devices = List.filter device_without_partitions devices in
>> +  let mds = Md.list_md_devices () in
>> +  let mds = List.filter device_without_partitions mds in
>
> Yes, this is better than the previous approach with sets.
>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> Fedora Windows cross-compiler. Compile Windows programs, test, and
> build Windows installers. Over 100 libraries supported.
> http://fedoraproject.org/wiki/MinGW
--
    Nikolay Ivanets

On Tue, Jan 23, 2018 at 12:38:24PM +0200, Nikolay Ivanets
wrote:
> 2018-01-23 12:16 GMT+02:00 Richard W.M. Jones <rjones@redhat.com>:
> > On Tue, Jan 23, 2018 at 12:44:13AM +0200, Mykola Ivanets wrote:
> >> Filter partitioned md devices out the same way as partitioned
physical devices are filtered out
> >> ---
> >>  daemon/listfs.ml | 22 +++++++++-------------
> >>  1 file changed, 9 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/daemon/listfs.ml b/daemon/listfs.ml
> >> index 370ffb4..dc424f5 100644
> >> --- a/daemon/listfs.ml
> >> +++ b/daemon/listfs.ml
> >> @@ -24,24 +24,20 @@ 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
> >> +   * (RHBZ#590167). However vfs-type will fail to tell us
anything
> >
> > This whitespace change is wrong.
> 
> Ah, I didn't realize double-whitespace is used intentionally.
> Seems sentences should be separated with double-whitespace?
> What is the reason (just curious)?
It's part of the GNU coding standards that we (broadly) follow:

https://www.gnu.org/prep/standards/standards.html#Comments

More generally:

https://en.wikipedia.org/wiki/Sentence_spacing

Rich.
> >>     * 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.
> >> +   * get the list of partitions and exclude the corresponding
devices.
> >>     *)
> >> -  let devices_containing_partitions = List.fold_left (
> >> -    fun set part ->
> >> -      StringSet.add (Devsparts.part_to_dev part) set
> >> -  ) StringSet.empty partitions in
> >> -  let devices = List.filter (
> >> -    fun dev ->
> >> -      not (StringSet.mem dev devices_containing_partitions)
> >> -  ) devices in
> >> +  let device_without_partitions device > >> +    not
(Devsparts.is_partitioned_device device) in
> >> +
> >> +  let devices = Devsparts.list_devices () in
> >> +  let devices = List.filter device_without_partitions devices in
> >> +  let mds = Md.list_md_devices () in
> >> +  let mds = List.filter device_without_partitions mds in
> >
> > Yes, this is better than the previous approach with sets.
> >
> > Rich.
> >
> > --
> > Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
> > Read my programming and virtualization blog: http://rwmj.wordpress.com
> > Fedora Windows cross-compiler. Compile Windows programs, test, and
> > build Windows installers. Over 100 libraries supported.
> > http://fedoraproject.org/wiki/MinGW
> 
> --
>     Nikolay Ivanets
-- 
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