[Libguestfs] [PATCH v1 1/2] appliance: search all types of appliances for each path separately
Richard W.M. Jones
rjones at redhat.com
Thu May 4 12:34:18 UTC 2017
On Thu, May 04, 2017 at 02:20:28PM +0300, Pavel Butsykin wrote:
> This patch changes appliance search using paths with multiple directories. Now
> all appliance checks will be done separately for each directory. For example
> if the path LIBGUESTFS_PATH=/a:/b:/c, then all applainces are searched first in
> /a, then in /b and then in /c. It allows to flexibly configure the libguestfs
> to interact with different appliances.
>
> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
> ---
> lib/appliance.c | 231 ++++++++++++++++++++++++++------------------------------
> 1 file changed, 108 insertions(+), 123 deletions(-)
>
> diff --git a/lib/appliance.c b/lib/appliance.c
> index f12918573..c21525961 100644
> --- a/lib/appliance.c
> +++ b/lib/appliance.c
> @@ -37,18 +37,23 @@
> #include "guestfs.h"
> #include "guestfs-internal.h"
>
> +typedef struct appliance_files {
> + char *kernel;
> + char *initrd;
> + char *image;
> +} appliance_files;
As a matter of preference, I think that typedefs hide the meaning of
the code, so it becomes unclear what we are passing around as a
parameter. Attached is a patch which turns this into a simple struct.
> + r = contains_fixed_appliance (g, path, NULL);
> if (r == 1) {
> const size_t len = strlen (path);
> - *kernel = safe_malloc (g, len + 6 /* "kernel" */ + 2);
> - *initrd = safe_malloc (g, len + 6 /* "initrd" */ + 2);
> - *appliance = safe_malloc (g, len + 4 /* "root" */ + 2);
> - sprintf (*kernel, "%s/kernel", path);
> - sprintf (*initrd, "%s/initrd", path);
> - sprintf (*appliance, "%s/root", path);
> - return 0;
> + appliance->kernel = safe_malloc (g, len + 6 /* "kernel" */ + 2);
> + appliance->initrd = safe_malloc (g, len + 6 /* "initrd" */ + 2);
> + appliance->image = safe_malloc (g, len + 4 /* "root" */ + 2);
> + sprintf (appliance->kernel, "%s/kernel", path);
> + sprintf (appliance->initrd, "%s/initrd", path);
> + sprintf (appliance->image, "%s/root", path);
> + return 1;
This is copied from the existing code, and isn't wrong, but you might
as well simplify it by using safe_asprintf, ie:
appliance->kernel = safe_asprintf (g, "%s/kernel", path);
There are some further cases of the same thing below.
> +static int
> +search_appliance (guestfs_h *g, appliance_files *appliance)
> +{
> + const char *pelem = g->path;
> +
> + /* Note that if g->path is an empty string, we want to check the
> + * current directory (for backwards compatibility with
> + * libguestfs < 1.5.4).
> + */
> + do {
> + size_t len = strcspn (pelem, PATH_SEPARATOR);
> + /* Empty element or "." means current directory. */
> + char *path = (len == 0) ? safe_strdup (g, ".") :
> + safe_strndup (g, pelem, len);
Better to use:
CLEANUP_FREE char *path = ... etc ...
and drop the call to free (path) just after.
The rest looks fine to me.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
-------------- next part --------------
>From 5d367d3be510f2bd24cb5a622ed1343528b552df Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Thu, 4 May 2017 13:24:22 +0100
Subject: [PATCH] UPDATE appliance: search all types of appliances
---
lib/appliance.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/appliance.c b/lib/appliance.c
index 3bc3bc8..68e11ee 100644
--- a/lib/appliance.c
+++ b/lib/appliance.c
@@ -37,23 +37,23 @@
#include "guestfs.h"
#include "guestfs-internal.h"
-typedef struct appliance_files {
+struct appliance_files {
char *kernel;
char *initrd;
char *image;
-} appliance_files;
+};
/* Old-style appliance is going to be obsoleted. */
static const char kernel_name[] = "vmlinuz." host_cpu;
static const char initrd_name[] = "initramfs." host_cpu ".img";
-static int search_appliance (guestfs_h *g, appliance_files *appliance);
+static int search_appliance (guestfs_h *g, struct appliance_files *appliance);
static int dir_contains_file (guestfs_h *g, const char *dir, const char *file);
static int dir_contains_files (guestfs_h *g, const char *dir, ...);
static int contains_old_style_appliance (guestfs_h *g, const char *path, void *data);
static int contains_fixed_appliance (guestfs_h *g, const char *path, void *data);
static int contains_supermin_appliance (guestfs_h *g, const char *path, void *data);
-static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, appliance_files *appliance);
+static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, struct appliance_files *appliance);
static int run_supermin_build (guestfs_h *g, const char *lockfile, const char *appliancedir, const char *supermin_path);
/**
@@ -116,7 +116,7 @@ guestfs_int_build_appliance (guestfs_h *g,
char **appliance_rtn)
{
- appliance_files appliance;
+ struct appliance_files appliance;
if (search_appliance (g, &appliance) != 1)
return -1;
@@ -144,7 +144,7 @@ guestfs_int_build_appliance (guestfs_h *g,
*/
static int
locate_or_build_appliance (guestfs_h *g,
- appliance_files *appliance,
+ struct appliance_files *appliance,
const char *path)
{
int r;
@@ -197,7 +197,7 @@ locate_or_build_appliance (guestfs_h *g,
* -1 = error which aborts the launch process.
*/
static int
-search_appliance (guestfs_h *g, appliance_files *appliance)
+search_appliance (guestfs_h *g, struct appliance_files *appliance)
{
const char *pelem = g->path;
@@ -258,7 +258,7 @@ contains_supermin_appliance (guestfs_h *g, const char *path, void *data)
static int
build_supermin_appliance (guestfs_h *g,
const char *supermin_path,
- appliance_files *appliance)
+ struct appliance_files *appliance)
{
CLEANUP_FREE char *cachedir = NULL, *lockfile = NULL, *appliancedir = NULL;
--
2.9.3
More information about the Libguestfs
mailing list