Libguestfs - Aug 2009 - do_umount adjustment

Here's one final (I hope) question without a complete patch.
Given that I already ensure that stubs.c invokes
REQUIRE_ROOT_OR_RESOLVE_DEVICE just before calling do_umount,
does this new version of do_umount look ok?

/* Again, use the external /bin/umount program, so that /etc/mtab
 * is kept updated.
 */
int
do_umount (const char *pathordevice)
{
  int r;
  char *err;

  char *buf = sysroot_path (pathordevice);
  if (buf == NULL) {
    reply_with_perror ("malloc");
    return -1;
  }

  r = command (NULL, &err, "umount", buf, NULL);
  if (r == -1) {
    reply_with_error ("umount: %s: %s", pathordevice, err);
    free (err);
    free (buf);
    return -1;
  }

  free (err);

  /* update root_mounted? */

  return 0;
}

On Wed, Aug 12, 2009 at 10:14:51AM +0200, Jim Meyering
wrote:
> Here's one final (I hope) question without a complete patch.
> Given that I already ensure that stubs.c invokes
> REQUIRE_ROOT_OR_RESOLVE_DEVICE just before calling do_umount,
> does this new version of do_umount look ok?
> 
> /* Again, use the external /bin/umount program, so that /etc/mtab
>  * is kept updated.
>  */
> int
> do_umount (const char *pathordevice)
> {
>   int r;
>   char *err;
> 
>   char *buf = sysroot_path (pathordevice);
>   if (buf == NULL) {
>     reply_with_perror ("malloc");
>     return -1;
>   }
I think the problem here is you'll end up with a buf like
'/sysroot/dev/vda'.

In the original code:

  if (strncmp (pathordevice, "/dev/", 5) == 0) {
    buf = pathordevice;
    IS_DEVICE (buf, -1);
  } else {
    buf = sysroot_path (pathordevice);
    if (buf == NULL) {
      reply_with_perror ("malloc");
      return -1;
    }
    freeit = 1;
  }

it avoids prefixing /dev paths.

If Pathname args turn into two parameters (the original path, and the
sysroot-prefixed path), then maybe Pathname_or_device args should turn
into several parameters too, like:

  int is_device, const char *original_pathordevice, const char
*sysroot_pathordevice

(That's just an idea ...  maybe not a good one)

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top