Libguestfs - Apr 2014 - Re: [PATCH] listfs: If LDM not available, don't inhibit partition detection (RHBZ#1079182).

If a disk has type 0x42 partition (which would indicate LDM), but LDM
is not available then try parsing the partition anyway.  It might be
parseable as plain old NTFS.
---
 src/listfs.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/listfs.c b/src/listfs.c
index bbdb0a2..ffb0adc 100644
--- a/src/listfs.c
+++ b/src/listfs.c
@@ -47,8 +47,11 @@ guestfs__list_filesystems (guestfs_h *g)
 {
   size_t i;
   DECLARE_STRINGSBUF (ret);
+
   const char *lvm2[] = { "lvm2", NULL };
+  int has_lvm2 = guestfs_feature_available (g, (char **) lvm2);
   const char *ldm[] = { "ldm", NULL };
+  int has_ldm = guestfs_feature_available (g, (char **) ldm);
 
   CLEANUP_FREE_STRING_LIST char **devices = NULL;
   CLEANUP_FREE_STRING_LIST char **partitions = NULL;
@@ -83,7 +86,7 @@ guestfs__list_filesystems (guestfs_h *g)
 
   /* Use vfs-type to check for filesystems on partitions. */
   for (i = 0; partitions[i] != NULL; ++i) {
-    if (! is_mbr_partition_type_42 (g, partitions[i])) {
+    if (has_ldm == 0 || ! is_mbr_partition_type_42 (g, partitions[i])) {
       if (check_with_vfs_type (g, partitions[i], &ret) == -1)
         goto error;
     }
@@ -94,7 +97,7 @@ guestfs__list_filesystems (guestfs_h *g)
     if (check_with_vfs_type (g, mds[i], &ret) == -1)
       goto error;
 
-  if (guestfs_feature_available (g, (char **) lvm2)) {
+  if (has_lvm2 > 0) {
     /* Use vfs-type to check for filesystems on LVs. */
     lvs = guestfs_lvs (g);
     if (lvs == NULL) goto error;
@@ -104,7 +107,7 @@ guestfs__list_filesystems (guestfs_h *g)
         goto error;
   }
 
-  if (guestfs_feature_available (g, (char **) ldm)) {
+  if (has_ldm > 0) {
     /* Use vfs-type to check for filesystems on Windows dynamic disks. */
     ldmvols = guestfs_list_ldm_volumes (g);
     if (ldmvols == NULL) goto error;
-- 
1.8.5.3

On Friday 21 March 2014 15:24:01 Richard W.M. Jones
wrote:
> If a disk has type 0x42 partition (which would indicate LDM), but LDM
> is not available then try parsing the partition anyway.  It might be
> parseable as plain old NTFS.
Hasn't this been committed? It would seem fine.
> ---
>  src/listfs.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/listfs.c b/src/listfs.c
> index bbdb0a2..ffb0adc 100644
> --- a/src/listfs.c
> +++ b/src/listfs.c
> @@ -47,8 +47,11 @@ guestfs__list_filesystems (guestfs_h *g)
>  {
>    size_t i;
>    DECLARE_STRINGSBUF (ret);
> +
>    const char *lvm2[] = { "lvm2", NULL };
> +  int has_lvm2 = guestfs_feature_available (g, (char **) lvm2);
>    const char *ldm[] = { "ldm", NULL };
> +  int has_ldm = guestfs_feature_available (g, (char **) ldm);
Maybe it is more just matter of style, but has_lvm2 and has_ldm could be 
booleans to ease the code and the readability.

-- 
Pino Toscano

On Wed, Apr 09, 2014 at 03:12:59PM +0200, Pino Toscano
wrote:
> On Friday 21 March 2014 15:24:01 Richard W.M. Jones wrote:
> > If a disk has type 0x42 partition (which would indicate LDM), but LDM
> > is not available then try parsing the partition anyway.  It might be
> > parseable as plain old NTFS.
> 
> Hasn't this been committed? It would seem fine.
I didn't commit this because I wasn't really convinced it was correct
(at the time).  But looking at it now, I guess it seems fine.  For
regular Fedora/upstream builds we'll always have LDM.
> > ---
> >  src/listfs.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/listfs.c b/src/listfs.c
> > index bbdb0a2..ffb0adc 100644
> > --- a/src/listfs.c
> > +++ b/src/listfs.c
> > @@ -47,8 +47,11 @@ guestfs__list_filesystems (guestfs_h *g)
> >  {
> >    size_t i;
> >    DECLARE_STRINGSBUF (ret);
> > +
> >    const char *lvm2[] = { "lvm2", NULL };
> > +  int has_lvm2 = guestfs_feature_available (g, (char **) lvm2);
> >    const char *ldm[] = { "ldm", NULL };
> > +  int has_ldm = guestfs_feature_available (g, (char **) ldm);
> 
> Maybe it is more just matter of style, but has_lvm2 and has_ldm could be 
> booleans to ease the code and the readability.
They could be booleans, BUT I would have to write something like:

  bool has_lvm2 = guestfs_feature_available (g, (char **) lvm2) > 0;

Note the error case (-1) has to be considered at least, even if it is
ignored.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top