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(a)cn.fujitsu.com>
Date: Tue, 14 Feb 2012 13:47:06 +0800
Subject: [PATCH] blkid: start using libblkid directly instead
From: Wanlong Gao <gaowanlong(a)cn.fujitsu.com>
To: rjones(a)redhat.com
Cc: libguestfs(a)redhat.com
Use libblkid directly instead of the binary command in blkid.
Signed-off-by: Wanlong Gao <gaowanlong(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs