[Libcdio-devel] [Patch] Umount drive before ejecting

[R. Bernstein]

Overall patch looks fine. Thanks. 

Other comments in line.

address@hidden writes:
 > Hi,
 > 
 > I made a patch, which does the following:
 > 
 > - Before ejecting, we test if the device is mounted. This is done by
 >   reading /proc/mounts (or /etc/mtab as fallback) with a routine
 >   inspired by libunieject
 > 
 > - If the drive is mounted, we try to umount it. This is done by forking and
 >   exec'ing pumount (umount as fallback). My attempts to use the umount()
 >   system call failed, since it triggers an EPERM error if we aren't root,
 >   even if we would be allowed to umount the disc (the umount command
 >   is setuid root and seems to check for proper permissions itself).
 >   The umount code was inspired by the linux eject command.
 > 
 > - After umounting, the filedescriptor is closed and opened again. This
 >   is needed for eject to work (also tried sync() and sleep(2), without
 >   success).
 > 
 > - Ejecting then happens with the ioctl or mmc command like before.
 > 
 > - At the end, we close() the filedescriptor (before it was just set to
 >   -1) but only, if is wasn't open before the call to eject_media_linux().
 > 
 > - Also attached an ultra lightweight program cdio-eject.c I use for
 >   testing. It's modelled after the linux eject command, but only
 >   the -t option (close tray) is supported. I can put this into the
 >   libcdio tree as well, if there is interest.

Of course there is interest!  One place this could go is in the
examples directory. Another possibility, not mutually exclusive, is
using this as part of the regression tests. One would first test to
see if there is ejectable medium in a drive somewhere and if not skip
the test. And if the regression test is run afterward, I suppose the
tray door should be closed again.

I don't think it should be installed like cd-info or cd-drive, because
there's unieject which is probably a more complete solution. 

 > 
 > My remaining questions are:
 > 
 > 1. Are any objections against using fork() inside libcdio? I doubt that
 >    umounting a drive from a userspace app can be done in a better way.

Since we're talking GNU/Linux right now I don't see a problem.

 > 
 > 2. The solution is linux only. I think libcdio should behave similar on
 >    all supported platforms with respect to eject/umount. Maybe my
 >    mechanism can be ported to other systems as well, but I don't know.

It was my belief that libunieject has more of these smarts. I trust
his judgment.

 > 
 > If these questions are clarified, I'll apply it.
 > 
 > Cheers
 > 
 > Burkhard
 > 
 > ? cdio_eject.patch
 > ? out.txt
 > ? example/udffile
 > Index: lib/driver/gnu_linux.c
 > ===================================================================
 > RCS file: /sources/libcdio/libcdio/lib/driver/gnu_linux.c,v
 > retrieving revision 1.24
 > diff -u -r1.24 gnu_linux.c
 > --- lib/driver/gnu_linux.c   2 Aug 2006 11:00:31 -0000       1.24
 > +++ lib/driver/gnu_linux.c   20 Oct 2006 22:02:59 -0000
 > @@ -31,6 +31,9 @@
 >  static const char _rcsid[] = "$Id: gnu_linux.c,v 1.24 2006/08/02 11:00:31 
 > rocky Exp $";
 >  
 >  #include <string.h>
 > +#include <limits.h>
 > +#include <sys/types.h>
 > +#include <sys/wait.h>
 >  
 >  #include <cdio/sector.h>
 >  #include <cdio/util.h>
 > @@ -593,6 +596,107 @@
 >  }
 >  
 >  /*!
 > +  Follow symlinks until we have the real device file
 > +  (idea taken from libunieject). 
 > +*/
 > +
 > +static void follow_symlink (const char * src, char * dst) {
 > +  char tmp_src[PATH_MAX];
 > +  char tmp_dst[PATH_MAX];
 > +  
 > +  int len;
 > +
 > +  strcpy(tmp_src, src);
 > +  while(1) {
 > +    len = readlink(tmp_src, tmp_dst, PATH_MAX);
 > +    if(len < 0) {
 > +      strcpy(dst, tmp_src);
 > +      return;
 > +    }
 > +    else {
 > +      tmp_dst[len] = '\0';
 > +      strcpy(tmp_src, tmp_dst);
 > +    }
 > +  }
 > +}
 > +
 > +/*!
 > +  Check, if a device is mounted and return the target (=mountpoint)
 > +  needed for umounting (idea taken from libunieject).
 > + */
 > +
 > +static int is_mounted (const char * device, char * target) {
 > +  FILE * fp;
 > +  char real_device_1[PATH_MAX];
 > +  char real_device_2[PATH_MAX];
 > +
 > +  char file_device[PATH_MAX];
 > +  char file_target[PATH_MAX];
 > +  
 > +  fp = fopen ( "/proc/mounts", "r");
 > +  /* Older systems just have /etc/mtab */
 > +  if(!fp)
 > +    fp = fopen ( "/etc/mtab", "r");
 > +
 > +  /* Neither /proc/mounts nor /etc/mtab could be opened, give up here */
 > +  if(!fp) return 0;
 > +
 > +  /* Get real device */
 > +  follow_symlink(device, real_device_1);
 > +    
 > +  /* Read entries */
 > +
 > +  while ( fscanf(fp, "%s %s %*s %*s %*d %*d\n", file_device, file_target) 
 > != EOF ) {
 > +    follow_symlink(file_device, real_device_2);
 > +    if(!strcmp(real_device_1, real_device_2)) {
 > +      strcpy(target, file_target);
 > +      fclose(fp);
 > +      return 1;
 > +    }
 > +      
 > +  }
 > +  fclose(fp);
 > +  return 0;
 > +}
 > +
 > +/*!
 > +  Umount a filesystem specified by it's mountpoint. We must do this
 > +  by forking and calling the umount command, because the raw umount
 > +  (or umount2) system calls will *always* trigger an EPERM even if 
 > +  we are allowed to umount the filesystem. The umount command is 
 > +  suid root.
 > +
 > +  Code here is inspired by the standard linux eject command by
 > +  Jeff Tranter and Frank Lichtenheld.
 > + */
 > +
 > +static int do_umount(char * target) {
 > +  int status;
 > +
 > +  switch (fork()) {
 > +  case 0: /* child */
 > +    execlp("pumount", "pumount", target, NULL);
 > +    execlp("umount", "umount", target, NULL);
 > +    return -1;
 > +    break;
 > +  case -1:
 > +    return -1;
 > +    break;
 > +  default: /* parent */
 > +    wait(&status);
 > +    if (WIFEXITED(status) == 0) {
 > +      return -1;
 > +    }
 > +    if (WEXITSTATUS(status) != 0) {
 > +      return -1;
 > +    }
 > +    break;
 > +  }
 > +  return 0;
 > +}
 > +     
 > +
 > +/*!
 >    Eject media in CD-ROM drive. Return DRIVER_OP_SUCCESS if successful, 
 >    DRIVER_OP_ERROR on error.
 >   */
 > @@ -603,10 +707,15 @@
 >    _img_private_t *p_env = p_user_data;
 >    driver_return_code_t ret=DRIVER_OP_SUCCESS;
 >    int status;
 > -
 > +  int was_open = 0;
 > +  char mount_target[PATH_MAX];
 > +  
 >    if ( p_env->gen.fd <= -1 ) {
 >      p_env->gen.fd = open (p_env->gen.source_name, O_RDONLY|O_NONBLOCK);
 >    }
 > +  else {
 > +    was_open = 1;
 > +  }
 >    
 >    if ( p_env->gen.fd <= -1 ) return DRIVER_OP_ERROR;
 >  
 > @@ -622,6 +731,23 @@
 >        cdio_info ("Unknown state of CD-ROM (%d)\n", status);
 >        /* Fall through */
 >      case CDS_DISC_OK:
 > +      /* Some systems automount the drive, so we must umount it.
 > +         We check if the drive is actually mounted */
 > +      if(is_mounted (p_env->gen.source_name, mount_target)) {
 > +        /* Try to umount the drive */
 > +        if(do_umount(mount_target)) {
 > +          cdio_log(CDIO_LOG_WARN, "Could not umount %s\n",
 > +                   p_env->gen.source_name);
 > +          ret=DRIVER_OP_ERROR;
 > +          break;
 > +        }
 > +        /* For some reason, we must close and reopen the device after
 > +           it got umounted (at least the commandline eject program
 > +           opens the device just after umounting it) */
 > +        close(p_env->gen.fd);
 > +        p_env->gen.fd = open (p_env->gen.source_name, O_RDONLY|O_NONBLOCK);
 > +      }
 > +      
 >        if((ret = ioctl(p_env->gen.fd, CDROMEJECT)) != 0) {
 >          int eject_error = errno;
 >          /* Try ejecting the MMC way... */
 > @@ -642,7 +768,10 @@
 >      cdio_warn ("CDROM_DRIVE_STATUS failed: %s\n", strerror(errno));
 >      ret=DRIVER_OP_ERROR;
 >    }
 > -  p_env->gen.fd = -1;
 > +  if(!was_open) {
 > +    close(p_env->gen.fd);
 > +    p_env->gen.fd = -1;
 > +  }
 >    return ret;
 >  }
 >  
 > #include <stdio.h>
 > #include <string.h>
 > #include <cdio/cdio.h>
 > 
 > static void usage(char * progname)
 >   {
 >   fprintf(stderr, "Usage: %s [-t] <device>\n", progname);
 >   }
 > 
 > int main(int argc, char ** argv)
 >   {
 >   driver_return_code_t err;
 >   int close_tray = 0;
 >   const char * device;
 >   
 >   if(argc < 2 || argc > 3)
 >     {
 >     usage(argv[0]);
 >     return -1;
 >     }
 > 
 >   if((argc == 3) && strcmp(argv[1], "-t"))
 >     {
 >     usage(argv[0]);
 >     return -1;
 >     }
 > 
 >   if(argc == 2)
 >     device = argv[1];
 >   else if(argc == 3)
 >     {
 >     close_tray = 1;
 >     device = argv[2];
 >     }
 > 
 >   if(close_tray)
 >     {
 >     err = cdio_close_tray(device, NULL);
 >     if(err)
 >       {
 >       fprintf(stderr, "Closing tray failed for device %s: %s\n",
 >               device, cdio_driver_errmsg(err));
 >       return -1;
 >       }
 >     }
 >   else
 >     {
 >     err = cdio_eject_media_drive(device);
 >     if(err)
 >       {
 >       fprintf(stderr, "Ejecting failed for device %s: %s\n",
 >               device, cdio_driver_errmsg(err));
 >       return -1;
 >       }
 >     }
 > 
 >   return 0;
 >   
 >   }
 > 
 > _______________________________________________
 > Libcdio-devel mailing list
 > address@hidden
 > http://lists.gnu.org/mailman/listinfo/libcdio-devel