[PATCH] Switch FreeBSD code from libvolume-id to libblkid

[Aurelien Jarno]

This might be a bit controversial given that libblkid is provided by
util-linux, but on the other hand libvolume-id was provided by udev...
---
 hald/freebsd/probing/Makefile.am     |    8 ++--
 hald/freebsd/probing/probe-storage.c |   38 ++++++++++++---
 hald/freebsd/probing/probe-volume.c  |   88 +++++++++++++++++++---------------
 3 files changed, 83 insertions(+), 51 deletions(-)

diff --git a/hald/freebsd/probing/Makefile.am b/hald/freebsd/probing/Makefile.am
index fdba1eb..59a933f 100644
--- a/hald/freebsd/probing/Makefile.am
+++ b/hald/freebsd/probing/Makefile.am
@@ -30,15 +30,15 @@ hald_probe_scsi_LDADD = \
  $(top_builddir)/hald/freebsd/libprobe/libhald_freebsd_probe.la
 
 hald_probe_storage_SOURCES = freebsd_dvd_rw_utils.c freebsd_dvd_rw_utils.h probe-storage.c
-hald_probe_storage_CPPFLAGS = $(AM_CPPFLAGS) @GLIB_CFLAGS@ @VOLUME_ID_CFLAGS@
+hald_probe_storage_CPPFLAGS = $(AM_CPPFLAGS) @GLIB_CFLAGS@ @BLKID_CFLAGS@
 hald_probe_storage_LDADD = \
  @GLIB_LIBS@ \
- @VOLUME_ID_LIBS@ \
+ @BLKID_LIBS@ \
  $(top_builddir)/hald/freebsd/libprobe/libhald_freebsd_probe.la
 
 hald_probe_volume_SOURCES = freebsd_dvd_rw_utils.c freebsd_dvd_rw_utils.h probe-volume.c
-hald_probe_volume_CPPFLAGS = $(AM_CPPFLAGS) @GLIB_CFLAGS@ @VOLUME_ID_CFLAGS@
+hald_probe_volume_CPPFLAGS = $(AM_CPPFLAGS) @GLIB_CFLAGS@ @BLKID_CFLAGS@
 hald_probe_volume_LDADD = \
  @GLIB_LIBS@ \
- @VOLUME_ID_LIBS@ \
+ @BLKID_LIBS@ \
  $(top_builddir)/hald/freebsd/libprobe/libhald_freebsd_probe.la
diff --git a/hald/freebsd/probing/probe-storage.c b/hald/freebsd/probing/probe-storage.c
index b9498c4..a1737e7 100644
--- a/hald/freebsd/probing/probe-storage.c
+++ b/hald/freebsd/probing/probe-storage.c
@@ -33,7 +33,7 @@
 #include <sys/types.h>
 #include <netinet/in.h>
 #include <glib.h>
-#include <libvolume_id.h>
+#include <blkid.h>
 
 #include "libhal/libhal.h"
 
@@ -217,16 +217,38 @@ main (int argc, char **argv)
     }
   else if (! has_children) /* by definition, if it has children it has no fs */
     {
-      struct volume_id *vid;
+      blkid_probe pr;
+      int fd;
 
-      vid = volume_id_open_node(device_file);
-      if (! vid)
+      fd = open(device_file, O_RDONLY);
+      if (fd < 0)
  goto end;
 
-      if (volume_id_probe_all(vid, 0, 0) == 0 && vid->usage_id == VOLUME_ID_FILESYSTEM)
- ret = 2; /* has a filesystem */
-
-      volume_id_close(vid);
+      pr = blkid_new_probe ();
+      if (pr != NULL)
+        {
+          blkid_probe_set_request (pr, BLKID_PROBREQ_LABEL | BLKID_PROBREQ_UUID |
+                                       BLKID_PROBREQ_TYPE | BLKID_PROBREQ_SECTYPE |
+                                       BLKID_PROBREQ_USAGE | BLKID_PROBREQ_VERSION);
+          if (blkid_probe_set_device (pr, fd, 0, 0) == 0 &&
+              blkid_do_safeprobe (pr) == 0)
+            {
+              const char *usage;
+
+              /* signal to hald that we've found something and a fakevolume
+               * should be added - see hald/linux/blockdev.c:add_blockdev_probing_helper_done()
+               * and hald/linux/blockdev.c:block_rescan_storage_done().
+               */
+              if (blkid_probe_lookup_value (pr, "USAGE", &usage, NULL) == 0 &&
+                  (strcmp (usage, "filesystem") == 0 ||
+                   strcmp (usage, "raid") == 0 ||
+                   strcmp (usage, "other") == 0 ||
+                   strcmp (usage, "crypto") == 0))
+                ret = 2;
+            }
+          blkid_free_probe (pr);
+        }
+      close (fd);
     }
 
  end:
diff --git a/hald/freebsd/probing/probe-volume.c b/hald/freebsd/probing/probe-volume.c
index 7dce4ce..b057447 100644
--- a/hald/freebsd/probing/probe-volume.c
+++ b/hald/freebsd/probing/probe-volume.c
@@ -39,7 +39,7 @@
 #include <sys/types.h>
 #include <isofs/cd9660/iso.h>
 #include <glib.h>
-#include <libvolume_id.h>
+#include <blkid.h>
 
 #include "libhal/libhal.h"
 
@@ -98,19 +98,20 @@ hf_probe_volume_getenv_int (const char *name)
 }
 
 static char *
-hf_probe_volume_get_label (const struct volume_id *vid)
+hf_probe_volume_get_label (const blkid_probe pr)
 {
-  char *label = NULL;
+  const char *label;
+  char *volume_label = NULL;
 
-  if (vid && *vid->label)
+  if (blkid_probe_lookup_value(pr, "LABEL", &label, NULL) == 0)
     {
-      if (g_utf8_validate(vid->label, -1, NULL))
- label = g_strdup(vid->label);
+      if (g_utf8_validate(label, -1, NULL))
+        volume_label = g_strdup(label);
       else /* assume ISO8859-1 */
- label = g_convert(vid->label, -1, "UTF-8", "ISO8859-1", NULL, NULL, NULL);
+        volume_label = g_convert(label, -1, "UTF-8", "ISO8859-1", NULL, NULL, NULL);
     }
 
-  return label;
+  return volume_label;
 }
 
 static void
@@ -316,7 +317,7 @@ main (int argc, char **argv)
   char *grandparent_udi;
   char *parent_drive_type;
   int fd = -1;
-  struct volume_id *vid = NULL;
+  blkid_probe pr;
   int ret = 1;
   gboolean has_children;
   gboolean is_swap;
@@ -326,7 +327,10 @@ main (int argc, char **argv)
   gboolean has_data = FALSE;
   gboolean is_blank = FALSE;
   const char *usage;
-  char *label;
+  const char *type;
+  const char *type_version;
+  const char *label;
+  const char *uuid;
   unsigned int sector_size = 0;
   off_t media_size = 0;
 
@@ -379,21 +383,28 @@ main (int argc, char **argv)
    */
   if (! has_children && ! (is_cdrom && ! has_data))
     {
-      vid = volume_id_open_fd(fd);
-      if (vid)
- {
-  if (volume_id_probe_all(vid, 0, media_size) == 0)
-    has_data = TRUE;
-  else
-    {
-      volume_id_close(vid);
-      vid = NULL;
-    }
- }
+      pr = blkid_new_probe ();
+      if (pr != NULL)
+        {
+          int bid_ret;
+
+          blkid_probe_set_request (pr, BLKID_PROBREQ_LABEL | BLKID_PROBREQ_UUID |
+                                       BLKID_PROBREQ_TYPE | BLKID_PROBREQ_SECTYPE |
+                                       BLKID_PROBREQ_USAGE | BLKID_PROBREQ_VERSION);
+
+          bid_ret = blkid_probe_set_device (pr, fd, 0, media_size);
+          if (bid_ret == 0)
+            {
+              bid_ret = blkid_do_safeprobe (pr);
+              hfp_warning ("blkid_do_safeprobe returned an error");
+            }
+          if (bid_ret == 0)
+            has_data = TRUE;
+        }
     }
 
   if (! has_children && ! is_swap && ! has_audio && ! has_data && ! is_blank)
-    goto end;
+    goto end_free;
 
   libhal_device_add_capability(hfp_ctx, hfp_udi, "volume", &hfp_error);
   if (is_cdrom)
@@ -559,28 +570,24 @@ main (int argc, char **argv)
     usage = "partitiontable";
   else if (is_swap)
     usage = "other";
-  else
-    switch (vid ? vid->usage_id : (enum volume_id_usage) -1)
-      {
-      case VOLUME_ID_FILESYSTEM: usage = "filesystem"; break;
-      case VOLUME_ID_DISKLABEL: usage = "disklabel"; break;
-      case VOLUME_ID_OTHER: usage = "other"; break;
-      case VOLUME_ID_RAID: usage = "raid"; break;
-      case VOLUME_ID_CRYPTO: usage = "crypto"; break;
-      case VOLUME_ID_UNUSED: usage = "unused"; break;
-      default: usage = "unknown"; break;
-      }
-
+  else if (blkid_probe_lookup_value(pr, "USAGE", &usage, NULL))
+    usage = "";
   libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.fsusage", usage, &hfp_error);
-  libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.fstype", vid ? vid->type: "", &hfp_error);
-  if (vid && *vid->type_version)
-    libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.fsversion", vid->type_version, &hfp_error);
 
-  label = hf_probe_volume_get_label(vid);
+  if (blkid_probe_lookup_value(pr, "TYPE", &type, NULL))
+    type = "";
+  libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.fstype", type, &hfp_error);
+  if (blkid_probe_lookup_value(pr, "VERSION", &type, NULL))
+    type_version = "";
+  libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.fsversion", type_version, &hfp_error);
+
+  label = hf_probe_volume_get_label(pr);
   libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.label", label ? label : "", &hfp_error);
   g_free(label);
 
-  libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.uuid", vid ? vid->uuid : "", &hfp_error);
+  if (blkid_probe_lookup_value(pr, "UUID", &uuid, NULL))
+    uuid = "";
+  libhal_device_set_property_string(hfp_ctx, hfp_udi, "volume.uuid", uuid, &hfp_error);
 
   ioctl(fd, DIOCGSECTORSIZE, §or_size);
 
@@ -593,6 +600,9 @@ main (int argc, char **argv)
 
   ret = 0; /* is a volume */
 
+ end_free:
+  blkid_free_probe (pr);
+
  end:
   return ret;
 }
--
1.6.1.3


--
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@...                 http://www.aurel32.net
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[marcus-11]

On Mon, 2009-08-17 at 23:34 +0200, Aurelien Jarno wrote:
> This might be a bit controversial given that libblkid is provided by
> util-linux, but on the other hand libvolume-id was provided by udev...

I don't mind of the other patches assuming they work on native FreeBSD,
but this one can be problematic.  Is a port of libblkid readily
available for FreeBSD?  I'd rather not see something committed upstream
that FreeBSD cannot use.  Alternatively, can you add conditional support
for both libraries?

Joe

--
Joe Marcus Clarke
FreeBSD GNOME Team      ::      gnome@...
FreeNode / #freebsd-gnome
http://www.FreeBSD.org/gnome


_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Aurelien Jarno]

On Mon, Aug 17, 2009 at 06:36:51PM -0400, Joe Marcus Clarke wrote:
> On Mon, 2009-08-17 at 23:34 +0200, Aurelien Jarno wrote:
> > This might be a bit controversial given that libblkid is provided by
> > util-linux, but on the other hand libvolume-id was provided by udev...
>
> I don't mind of the other patches assuming they work on native FreeBSD,

The other patches work on native FreeBSD, one being essential (the
current git doesn't compile without it).

> but this one can be problematic.  Is a port of libblkid readily
> available for FreeBSD?  I'd rather not see something committed upstream
> that FreeBSD cannot use.  Alternatively, can you add conditional support
> for both libraries?

libblkid already exists on FreeBSD (provided by e2fsprogs), but does not
exists in a sufficient version (that is a version provided by
util-linux). Anyway the current version of git (as well as version 0.5.13)
already does not build on FreeBSD, as the configure script already checks
for libblkid >= 2.15.

I guess the best is to make a port of util-linux for FreeBSD. Despite
his name this code intents to be portable on other OS (except for really
Linux specific code), and patches are usually accepted to fix linuxisms.
It should be possible to build only libuuid and libblkid using a long list
of --disable- options, maybe adding a --enable-libs-only option would
help there.

--
Aurelien Jarno                        GPG: 1024D/F1BCDB73
aurelien@...                 http://www.aurel32.net
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[marcus-11]

On Tue, 2009-08-18 at 14:22 +0200, Aurelien Jarno wrote:
> On Mon, Aug 17, 2009 at 06:36:51PM -0400, Joe Marcus Clarke wrote:
> > On Mon, 2009-08-17 at 23:34 +0200, Aurelien Jarno wrote:
> > > This might be a bit controversial given that libblkid is provided by
> > > util-linux, but on the other hand libvolume-id was provided by udev...
> >
> > I don't mind of the other patches assuming they work on native FreeBSD,
>
> The other patches work on native FreeBSD, one being essential (the
> current git doesn't compile without it).

I have quite a few patches in the local FreeBSD port that I need to push
upstream.

>
> > but this one can be problematic.  Is a port of libblkid readily
> > available for FreeBSD?  I'd rather not see something committed upstream
> > that FreeBSD cannot use.  Alternatively, can you add conditional support
> > for both libraries?
>
> libblkid already exists on FreeBSD (provided by e2fsprogs), but does not
> exists in a sufficient version (that is a version provided by
> util-linux). Anyway the current version of git (as well as version 0.5.13)
> already does not build on FreeBSD, as the configure script already checks
> for libblkid >= 2.15.

I've since fixed this in our hal port at
http://www.marcuscom.com:8080/cgi-bin/cvsweb.cgi/ports .

>
> I guess the best is to make a port of util-linux for FreeBSD. Despite
> his name this code intents to be portable on other OS (except for really
> Linux specific code), and patches are usually accepted to fix linuxisms.
> It should be possible to build only libuuid and libblkid using a long list
> of --disable- options, maybe adding a --enable-libs-only option would
> help there.

What are the advantages of libblkid over libvolume_id?  Are there any
benefits to the latter on FreeBSD given that the most dominant file
systems returned are ufs, vfat, iso9660, and udf which libvolume_id
already supports?

Joe

>
--
Joe Marcus Clarke
FreeBSD GNOME Team      ::      gnome@...
FreeNode / #freebsd-gnome
http://www.FreeBSD.org/gnome


_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Aurelien Jarno]

Joe Marcus Clarke a écrit :

> On Tue, 2009-08-18 at 14:22 +0200, Aurelien Jarno wrote:
>> On Mon, Aug 17, 2009 at 06:36:51PM -0400, Joe Marcus Clarke wrote:
>>> On Mon, 2009-08-17 at 23:34 +0200, Aurelien Jarno wrote:
>>>> This might be a bit controversial given that libblkid is provided by
>>>> util-linux, but on the other hand libvolume-id was provided by udev...
>>> I don't mind of the other patches assuming they work on native FreeBSD,
>> The other patches work on native FreeBSD, one being essential (the
>> current git doesn't compile without it).
>
> I have quite a few patches in the local FreeBSD port that I need to push
> upstream.
>
>>> but this one can be problematic.  Is a port of libblkid readily
>>> available for FreeBSD?  I'd rather not see something committed upstream
>>> that FreeBSD cannot use.  Alternatively, can you add conditional support
>>> for both libraries?
>> libblkid already exists on FreeBSD (provided by e2fsprogs), but does not
>> exists in a sufficient version (that is a version provided by
>> util-linux). Anyway the current version of git (as well as version 0.5.13)
>> already does not build on FreeBSD, as the configure script already checks
>> for libblkid >= 2.15.
>
> I've since fixed this in our hal port at
> http://www.marcuscom.com:8080/cgi-bin/cvsweb.cgi/ports .

Could you please send those patches, in order to avoid duplicated work
and/or conflicts?

>> I guess the best is to make a port of util-linux for FreeBSD. Despite
>> his name this code intents to be portable on other OS (except for really
>> Linux specific code), and patches are usually accepted to fix linuxisms.
>> It should be possible to build only libuuid and libblkid using a long list
>> of --disable- options, maybe adding a --enable-libs-only option would
>> help there.
>
> What are the advantages of libblkid over libvolume_id?  Are there any
> benefits to the latter on FreeBSD given that the most dominant file
> systems returned are ufs, vfat, iso9660, and udf which libvolume_id
> already supports?
>

libvolume_id on FreeBSD is basically a library extracted from udev, and
udev >= 145 is not shipping libvolume_id anymore, it uses libblkid
instead. Also libvolume_id is licensed under GPL while libblkid is LGPL.

I guess that in the meanwhile the best is to support both libvolume_id
and libblkid in hal (I'll work on a patch), but FreeBSD should think
about switching to libblkid in the long term.

--
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@...                 http://www.aurel32.net
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[marcus-11]

On Tue, 2009-08-18 at 15:18 +0200, Aurelien Jarno wrote:

> Joe Marcus Clarke a écrit :
> > On Tue, 2009-08-18 at 14:22 +0200, Aurelien Jarno wrote:
> >> On Mon, Aug 17, 2009 at 06:36:51PM -0400, Joe Marcus Clarke wrote:
> >>> On Mon, 2009-08-17 at 23:34 +0200, Aurelien Jarno wrote:
> >>>> This might be a bit controversial given that libblkid is provided by
> >>>> util-linux, but on the other hand libvolume-id was provided by udev...
> >>> I don't mind of the other patches assuming they work on native FreeBSD,
> >> The other patches work on native FreeBSD, one being essential (the
> >> current git doesn't compile without it).
> >
> > I have quite a few patches in the local FreeBSD port that I need to push
> > upstream.
> >
> >>> but this one can be problematic.  Is a port of libblkid readily
> >>> available for FreeBSD?  I'd rather not see something committed upstream
> >>> that FreeBSD cannot use.  Alternatively, can you add conditional support
> >>> for both libraries?
> >> libblkid already exists on FreeBSD (provided by e2fsprogs), but does not
> >> exists in a sufficient version (that is a version provided by
> >> util-linux). Anyway the current version of git (as well as version 0.5.13)
> >> already does not build on FreeBSD, as the configure script already checks
> >> for libblkid >= 2.15.
> >
> > I've since fixed this in our hal port at
> > http://www.marcuscom.com:8080/cgi-bin/cvsweb.cgi/ports .
>
> Could you please send those patches, in order to avoid duplicated work
> and/or conflicts?

I've just pushed almost all of our local patches into git.

>
> >> I guess the best is to make a port of util-linux for FreeBSD. Despite
> >> his name this code intents to be portable on other OS (except for really
> >> Linux specific code), and patches are usually accepted to fix linuxisms.
> >> It should be possible to build only libuuid and libblkid using a long list
> >> of --disable- options, maybe adding a --enable-libs-only option would
> >> help there.
> >
> > What are the advantages of libblkid over libvolume_id?  Are there any
> > benefits to the latter on FreeBSD given that the most dominant file
> > systems returned are ufs, vfat, iso9660, and udf which libvolume_id
> > already supports?
> >
>
> libvolume_id on FreeBSD is basically a library extracted from udev, and
> udev >= 145 is not shipping libvolume_id anymore, it uses libblkid
> instead. Also libvolume_id is licensed under GPL while libblkid is LGPL.
>
> I guess that in the meanwhile the best is to support both libvolume_id
> and libblkid in hal (I'll work on a patch), but FreeBSD should think
> about switching to libblkid in the long term.

Thanks.

Joe

>
--
Joe Marcus Clarke
FreeBSD GNOME Team      ::      gnome@...
FreeNode / #freebsd-gnome
http://www.FreeBSD.org/gnome


_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal