[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH RFC] blkid: start using libblkid directly instead



On 02/15/2012 04:42 PM, Wanlong Gao wrote:

> On 02/14/2012 06:53 PM, Karel Zak wrote:
> 
>> On Tue, Feb 14, 2012 at 08:56:12AM +0000, Richard W.M. Jones wrote:
>>> On Tue, Feb 14, 2012 at 03:30:26PM +0800, Wanlong Gao wrote:
>>> It's one of those things that might be an improvement, but it would
>>> have to be distinctly better than the existing approach of calling out
>>> to the 'blkid' binary.
>>>
>>> Is the libblkid library interface stable?  Is it meant to be used by
>>> programs other than the blkid binary?
>>
>>  Yes, for example the latest udevd is linked with libblkid.
>>
>>>> +#include <blkid/blkid.h>
>>
>>  #include <blkid.h>  if you want to use pkg-config
>>
>>>>  static char *
>>>>  get_blkid_tag (const char *device, const char *tag)
>>>>  {
>>>> -  char *out, *err;
>>>> -  int r;
>>>> +  int fd, rc;
>>>> +  const char *data = NULL;
>>>> +  blkid_probe blkprobe;
>>>>  
>>>> -  r = commandr (&out, &err,
>>>> -                "blkid",
>>>> -                /* Adding -c option kills all caching, even on RHEL 5. */
>>>> -                "-c", "/dev/null",
>>>> -                "-o", "value", "-s", tag, device, NULL);
>>>> -  if (r != 0 && r != 2) {
>>>> -    if (r >= 0)
>>>> -      reply_with_error ("%s: %s (blkid returned %d)", device, err, r);
>>>> -    else
>>>> -      reply_with_error ("%s: %s", device, err);
>>>> -    free (out);
>>>> -    free (err);
>>>> +  if (!device || !tag)
>>>>      return NULL;
>>>> -  }
>>>>  
>>>> -  free (err);
>>>> +  fd = open (device, O_RDONLY);
>>>> +  if (fd < 0)
>>>> +    return NULL;
>>>>  
>>>> -  if (r == 2) {                 /* means UUID etc not found */
>>>> -    free (out);
>>>> -    out = strdup ("");
>>>> -    if (out == NULL)
>>>> -      reply_with_perror ("strdup");
>>>> -    return out;
>>>> -  }
>>>> +  blkprobe = blkid_new_probe ();
>>>> +  if (!blkprobe)
>>>> +    goto done;
>>>> +  if (blkid_probe_set_device (blkprobe, fd, 0, 0))
>>>> +    goto done;
>>>> +
>>>> +  blkid_probe_enable_superblocks(blkprobe, 1);
>>
>>  You can also optionally enable
>>
>>       blkid_probe_enable_partitions(pr, 1);
>>
>>  to check for collisions between raids and partition tables (for example
>>  RAID1 could be partitioned -- then PT could be at the begin of the device
>>  and raid superblock at the end of the device, etc.)
>>
>>>> +  blkid_probe_set_superblocks_flags (blkprobe, BLKID_SUBLKS_LABEL |
>>>> +                                     BLKID_SUBLKS_UUID | BLKID_SUBLKS_TYPE);
>>>>  
>>>> -  /* Trim trailing \n if present. */
>>>> -  size_t len = strlen (out);
>>>> -  if (len > 0 && out[len-1] == '\n')
>>>> -    out[len-1] = '\0';
>>>> +  rc = blkid_do_safeprobe (blkprobe);
>>>> +  if (!rc)
>>>> +    blkid_probe_lookup_value (blkprobe, tag, &data, NULL);
>>>>  
>>>> -  return out;                   /* caller frees */
>>>> +done:
>>>> +  close (fd);
>>>> +  if (blkprobe)
>>>> +    blkid_free_probe (blkprobe);
>>>> +  return data ? strdup ((char *) data) : NULL;
>>
>>  after blkid_free_probe() the "data" are deallocated, must be:
>>
>>  done:
>>     close (fd);
>>     if (data)
>>         data = strdup ((char *) data);
>>     blkid_free_probe (blkprobe);
>>     return data;
>>
>>  Note that if-before-free is unnecessary for blkid_free_probe.
>>
>>     Karel
>>
> 
> 
> 
> I did like this.
> Now get a problem like below:
> 
> The *vfs_type* calls this *get_blkid_tag* function.
> I added a disk image to guestfish, this disk image
> contains a partition "/dev/vda1" with "ext4" filesystem.
> Then I ran *vfs_type /dev/vda1", I got the result,
> but with "vfs_type /dev/vda", it hang.
> 
> It seems that the vfs_type can't get a type of /dev/vda,
> it's right that the type of /dev/vda is unknown, but
> why can't it go back with unknown result or NULL result?
> 
> Maybe there's something wrong with the code of *get_blkid_tag* ?
> 
> Can you give some advise? Karel? Rich?


And the updated patch is below:

--------------------------------------------------------
>From c89fef505421266ac5138a4542236ef95184664b Mon Sep 17 00:00:00 2001
From: Wanlong Gao <gaowanlong cn fujitsu com>
Date: Tue, 14 Feb 2012 13:47:06 +0800
Subject: [PATCH] blkid: start using libblkid directly instead
From: Wanlong Gao <gaowanlong cn fujitsu com>
To: rjones redhat com
Cc: libguestfs redhat com

Use libblkid directly instead of the binary command in blkid.

Signed-off-by: Wanlong Gao <gaowanlong cn fujitsu com>
---
 appliance/packagelist.in |    1 +
 configure.ac             |   10 ++++++++
 daemon/Makefile.am       |    1 +
 daemon/blkid.c           |   58 +++++++++++++++++++++++----------------------
 4 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index 2ab6b80..575b8dd 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -45,6 +45,7 @@
   vim-minimal
   xz
   zfs-fuse
+  libblkid
 #endif /* REDHAT */
 
 #ifdef DEBIAN
diff --git a/configure.ac b/configure.ac
index cc11b2f..0beef28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -393,6 +393,16 @@ if test "x$have_libselinux" = "xyes"; then
         AC_DEFINE([HAVE_LIBSELINUX],[1],[Define to 1 if you have libselinux])
 fi
 AC_SUBST([SELINUX_LIB])
+AC_CHECK_HEADERS([blkid/blkid.h])
+AC_CHECK_LIB([blkid],[blkid_new_probe],[
+        have_libblkid="$ac_cv_header_blkid_blkid_h"
+        LIBBLKID="-lblkid"
+        LIBS="$LIBS $LIBBLKID"
+        ],[have_libblkid=no])
+if test "x$have_libblkid" = "xyes"; then
+        AC_DEFINE([HAVE_LIBBLKID],[1],[Define to 1 if you have libblkid])
+fi
+AC_SUBST([LIBBLKID])
 
 dnl Check for systemtap/DTrace userspace probes (optional).
 dnl http://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 3a698cc..5e4db57 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -167,6 +167,7 @@ guestfsd_LDADD = \
 	liberrnostring.a \
 	libprotocol.a \
 	$(SELINUX_LIB) \
+	$(LIBBLKID) \
 	$(AUGEAS_LIBS) \
 	$(top_builddir)/gnulib/lib/.libs/libgnu.a \
 	$(GETADDRINFO_LIB) \
diff --git a/daemon/blkid.c b/daemon/blkid.c
index 8728c29..fdde5f8 100644
--- a/daemon/blkid.c
+++ b/daemon/blkid.c
@@ -23,6 +23,8 @@
 #include <string.h>
 #include <unistd.h>
 #include <limits.h>
+#include <fcntl.h>
+#include <blkid/blkid.h>
 
 #include "daemon.h"
 #include "actions.h"
@@ -30,40 +32,40 @@
 static char *
 get_blkid_tag (const char *device, const char *tag)
 {
-  char *out, *err;
-  int r;
+  int fd, rc;
+  const char *data = NULL;
+  blkid_probe blkprobe;
 
-  r = commandr (&out, &err,
-                "blkid",
-                /* Adding -c option kills all caching, even on RHEL 5. */
-                "-c", "/dev/null",
-                "-o", "value", "-s", tag, device, NULL);
-  if (r != 0 && r != 2) {
-    if (r >= 0)
-      reply_with_error ("%s: %s (blkid returned %d)", device, err, r);
-    else
-      reply_with_error ("%s: %s", device, err);
-    free (out);
-    free (err);
+  if (!device || !tag)
     return NULL;
-  }
 
-  free (err);
+  fd = open (device, O_RDONLY);
+  if (fd < 0)
+    return NULL;
 
-  if (r == 2) {                 /* means UUID etc not found */
-    free (out);
-    out = strdup ("");
-    if (out == NULL)
-      reply_with_perror ("strdup");
-    return out;
-  }
+  blkprobe = blkid_new_probe ();
+  if (!blkprobe)
+    goto done;
+  if (blkid_probe_set_device (blkprobe, fd, 0, 0))
+    goto done;
+
+  blkid_probe_enable_superblocks (blkprobe, 1);
+  blkid_probe_enable_partitions (blkprobe, 1);
+
+  blkid_probe_set_superblocks_flags (blkprobe, BLKID_SUBLKS_LABEL |
+                                     BLKID_SUBLKS_UUID | BLKID_SUBLKS_TYPE);
+
+  rc = blkid_do_safeprobe (blkprobe);
+  if (!rc)
+    blkid_probe_lookup_value (blkprobe, tag, &data, NULL);
 
-  /* Trim trailing \n if present. */
-  size_t len = strlen (out);
-  if (len > 0 && out[len-1] == '\n')
-    out[len-1] = '\0';
+done:
+  close (fd);
+  if (data)
+    data = strdup ((char *) data);
+  blkid_free_probe (blkprobe);
 
-  return out;                   /* caller frees */
+  return (char *) data;
 }
 
 char *
-- 
1.7.9

--------------------------------------------------------

> 
> Thanks
> -Wanlong Gao
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs redhat com
> https://www.redhat.com/mailman/listinfo/libguestfs
> 



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]