From f1041e912b72116d66274d2f15e50ce34a9531fd Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Mon, 17 Oct 2011 15:28:47 +0100 Subject: [PATCH] launch: Store drive information in guestfs_h This is a NFC on its own, but provides a place-holder for drive metadata which can be used after launch. Fixes by RWMJ: - Fix the tests: this requires a new internal function 'debug-drives' that dumps out the g->drives information so it can be checked in two of the tests. Previously these tests used 'debug-cmdline'. - Test file existence / use_cache_off in the add_drive_opts function, not when launching qemu in the child process. - Call free along error paths. - Add comments. --- generator/generator_actions.ml | 7 +++ regressions/test-add-domain.sh | 4 +- regressions/test-guestfish-d.sh | 2 +- src/guestfs-internal.h | 19 ++++++- src/guestfs.c | 19 +++++++ src/launch.c | 120 +++++++++++++++++++++++++++++----------- src/virt.c | 4 +- 7 files changed, 137 insertions(+), 38 deletions(-) diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index 86865b8..4967ce2 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -1085,6 +1085,13 @@ Please read L for more details."); This returns the internal QEMU command line. 'debug' commands are not part of the formal API and can be removed or changed at any time."); + ("debug_drives", (RStringList "cmdline", [], []), -1, [NotInDocs], + [], + "debug the drives (internal use only)", + "\ +This returns the internal list of drives. 'debug' commands are +not part of the formal API and can be removed or changed at any time."); + ("add_domain", (RInt "nrdisks", [String "dom"], [String "libvirturi"; Bool "readonly"; String "iface"; Bool "live"; Bool "allowuuid"]), -1, [FishAlias "domain"], [], "add the disk(s) from a named libvirt domain", diff --git a/regressions/test-add-domain.sh b/regressions/test-add-domain.sh index d124b62..ea6d388 100755 --- a/regressions/test-add-domain.sh +++ b/regressions/test-add-domain.sh @@ -58,7 +58,7 @@ EOF ../fish/guestfish >test.out <test.out < test.xml <test.out + debug-drives test.out grep -sq "test1.img.*snapshot=on" test.out ! grep -sq "test1.img.*format" test.out grep -sq "test2.img.*snapshot=on.*format=raw" test.out diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index cd732e1..7bdb4e4 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -149,6 +149,18 @@ struct event { void *opaque2; }; +/* Linked list of drives added to the handle. */ +struct drive { + struct drive *next; + + char *path; + + int readonly; + char *format; + char *iface; + int use_cache_off; +}; + struct guestfs_h { struct guestfs_h *next; /* Linked list of open handles. */ @@ -156,6 +168,8 @@ struct guestfs_h /* State: see the state machine diagram in the man page guestfs(3). */ enum state state; + struct drive *drives; /* Drives added by add-drive* APIs. */ + int fd[2]; /* Stdin/stdout of qemu. */ int sock; /* Daemon communications socket. */ pid_t pid; /* Qemu PID. */ @@ -359,6 +373,7 @@ extern void guestfs___trace (guestfs_h *g, const char *fs, ...) extern const char *guestfs___persistent_tmpdir (void); extern void guestfs___print_timestamped_message (guestfs_h *g, const char *fs, ...); extern void guestfs___free_inspect_info (guestfs_h *g); +extern void guestfs___free_drives (struct drive **drives); extern int guestfs___set_busy (guestfs_h *g); extern int guestfs___end_busy (guestfs_h *g); extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, uint64_t optargs_bitmask, xdrproc_t xdrp, char *args); @@ -380,8 +395,8 @@ extern int guestfs___match2 (guestfs_h *g, const char *str, const pcre *re, char extern int guestfs___match3 (guestfs_h *g, const char *str, const pcre *re, char **ret1, char **ret2, char **ret3); extern int guestfs___feature_available (guestfs_h *g, const char *feature); extern void guestfs___free_string_list (char **); -extern size_t guestfs___checkpoint_cmdline (guestfs_h *g); -extern void guestfs___rollback_cmdline (guestfs_h *g, size_t pos); +extern struct drive ** guestfs___checkpoint_drives (guestfs_h *g); +extern void guestfs___rollback_drives (guestfs_h *g, struct drive **i); extern void guestfs___call_callbacks_void (guestfs_h *g, uint64_t event); extern void guestfs___call_callbacks_message (guestfs_h *g, uint64_t event, const char *buf, size_t buf_len); extern void guestfs___call_callbacks_array (guestfs_h *g, uint64_t event, const uint64_t *array, size_t array_len); diff --git a/src/guestfs.c b/src/guestfs.c index f7ad967..170d0d3 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -206,6 +206,7 @@ guestfs_close (guestfs_h *g) g->events = NULL; guestfs___free_inspect_info (g); + guestfs___free_drives (&g->drives); /* Close sockets. */ if (g->fd[0] >= 0) @@ -994,3 +995,21 @@ guestfs___free_string_list (char **argv) free (argv[i]); free (argv); } + +void +guestfs___free_drives (struct drive **drives) +{ + struct drive *i = *drives; + *drives = NULL; + + while (i != NULL) { + struct drive *next = i->next; + + free (i->path); + free (i->format); + free (i->iface); + free (i); + + i = next; + } +} diff --git a/src/launch.c b/src/launch.c index dbf8ba6..d6e99f7 100644 --- a/src/launch.c +++ b/src/launch.c @@ -139,23 +139,18 @@ add_cmdline (guestfs_h *g, const char *str) return 0; } -size_t -guestfs___checkpoint_cmdline (guestfs_h *g) +struct drive ** +guestfs___checkpoint_drives (guestfs_h *g) { - return g->cmdline_size; + struct drive **i = &g->drives; + while (*i != NULL) i = &((*i)->next); + return i; } void -guestfs___rollback_cmdline (guestfs_h *g, size_t pos) +guestfs___rollback_drives (guestfs_h *g, struct drive **i) { - size_t i; - - assert (g->cmdline_size >= pos); - - for (i = pos; i < g->cmdline_size; ++i) - free (g->cmdline[i]); - - g->cmdline_size = pos; + guestfs___free_drives(i); } /* Internal command to return the command line. */ @@ -178,6 +173,39 @@ guestfs__debug_cmdline (guestfs_h *g) return r; /* caller frees */ } +/* Internal command to return the list of drives. */ +char ** +guestfs__debug_drives (guestfs_h *g) +{ + size_t i, count; + char **ret; + struct drive *drv; + + for (count = 0, drv = g->drives; drv; count++, drv = drv->next) + ; + + ret = safe_malloc (g, sizeof (char *) * (count + 1)); + + for (i = 0, drv = g->drives; drv; i++, drv = drv->next) { + size_t len = 64 + strlen (drv->path) + strlen (drv->iface); + if (drv->format) len += strlen (drv->format); + + ret[i] = safe_malloc (g, len); + + snprintf (ret[i], len, "file=%s%s%s%s%s,if=%s", + drv->path, + drv->readonly ? ",snapshot=on" : "", + drv->use_cache_off ? ",cache=off" : "", + drv->format ? ",format=" : "", + drv->format ? drv->format : "", + drv->iface); + } + + ret[count] = NULL; + + return ret; /* caller frees */ +} + int guestfs__config (guestfs_h *g, const char *qemu_param, const char *qemu_value) @@ -259,8 +287,9 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, const struct guestfs_add_drive_opts_argv *optargs) { int readonly; - const char *format; - const char *iface; + char *format; + char *iface; + int use_cache_off; if (strchr (filename, ',') != NULL) { error (g, _("filename cannot contain ',' (comma) character")); @@ -270,18 +299,22 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, readonly = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_READONLY_BITMASK ? optargs->readonly : 0; format = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_FORMAT_BITMASK - ? optargs->format : NULL; + ? safe_strdup (g, optargs->format) : NULL; iface = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK - ? optargs->iface : DRIVE_IF; + ? safe_strdup (g, optargs->iface) : safe_strdup (g, DRIVE_IF); if (format && !valid_format_iface (format)) { error (g, _("%s parameter is empty or contains disallowed characters"), "format"); + free (format); + free (iface); return -1; } if (!valid_format_iface (iface)) { error (g, _("%s parameter is empty or contains disallowed characters"), "iface"); + free (format); + free (iface); return -1; } @@ -289,31 +322,34 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, * checks for the existence of the file. For readonly we have * to do the check explicitly. */ - int use_cache_off = readonly ? 0 : test_cache_off (g, filename); - if (use_cache_off == -1) + use_cache_off = readonly ? 0 : test_cache_off (g, filename); + if (use_cache_off == -1) { + free (format); + free (iface); return -1; + } if (readonly) { if (access (filename, F_OK) == -1) { perrorf (g, "%s", filename); + free (format); + free (iface); return -1; } } - /* Construct the final -drive parameter. */ - size_t len = 64 + strlen (filename) + strlen (iface); - if (format) len += strlen (format); - char buf[len]; + struct drive **i = &(g->drives); + while (*i != NULL) i = &((*i)->next); - snprintf (buf, len, "file=%s%s%s%s%s,if=%s", - filename, - readonly ? ",snapshot=on" : "", - use_cache_off ? ",cache=off" : "", - format ? ",format=" : "", - format ? format : "", - iface); + *i = safe_malloc (g, sizeof (struct drive)); + (*i)->next = NULL; + (*i)->path = safe_strdup (g, filename); + (*i)->readonly = readonly; + (*i)->format = format; + (*i)->iface = iface; + (*i)->use_cache_off = use_cache_off; - return guestfs__config (g, "-drive", buf); + return 0; } int @@ -433,7 +469,7 @@ launch_appliance (guestfs_h *g) /* At present you must add drives before starting the appliance. In * future when we enable hotplugging you won't need to do this. */ - if (!g->cmdline) { + if (!g->drives) { error (g, _("you must call guestfs_add_drive before guestfs_launch")); return -1; } @@ -527,6 +563,28 @@ launch_appliance (guestfs_h *g) alloc_cmdline (g); g->cmdline[0] = g->qemu; + /* Add drives */ + struct drive *i = g->drives; + while (i != NULL) { + /* Construct the final -drive parameter. */ + size_t len = 64 + strlen (i->path) + strlen (i->iface); + if (i->format) len += strlen (i->format); + char buf[len]; + + snprintf (buf, len, "file=%s%s%s%s%s,if=%s", + i->path, + i->readonly ? ",snapshot=on" : "", + i->use_cache_off ? ",cache=off" : "", + i->format ? ",format=" : "", + i->format ? i->format : "", + i->iface); + + add_cmdline (g, "-drive"); + add_cmdline (g, buf); + + i = i->next; + } + if (qemu_supports (g, "-nodefconfig")) add_cmdline (g, "-nodefconfig"); diff --git a/src/virt.c b/src/virt.c index 58cb999..cc11c68 100644 --- a/src/virt.c +++ b/src/virt.c @@ -392,10 +392,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom, /* Checkpoint the command line around the operation so that either * all disks are added or none are added. */ - cmdline_pos = guestfs___checkpoint_cmdline (g); + struct drive **cp = guestfs___checkpoint_drives (g); r = guestfs___for_each_disk (g, dom, add_disk, &optargs2); if (r == -1) - guestfs___rollback_cmdline (g, cmdline_pos); + guestfs___rollback_drives (g, cp); return r; } -- 1.8.3.1