On Thu, Feb 07, 2013 at 03:57:52PM +0000, Matthew Booth wrote:
These 2 internal functions allow mounting and unmounting of
mountables
outside /sysroot.
There seems to be some variable movement in this patch which confuses
things. Variable placement should be left alone in unrelated patches.
daemon/daemon.h | 8 +++++
daemon/mount.c | 103 +++++++++++++++++++++++++++++++++++++++-----------------
2 files changed, 81 insertions(+), 30 deletions(-)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index a94c338..78591a0 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -65,6 +65,14 @@ extern int xread (int sock, void *buf, size_t len)
extern char *mountable_to_string (const mountable_t *mountable);
+/*-- in mount.c --*/
+
+extern int mount_vfs_internal (const char *options, const char *vfstype,
+ const mountable_t *mountable,
+ const char *mp, const char *user_mp, char **err);
+extern int umount_internal (const char *pathordevice,
+ int force, int lazyunmount, char **err);
+
/* Growable strings buffer. */
struct stringsbuf {
char **argv;
diff --git a/daemon/mount.c b/daemon/mount.c
index 484e45c..34510bd 100644
--- a/daemon/mount.c
+++ b/daemon/mount.c
@@ -126,31 +126,44 @@ int
do_mount_vfs (const char *options, const char *vfstype,
const mountable_t *mountable, const char *mountpoint)
{
- int r;
- CLEANUP_FREE char *mp = NULL;
- CLEANUP_FREE char *error = NULL;
- struct stat statbuf;
-
ABS_PATH (mountpoint, , return -1);
- mp = sysroot_path (mountpoint);
+ CLEANUP_FREE char *mp = sysroot_path (mountpoint);
if (!mp) {
reply_with_perror ("malloc");
return -1;
}
/* Check the mountpoint exists and is a directory. */
+ struct stat statbuf;
if (stat (mp, &statbuf) == -1) {
reply_with_perror ("mount: %s", mountpoint);
return -1;
}
+
if (!S_ISDIR (statbuf.st_mode)) {
reply_with_perror ("mount: %s: mount point is not a directory",
mountpoint);
return -1;
}
+ CLEANUP_FREE char *err = NULL;
+ int r = mount_vfs_internal (options, vfstype, mountable,
+ mp, mountpoint, &err);
+
+ if (r == -1) {
+ reply_with_error ("%s", err ? err : "malloc");
+ }
+
+ return r;
+}
+
+int
+mount_vfs_internal (const char *options, const char *vfstype,
+ const mountable_t *mountable,
+ const char *mp, const char *user_mp,
+ char **err)
+{
CLEANUP_FREE char *options_plus = NULL;
- const char *device = mountable->device;
switch (mountable->type) {
case MOUNTABLE_DEVICE:
break;
@@ -160,24 +173,34 @@ do_mount_vfs (const char *options, const char *vfstype,
if (asprintf (&options_plus, "subvol=%s,%s",
mountable->volume, options) == -1)
{
- reply_with_perror ("asprintf");
+ /* No point trying to allocate more memory for an error message */
+ fprintf (stderr, "asprintf: %m\n");
return -1;
This may or may not be a good thing, but it shouldn't be
a part of this patch.
On a separate point, just because asprintf fails doesn't mean that
reply_with_perror would fail. asprintf might have failed because
'options' was somehow an unterminated string (ie. it requested a huge
amount of memory), not because there's no memory left.
}
}
else {
if (asprintf (&options_plus, "subvol=%s", mountable->volume) ==
-1) {
- reply_with_perror ("asprintf");
+ /* No point trying to allocate more memory for an error message */
+ fprintf (stderr, "asprintf: %m\n");
return -1;
Ditto.
}
break;
default:
- reply_with_error ("Invalid mountable type: %i", mountable->type);
+ if (asprintf (err, "Invalid mountable type: %i", mountable->type) ==
-1)
+ {
+ /* No point trying to allocate more memory for an error message */
+ fprintf (stderr, "Invalid mountable type: %i", mountable->type);
+ fprintf (stderr, "asprintf: %m\n");
+ }
return -1;
}
Ditto.
I didn't review further. I think this patch needs reworking.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top