mirrors/libguestfs
1
0
Fork 0
mirror of https://github.com/libguestfs/libguestfs.git synced 2026-03-21 22:53:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix upload losing synchronization if root not mounted (RHBZ#576879).

Browse Source 
Modify the generator so that it can correctly handle early
cancellation for Pathname|Device|.. parameters.  This fixes
the upload command, but consequently we need to fix the
parameters for tar_in and t?z_in commands.  This should also
mean that 'win:' can now be used as the second argument of
tar_in and t?z_in commands in guestfish, whereas previously
this wouldn't have worked.

Adds a regression test for the original problem.
This commit is contained in:
Richard Jones
2010-04-17 14:25:55 +01:00
parent 68467cb1c2
commit 5922d7084d
11 changed files with 92 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
daemon/command.c
View File

@@ -36,7 +36,7 @@ do_command (char *const *argv)
int dev_ok, dev_pts_ok, proc_ok, selinux_ok, sys_ok;
/* We need a root filesystem mounted to do this. */
NEED_ROOT (return NULL);
NEED_ROOT (, return NULL);
/* Conveniently, argv is already a NULL-terminated argv-style array
* of parameters, so we can pass it straight in to our internal

26
daemon/daemon.h
View File

@@ -68,7 +68,7 @@ extern char **split_lines (char *str);
extern void trim (char *str);
extern int device_name_translation (char *device, const char *func);
extern int device_name_translation (char *device);
extern int prog_exists (const char *prog);
@@ -155,9 +155,10 @@ extern void reply (xdrproc_t xdrp, char *ret);
/* Helper for functions that need a root filesystem mounted.
* NB. Cannot be used for FileIn functions.
*/
#define NEED_ROOT(fail_stmt) \
#define NEED_ROOT(cancel_stmt,fail_stmt) \
do { \
if (!root_mounted) { \
cancel_stmt; \
reply_with_error ("%s: you must call 'mount' first to mount the root filesystem", __func__); \
fail_stmt; \
} \
@@ -167,9 +168,10 @@ extern void reply (xdrproc_t xdrp, char *ret);
/* Helper for functions that need an argument ("path") that is absolute.
* NB. Cannot be used for FileIn functions.
*/
#define ABS_PATH(path,fail_stmt) \
#define ABS_PATH(path,cancel_stmt,fail_stmt) \
do { \
if ((path)[0] != '/') { \
cancel_stmt; \
reply_with_error ("%s: path must start with a / character", __func__); \
fail_stmt; \
} \
@@ -182,14 +184,20 @@ extern void reply (xdrproc_t xdrp, char *ret);
*
* NB. Cannot be used for FileIn functions.
*/
#define RESOLVE_DEVICE(path,fail_stmt) \
#define RESOLVE_DEVICE(path,cancel_stmt,fail_stmt) \
do { \
if (STRNEQLEN ((path), "/dev/", 5)) { \
cancel_stmt; \
reply_with_error ("%s: %s: expecting a device name", __func__, (path)); \
fail_stmt; \
} \
if (device_name_translation ((path), __func__) == -1) \
if (device_name_translation ((path)) == -1) { \
int err = errno; \
cancel_stmt; \
errno = err; \
reply_with_perror ("%s: %s", __func__, path); \
fail_stmt; \
} \
} while (0)
/* Helper for functions which need either an absolute path in the
@@ -202,13 +210,13 @@ extern void reply (xdrproc_t xdrp, char *ret);
* because we intend in future to make device parameters a distinct
* type from filenames.
*/
#define REQUIRE_ROOT_OR_RESOLVE_DEVICE(path,fail_stmt) \
#define REQUIRE_ROOT_OR_RESOLVE_DEVICE(path,cancel_stmt,fail_stmt) \
do { \
if (STREQLEN ((path), "/dev/", 5)) \
RESOLVE_DEVICE ((path), fail_stmt); \
RESOLVE_DEVICE ((path), cancel_stmt, fail_stmt); \
else { \
NEED_ROOT (fail_stmt); \
ABS_PATH ((path),fail_stmt); \
NEED_ROOT (cancel_stmt, fail_stmt); \
ABS_PATH ((path), cancel_stmt, fail_stmt); \
} \
} while (0)

4
daemon/df.c
View File

@@ -33,7 +33,7 @@ do_df (void)
int r;
char *out, *err;
NEED_ROOT (return NULL);
NEED_ROOT (, return NULL);
r = command (&out, &err, "df", NULL);
if (r == -1) {
@@ -54,7 +54,7 @@ do_df_h (void)
int r;
char *out, *err;
NEED_ROOT (return NULL);
NEED_ROOT (, return NULL);
r = command (&out, &err, "df", "-h", NULL);
if (r == -1) {

24
daemon/guestfsd.c
View File

@@ -1022,42 +1022,36 @@ print_arginfo (const struct printf_info *info, size_t n, int *argtypes)
* the device nodes themselves will exist in the appliance.
*/
int
device_name_translation (char *device, const char *func)
device_name_translation (char *device)
{
int fd;
fd = open (device, O_RDONLY);
if (fd >= 0) {
close_ok:
close (fd);
return 0;
}
if (errno != ENXIO && errno != ENOENT) {
error:
reply_with_perror ("%s: %s", func, device);
if (errno != ENXIO && errno != ENOENT)
return -1;
}
/* If the name begins with "/dev/sd" then try the alternatives. */
if (STRNEQLEN (device, "/dev/sd", 7))
goto error;
return -1;
device[5] = 'h'; /* /dev/hd (old IDE driver) */
fd = open (device, O_RDONLY);
if (fd >= 0) {
close (fd);
return 0;
}
if (fd >= 0)
goto close_ok;
device[5] = 'v'; /* /dev/vd (for virtio devices) */
fd = open (device, O_RDONLY);
if (fd >= 0) {
close (fd);
return 0;
}
if (fd >= 0)
goto close_ok;
device[5] = 's'; /* Restore original device name. */
goto error;
return -1;
}
/* Check program exists and is executable on $PATH. Actually, we

2
daemon/inotify.c
View File

@@ -70,7 +70,7 @@ do_inotify_init (int max_events)
#ifdef HAVE_SYS_INOTIFY_H
FILE *fp;
NEED_ROOT (return -1);
NEED_ROOT (, return -1);
if (max_events < 0) {
reply_with_error ("max_events < 0");

8
daemon/mount.c
View File

@@ -48,7 +48,7 @@ do_mount_vfs (const char *options, const char *vfstype,
char *mp;
char *error;
ABS_PATH (mountpoint, return -1);
ABS_PATH (mountpoint, , return -1);
is_root = STREQ (mountpoint, "/");
@@ -121,7 +121,7 @@ do_umount (const char *pathordevice)
}
if (is_dev)
RESOLVE_DEVICE (buf, { free (buf); return -1; });
RESOLVE_DEVICE (buf, , { free (buf); return -1; });
r = command (NULL, &err, "umount", buf, NULL);
free (buf);
@@ -356,7 +356,7 @@ do_mkmountpoint (const char *path)
int r;
/* NEED_ROOT (return -1); - we don't want this test for this call. */
ABS_PATH (path, return -1);
ABS_PATH (path, , return -1);
CHROOT_IN;
r = mkdir (path, 0777);
@@ -381,7 +381,7 @@ do_rmmountpoint (const char *path)
int r;
/* NEED_ROOT (return -1); - we don't want this test for this call. */
ABS_PATH (path, return -1);
ABS_PATH (path, , return -1);
CHROOT_IN;
r = rmdir (path);

12
daemon/tar.c
View File

@@ -42,12 +42,6 @@ do_tar_in (const char *dir)
FILE *fp;
char *cmd;
if (!root_mounted || dir[0] != '/') {
cancel_receive ();
reply_with_error ("root must be mounted and path must be absolute");
return -1;
}
/* "tar -C /sysroot%s -xf -" but we have to quote the dir. */
if (asprintf_nowarn (&cmd, "tar -C %R -xf -", dir) == -1) {
err = errno;
@@ -165,12 +159,6 @@ do_tXz_in (const char *dir, char filter)
FILE *fp;
char *cmd;
if (!root_mounted || dir[0] != '/') {
cancel_receive ();
reply_with_error ("root must be mounted and path must be absolute");
return -1;
}
/* "tar -C /sysroot%s -zxf -" but we have to quote the dir. */
if (asprintf_nowarn (&cmd, "tar -C %R -%cxf -", dir, filter) == -1) {
err = errno;

7
daemon/upload.c
View File

@@ -41,13 +41,6 @@ do_upload (const char *filename)
int err, fd, r, is_dev;
is_dev = STRPREFIX (filename, "/dev/");
if (!is_dev) {
if (!root_mounted || filename[0] != '/') {
cancel_receive ();
reply_with_error ("root must be mounted and path must be absolute");
return -1;
}
}
if (!is_dev) CHROOT_IN;
fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0666);

1
regressions/Makefile.am
View File

@@ -27,6 +27,7 @@ TESTS = \
rhbz503169c10.sh \
rhbz503169c13.sh \
rhbz557655.sh \
rhbz576879.sh \
rhbz578407.sh \
rhbz580246.sh \
test-cancellation-download-librarycancels.sh \

33
regressions/rhbz576879.sh Executable file
View File

@@ -0,0 +1,33 @@
#!/bin/bash -
# libguestfs
# Copyright (C) 2010 Red Hat Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
# Regression test for:
# https://bugzilla.redhat.com/show_bug.cgi?id=576879
# upload loses synchronization if the disk is not mounted
set -e
rm -f test1.img
../fish/guestfish <<EOF
alloc test1.img 10M
run
-upload $srcdir/rhbz576879.sh /test.sh
# Shouldn't lose synchronization, so next command should work:
ping-daemon
EOF

37
src/generator.ml
View File

@@ -2028,7 +2028,7 @@ Compute the SHA512 hash (using the C<sha512sum> program).
The checksum is returned as a printable string.");
("tar_in", (RErr, [FileIn "tarfile"; String "directory"]), 69, [],
("tar_in", (RErr, [FileIn "tarfile"; Pathname "directory"]), 69, [],
[InitBasicFS, Always, TestOutput (
[["tar_in"; "../images/helloworld.tar"; "/"];
["cat"; "/hello"]], "hello\n")],
@@ -2050,7 +2050,7 @@ it to local file C<tarfile>.
To download a compressed tarball, use C<guestfs_tgz_out>
or C<guestfs_txz_out>.");
("tgz_in", (RErr, [FileIn "tarball"; String "directory"]), 71, [],
("tgz_in", (RErr, [FileIn "tarball"; Pathname "directory"]), 71, [],
[InitBasicFS, Always, TestOutput (
[["tgz_in"; "../images/helloworld.tar.gz"; "/"];
["cat"; "/hello"]], "hello\n")],
@@ -4382,7 +4382,7 @@ This command writes zeroes over the entire C<device>. Compare
with C<guestfs_zero> which just zeroes the first few blocks of
a device.");
("txz_in", (RErr, [FileIn "tarball"; String "directory"]), 229, [],
("txz_in", (RErr, [FileIn "tarball"; Pathname "directory"]), 229, [],
[InitBasicFS, Always, TestOutput (
[["txz_in"; "../images/helloworld.tar.xz"; "/"];
["cat"; "/hello"]], "hello\n")],
@@ -6032,14 +6032,19 @@ and generate_daemon_actions () =
);
pr "\n";
let is_filein =
List.exists (function FileIn _ -> true | _ -> false) (snd style) in
(match snd style with
| [] -> ()
| args ->
pr " memset (&args, 0, sizeof args);\n";
pr "\n";
pr " if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name;
if is_filein then
pr " cancel_receive ();\n";
pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n";
pr " return;\n";
pr " goto done;\n";
pr " }\n";
let pr_args n =
pr " char *%s = args.%s;\n" n n
@@ -6048,6 +6053,8 @@ and generate_daemon_actions () =
pr " %s = realloc (args.%s.%s_val,\n" n n n;
pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n;
pr " if (%s == NULL) {\n" n;
if is_filein then
pr " cancel_receive ();\n";
pr " reply_with_perror (\"realloc\");\n";
pr " goto done;\n";
pr " }\n";
@@ -6058,13 +6065,16 @@ and generate_daemon_actions () =
function
| Pathname n ->
pr_args n;
pr " ABS_PATH (%s, goto done);\n" n;
pr " ABS_PATH (%s, %s, goto done);\n"
n (if is_filein then "cancel_receive ()" else "");
| Device n ->
pr_args n;
pr " RESOLVE_DEVICE (%s, goto done);\n" n;
pr " RESOLVE_DEVICE (%s, %s, goto done);\n"
n (if is_filein then "cancel_receive ()" else "");
| Dev_or_Path n ->
pr_args n;
pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, goto done);\n" n;
pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n"
n (if is_filein then "cancel_receive ()" else "");
| String n -> pr_args n
| OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n
| StringList n ->
@@ -6074,7 +6084,8 @@ and generate_daemon_actions () =
pr " /* Ensure that each is a device,\n";
pr " * and perform device name translation. */\n";
pr " { int pvi; for (pvi = 0; physvols[pvi] != NULL; ++pvi)\n";
pr " RESOLVE_DEVICE (physvols[pvi], goto done);\n";
pr " RESOLVE_DEVICE (physvols[pvi], %s, goto done);\n"
(if is_filein then "cancel_receive ()" else "");
pr " }\n";
| Bool n -> pr " %s = args.%s;\n" n n
| Int n -> pr " %s = args.%s;\n" n n
@@ -6089,7 +6100,8 @@ and generate_daemon_actions () =
if List.exists (function Pathname _ -> true | _ -> false) (snd style) then (
(* Emit NEED_ROOT just once, even when there are two or
more Pathname args *)
pr " NEED_ROOT (goto done);\n";
pr " NEED_ROOT (%s, goto done);\n"
(if is_filein then "cancel_receive ()" else "");
);
(* Don't want to call the impl with any FileIn or FileOut
@@ -6175,15 +6187,14 @@ and generate_daemon_actions () =
);
(* Free the args. *)
pr "done:\n";
(match snd style with
| [] ->
pr "done: ;\n";
| [] -> ()
| _ ->
pr "done:\n";
pr " xdr_free ((xdrproc_t) xdr_guestfs_%s_args, (char *) &args);\n"
name
);
pr " return;\n";
pr "}\n\n";
) daemon_functions;