shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2024-10-28 20:25:38 +03:00
Code

machine-image: rework error handling

Browse Source 
Let's rework error handling a bit in image_find() and friends: when we
can't find an image, return -ENOENT rather than 0. That's better as
before we violated the usual rule in our codebase that return parameters
are initialized when the return value is >= 0 and otherwise not touched.

This also makes enumeration and validation a bit more strict: we'll only
accept ".raw" as suffix for regular files, and filter out this suffix
handling on directories/subvolumes, where it makes no sense.
This commit is contained in:
Lennart Poettering 2018-04-06 18:53:57 +02:00
parent 225219e504
commit 3a6ce860ac
7 changed files with 138 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/import/export.c
View File

@ -70,12 +70,10 @@ static int export_tar(int argc, char *argv[], void *userdata) {
if (machine_name_is_valid(argv[1])) {
r = image_find(IMAGE_MACHINE, argv[1], &image);
if (r == -ENOENT)
return log_error_errno(r, "Machine image %s not found.", argv[1]);
if (r < 0)
return log_error_errno(r, "Failed to look for machine %s: %m", argv[1]);
if (r == 0) {
log_error("Machine image %s not found.", argv[1]);
return -ENOENT;
}
local = image->path;
} else
@ -149,12 +147,10 @@ static int export_raw(int argc, char *argv[], void *userdata) {
if (machine_name_is_valid(argv[1])) {
r = image_find(IMAGE_MACHINE, argv[1], &image);
if (r == -ENOENT)
return log_error_errno(r, "Machine image %s not found.", argv[1]);
if (r < 0)
return log_error_errno(r, "Failed to look for machine %s: %m", argv[1]);
if (r == 0) {
log_error("Machine image %s not found.", argv[1]);
return -ENOENT;
}
local = image->path;
} else

14
src/import/import.c
View File

@ -76,9 +76,10 @@ static int import_tar(int argc, char *argv[], void *userdata) {
if (!arg_force) {
r = image_find(IMAGE_MACHINE, local, NULL);
if (r < 0)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
else if (r > 0) {
if (r < 0) {
if (r != -ENOENT)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
} else {
log_error("Image '%s' already exists.", local);
return -EEXIST;
}
@ -171,9 +172,10 @@ static int import_raw(int argc, char *argv[], void *userdata) {
if (!arg_force) {
r = image_find(IMAGE_MACHINE, local, NULL);
if (r < 0)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
else if (r > 0) {
if (r < 0) {
if (r != -ENOENT)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
} else {
log_error("Image '%s' already exists.", local);
return -EEXIST;
}

14
src/import/pull.c
View File

@ -84,9 +84,10 @@ static int pull_tar(int argc, char *argv[], void *userdata) {
if (!arg_force) {
r = image_find(IMAGE_MACHINE, local, NULL);
if (r < 0)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
else if (r > 0) {
if (r < 0) {
if (r != -ENOENT)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
} else {
log_error("Image '%s' already exists.", local);
return -EEXIST;
}
@ -170,9 +171,10 @@ static int pull_raw(int argc, char *argv[], void *userdata) {
if (!arg_force) {
r = image_find(IMAGE_MACHINE, local, NULL);
if (r < 0)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
else if (r > 0) {
if (r < 0) {
if (r != -ENOENT)
return log_error_errno(r, "Failed to check whether image '%s' exists: %m", local);
} else {
log_error("Image '%s' already exists.", local);
return -EEXIST;
}

4
src/machine/image-dbus.c
View File

@ -436,7 +436,9 @@ int image_object_find(sd_bus *bus, const char *path, const char *interface, void
return r;
r = image_find(IMAGE_MACHINE, e, &image);
if (r <= 0)
if (r == -ENOENT)
return 0;
if (r < 0)
return r;
image->userdata = m;

38
src/machine/machined-dbus.c
View File

@ -146,7 +146,7 @@ static int method_get_image(sd_bus_message *message, void *userdata, sd_bus_erro
return r;
r = image_find(IMAGE_MACHINE, name, NULL);
if (r == 0)
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
@ -739,10 +739,10 @@ static int method_remove_image(sd_bus_message *message, void *userdata, sd_bus_e
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_remove(message, i, error);
@ -763,10 +763,10 @@ static int method_rename_image(sd_bus_message *message, void *userdata, sd_bus_e
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", old_name);
r = image_find(IMAGE_MACHINE, old_name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", old_name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", old_name);
i->userdata = userdata;
return bus_image_method_rename(message, i, error);
@ -787,10 +787,10 @@ static int method_clone_image(sd_bus_message *message, void *userdata, sd_bus_er
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", old_name);
r = image_find(IMAGE_MACHINE, old_name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", old_name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", old_name);
i->userdata = userdata;
return bus_image_method_clone(message, i, error);
@ -811,10 +811,10 @@ static int method_mark_image_read_only(sd_bus_message *message, void *userdata,
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_mark_read_only(message, i, error);
@ -835,10 +835,10 @@ static int method_get_image_hostname(sd_bus_message *message, void *userdata, sd
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_get_hostname(message, i, error);
@ -859,10 +859,10 @@ static int method_get_image_machine_id(sd_bus_message *message, void *userdata,
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_get_machine_id(message, i, error);
@ -883,10 +883,10 @@ static int method_get_image_machine_info(sd_bus_message *message, void *userdata
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_get_machine_info(message, i, error);
@ -907,10 +907,10 @@ static int method_get_image_os_release(sd_bus_message *message, void *userdata,
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_get_os_release(message, i, error);
@ -1212,10 +1212,10 @@ static int method_set_image_limit(sd_bus_message *message, void *userdata, sd_bu
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
r = image_find(IMAGE_MACHINE, name, &i);
if (r == -ENOENT)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
if (r < 0)
return r;
if (r == 0)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
i->userdata = userdata;
return bus_image_method_set_limit(message, i, error);

6
src/nspawn/nspawn.c
View File

@ -2400,12 +2400,10 @@ static int determine_names(void) {
_cleanup_(image_unrefp) Image *i = NULL;
r = image_find(IMAGE_MACHINE, arg_machine, &i);
if (r == -ENOENT)
return log_error_errno(r, "No image for machine '%s'.", arg_machine);
if (r < 0)
return log_error_errno(r, "Failed to find image for machine '%s': %m", arg_machine);
if (r == 0) {
log_error("No image for machine '%s'.", arg_machine);
return -ENOENT;
}
if (IN_SET(i->type, IMAGE_RAW, IMAGE_BLOCK))
r = free_and_strdup(&arg_image, i->path);

138
src/shared/machine-image.c
View File

@ -146,7 +146,6 @@ static int image_new(
i->path = strjoin(path, "/", filename);
else
i->path = strdup(filename);
if (!i->path)
return -ENOMEM;
@ -194,31 +193,40 @@ static int image_make(
int dfd,
const char *path,
const char *filename,
const struct stat *st,
Image **ret) {
_cleanup_free_ char *pretty_buffer = NULL;
struct stat st;
struct stat stbuf;
bool read_only;
int r;
assert(dfd >= 0 || dfd == AT_FDCWD);
assert(filename);
/* We explicitly *do* follow symlinks here, since we want to allow symlinking trees, raw files and block
* devices into /var/lib/machines/, and treat them normally. */
* devices into /var/lib/machines/, and treat them normally.
*
* This function returns -ENOENT if we can't find the image after all, and -EMEDIUMTYPE if it's not a file we
* recognize. */
if (fstatat(dfd, filename, &st, 0) < 0)
return -errno;
if (!st) {
if (fstatat(dfd, filename, &stbuf, 0) < 0)
return -errno;
st = &stbuf;
}
read_only =
(path && path_startswith(path, "/usr")) ||
(faccessat(dfd, filename, W_OK, AT_EACCESS) < 0 && errno == EROFS);
if (S_ISDIR(st.st_mode)) {
if (S_ISDIR(st->st_mode)) {
_cleanup_close_ int fd = -1;
unsigned file_attr = 0;
if (!ret)
return 1;
return 0;
if (!pretty) {
r = extract_pretty(filename, NULL, &pretty_buffer);
@ -233,7 +241,7 @@ static int image_make(
return -errno;
/* btrfs subvolumes have inode 256 */
if (st.st_ino == 256) {
if (st->st_ino == 256) {
r = btrfs_is_filesystem(fd);
if (r < 0)
@ -271,7 +279,7 @@ static int image_make(
}
}
return 1;
return 0;
}
}
@ -292,15 +300,15 @@ static int image_make(
if (r < 0)
return r;
return 1;
return 0;
} else if (S_ISREG(st.st_mode) && endswith(filename, ".raw")) {
} else if (S_ISREG(st->st_mode) && endswith(filename, ".raw")) {
usec_t crtime = 0;
/* It's a RAW disk image */
if (!ret)
return 1;
return 0;
(void) fd_getcrtime_at(dfd, filename, &crtime, 0);
@ -316,26 +324,26 @@ static int image_make(
pretty,
path,
filename,
!(st.st_mode & 0222) || read_only,
!(st->st_mode & 0222) || read_only,
crtime,
timespec_load(&st.st_mtim),
timespec_load(&st->st_mtim),
ret);
if (r < 0)
return r;
(*ret)->usage = (*ret)->usage_exclusive = st.st_blocks * 512;
(*ret)->limit = (*ret)->limit_exclusive = st.st_size;
(*ret)->usage = (*ret)->usage_exclusive = st->st_blocks * 512;
(*ret)->limit = (*ret)->limit_exclusive = st->st_size;
return 1;
return 0;
} else if (S_ISBLK(st.st_mode)) {
} else if (S_ISBLK(st->st_mode)) {
_cleanup_close_ int block_fd = -1;
uint64_t size = UINT64_MAX;
/* A block device */
if (!ret)
return 1;
return 0;
if (!pretty) {
r = extract_pretty(filename, NULL, &pretty_buffer);
@ -349,9 +357,12 @@ static int image_make(
if (block_fd < 0)
log_debug_errno(errno, "Failed to open block device %s/%s, ignoring: %m", path, filename);
else {
if (fstat(block_fd, &st) < 0)
/* Refresh stat data after opening the node */
if (fstat(block_fd, &stbuf) < 0)
return -errno;
if (!S_ISBLK(st.st_mode)) /* Verify that what we opened is actually what we think it is */
st = &stbuf;
if (!S_ISBLK(st->st_mode)) /* Verify that what we opened is actually what we think it is */
return -ENOTTY;
if (!read_only) {
@ -373,7 +384,7 @@ static int image_make(
pretty,
path,
filename,
!(st.st_mode & 0222) || read_only,
!(st->st_mode & 0222) || read_only,
0,
0,
ret);
@ -383,10 +394,10 @@ static int image_make(
if (size != 0 && size != UINT64_MAX)
(*ret)->usage = (*ret)->usage_exclusive = (*ret)->limit = (*ret)->limit_exclusive = size;
return 1;
return 0;
}
return 0;
return -EMEDIUMTYPE;
}
int image_find(ImageClass class, const char *name, Image **ret) {
@ -399,10 +410,11 @@ int image_find(ImageClass class, const char *name, Image **ret) {
/* There are no images with invalid names */
if (!image_name_is_valid(name))
return 0;
return -ENOENT;
NULSTR_FOREACH(path, image_search_path[class]) {
_cleanup_closedir_ DIR *d = NULL;
struct stat st;
d = opendir(path);
if (!d) {
@ -412,18 +424,39 @@ int image_find(ImageClass class, const char *name, Image **ret) {
return -errno;
}
r = image_make(name, dirfd(d), path, name, ret);
if (IN_SET(r, 0, -ENOENT)) {
/* As mentioned above, we follow symlinks on this fstatat(), because we want to permit people to
* symlink block devices into the search path */
if (fstatat(dirfd(d), name, &st, 0) < 0) {
_cleanup_free_ char *raw = NULL;
if (errno != ENOENT)
return -errno;
raw = strappend(name, ".raw");
if (!raw)
return -ENOMEM;
r = image_make(name, dirfd(d), path, raw, ret);
if (IN_SET(r, 0, -ENOENT))
if (fstatat(dirfd(d), raw, &st, 0) < 0) {
if (errno == ENOENT)
continue;
return -errno;
}
if (!S_ISREG(st.st_mode))
continue;
r = image_make(name, dirfd(d), path, raw, &st, ret);
} else {
if (!S_ISDIR(st.st_mode) && !S_ISBLK(st.st_mode))
continue;
r = image_make(name, dirfd(d), path, name, &st, ret);
}
if (IN_SET(r, -ENOENT, -EMEDIUMTYPE))
continue;
if (r < 0)
return r;
@ -431,9 +464,9 @@ int image_find(ImageClass class, const char *name, Image **ret) {
}
if (class == IMAGE_MACHINE && streq(name, ".host"))
return image_make(".host", AT_FDCWD, NULL, "/", ret);
return image_make(".host", AT_FDCWD, NULL, "/", NULL, ret);
return 0;
return -ENOENT;
};
int image_discover(ImageClass class, Hashmap *h) {
@ -459,20 +492,37 @@ int image_discover(ImageClass class, Hashmap *h) {
FOREACH_DIRENT_ALL(de, d, return -errno) {
_cleanup_(image_unrefp) Image *image = NULL;
_cleanup_free_ char *truncated = NULL;
const char *pretty, *e;
const char *pretty;
struct stat st;
if (dot_or_dot_dot(de->d_name))
continue;
e = endswith(de->d_name, ".raw");
if (e) {
/* As mentioned above, we follow symlinks on this fstatat(), because we want to permit people
* to symlink block devices into the search path */
if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) {
if (errno == ENOENT)
continue;
return -errno;
}
if (S_ISREG(st.st_mode)) {
const char *e;
e = endswith(de->d_name, ".raw");
if (!e)
continue;
truncated = strndup(de->d_name, e - de->d_name);
if (!truncated)
return -ENOMEM;
pretty = truncated;
} else
} else if (S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode))
pretty = de->d_name;
else
continue;
if (!image_name_is_valid(pretty))
continue;
@ -480,8 +530,8 @@ int image_discover(ImageClass class, Hashmap *h) {
if (hashmap_contains(h, pretty))
continue;
r = image_make(pretty, dirfd(d), path, de->d_name, &image);
if (IN_SET(r, 0, -ENOENT))
r = image_make(pretty, dirfd(d), path, de->d_name, &st, &image);
if (IN_SET(r, -ENOENT, -EMEDIUMTYPE))
continue;
if (r < 0)
return r;
@ -497,7 +547,7 @@ int image_discover(ImageClass class, Hashmap *h) {
if (class == IMAGE_MACHINE && !hashmap_contains(h, ".host")) {
_cleanup_(image_unrefp) Image *image = NULL;
r = image_make(".host", AT_FDCWD, NULL, "/", &image);
r = image_make(".host", AT_FDCWD, NULL, "/", NULL, &image);
if (r < 0)
return r;
@ -640,10 +690,10 @@ int image_rename(Image *i, const char *new_name) {
return r;
r = image_find(IMAGE_MACHINE, new_name, NULL);
if (r < 0)
return r;
if (r > 0)
if (r >= 0)
return -EEXIST;
if (r != -ENOENT)
return r;
switch (i->type) {
@ -753,10 +803,10 @@ int image_clone(Image *i, const char *new_name, bool read_only) {
return r;
r = image_find(IMAGE_MACHINE, new_name, NULL);
if (r < 0)
return r;
if (r > 0)
if (r >= 0)
return -EEXIST;
if (r != -ENOENT)
return r;
switch (i->type) {