Re: [Libguestfs] [PATCH v1 1/2] appliance: search all types of appliances for each path separately
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(a)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
Attachments:
- 0001-UPDATE-appliance-search-all-types-of-appliances.patch (text/plain — 2.9 KB)