[Libguestfs] [PATCH V3] virt-diff: add new virt-diff tool
Wanlong Gao
gaowanlong at cn.fujitsu.com
Tue Aug 21 08:04:54 UTC 2012
On 08/21/2012 02:44 PM, Richard W.M. Jones wrote:
> On Mon, Aug 20, 2012 at 04:22:23PM +0800, Wanlong Gao wrote:
>> add new virt-diff tool
>>
>> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
>> ---
>> cat/Makefile.am | 20 ++-
>> cat/virt-diff.c | 525 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> po/POTFILES | 1 +
>> 3 files changed, 545 insertions(+), 1 deletion(-)
>> create mode 100644 cat/virt-diff.c
>>
>> diff --git a/cat/Makefile.am b/cat/Makefile.am
>> index f7c763a..5f6a986 100644
>> --- a/cat/Makefile.am
>> +++ b/cat/Makefile.am
>> @@ -27,7 +27,7 @@ EXTRA_DIST = \
>>
>> CLEANFILES = stamp-virt-cat.pod stamp-virt-ls.pod stamp-virt-filesystems.pod
>
> This program has to have a manual page.
I see.
>> + exit (EXIT_FAILURE);
>> +
>> + if (roots[0] == NULL) {
>> + fprintf (stderr,
>> + _("%s: no operating system was found on this disk\n"
>> + "\n"
>> + "If using guestfish '-i' option, remove this option and instead\n"
>> + "use the commands 'run' followed by 'list-filesystems'.\n"
>> + "You can then mount filesystems you want by hand using the\n"
>> + "'mount' or 'mount-ro' command.\n"
>> + "\n"
>> + "If using guestmount '-i', remove this option and choose the\n"
>> + "filesystem(s) you want to see by manually adding '-m' option(s).\n"
>> + "Use 'virt-filesystems' to see what filesystems are available.\n"
>> + "\n"
>> + "If using other virt tools, this disk image won't work\n"
>> + "with these tools. Use the guestfish equivalent commands\n"
>> + "(see the virt tool manual page).\n"),
>> + program_name);
>> + free_strings (roots);
>> + exit (EXIT_FAILURE);
>> + }
>
> This long generic text copied from guestfish isn't appropriate.
> Just reduce it to the first line ("no operating system was found ...").
OK, thank you.
>
>> + if (roots[1] != NULL) {
>> + fprintf (stderr,
>> + _("%s: multi-boot operating systems are not supported\n"
>> + "\n"
>> + "If using guestfish '-i' option, remove this option and instead\n"
>> + "use the commands 'run' followed by 'list-filesystems'.\n"
>> + "You can then mount filesystems you want by hand using the\n"
>> + "'mount' or 'mount-ro' command.\n"
>> + "\n"
>> + "If using guestmount '-i', remove this option and choose the\n"
>> + "filesystem(s) you want to see by manually adding '-m' option(s).\n"
>> + "Use 'virt-filesystems' to see what filesystems are available.\n"
>> + "\n"
>> + "If using other virt tools, multi-boot operating systems won't work\n"
>> + "with these tools. Use the guestfish equivalent commands\n"
>> + "(see the virt tool manual page).\n"),
>> + program_name);
>> + free_strings (roots);
>> + exit (EXIT_FAILURE);
>> + }
>
> And the same here.
OK.
>
>> + root = roots[0];
>> + free (roots);
>> +
>> + diff_inspect_mount_root (g, root);
>> +}
>> +
>> +static void
>> +free_drive (struct drv *drv)
>> +{
>> + if (!drv) return;
>> +
>> + free (drv->device);
>> + free (drv);
>> +}
>> +
>> +int
>> +main (int argc, char *argv[])
>> +{
>> + /* set global program name that is not polluted with libtool artifacts. */
>> + set_program_name (argv[0]);
>> + bindtextdomain (PACKAGE, LOCALEBASEDIR);
>> + textdomain (PACKAGE);
>> +
>> + enum { HELP_OPTION = CHAR_MAX + 1 };
>> +
>> + static const char *options = "s:d:";
>> + static const struct option long_options[] = {
>> + {"seed", 1, 0, 's'},
>> + {"domain", 1, 0, 'd'},
>> + {"help", 0, 0, HELP_OPTION},
>> + {0, 0, 0, 0}
>> + };
>
> Lots of long options have disappeared here, eg. "keys-from-stdin" ...
oops.
>
>> + struct drv *sdrv = NULL;
>> + struct drv *ddrv = NULL;
>> + int c, nr;
>> + int option_index;
>> + int spid, dpid;
>> +
>> + sg = guestfs_create ();
>> + if (sg == NULL) {
>> + fprintf (stderr, _("guestfs_create: failed to create seed handle\n"));
>> + exit (EXIT_FAILURE);
>> + }
>> +
>> + dg = guestfs_create ();
>> + if (dg == NULL) {
>> + fprintf (stderr, _("guestfs_create: failed to create comparison handle\n"));
>> + exit (EXIT_FAILURE);
>> + }
>> +
>> + argv[0] = diff_bad_case (program_name);
>> +
>> + for (;;) {
>> + c = getopt_long (argc, argv, options, long_options, &option_index);
>> + if (c == -1) break;
>> +
>> + switch (c) {
>> + case 0:
>> + if (STREQ (long_options[option_index].name, "keys-from-stdin")) {
>> + keys_from_stdin = 1;
>> + } else if (STREQ (long_options[option_index].name, "echo-keys")) {
>> + echo_keys = 1;
>> + } else if (STREQ (long_options[option_index].name, "seed")) {
>> + if (sdrv) {
>> + fprintf(stderr, _("Only one seed device"));
>
> Missing program_name and \n (but see below).
>
>> + exit (EXIT_FAILURE);
>> + }
>> + sdrv = calloc (1, sizeof (struct drv));
>> + if (!sdrv) {
>> + perror ("malloc");
>> + exit (EXIT_FAILURE);
>> + }
>> + sdrv->type = drv_d;
>> + sdrv->nr_drives = -1;
>> + sdrv->d.guest = optarg;
>> + sdrv->next = NULL;
>> + } else if (STREQ (long_options[option_index].name, "domain")) {
>> + if (ddrv) {
>> + fprintf (stderr, _("Only one diff device"));
>> + exit (EXIT_FAILURE);
>> + }
>> + ddrv = calloc (1, sizeof (struct drv));
>> + if (!ddrv) {
>> + perror ("malloc");
>> + exit (EXIT_FAILURE);
>> + }
>> + ddrv->type = drv_d;
>> + ddrv->nr_drives = -1;
>> + ddrv->d.guest = optarg;
>> + ddrv->next = NULL;
>
> This code will never be used. When getopt_long has an equivalent
> short option, it'll use that instead.
Got it.
>
>> + } else {
>> + fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
>> + program_name, long_options[option_index].name, option_index);
>> + exit (EXIT_FAILURE);
>> + }
>> + break;
>> +
>> + case 'c':
>> + libvirt_uri = optarg;
>
> Missing 'break'.
thank you.
>
>> + case 's':
>> + if (sdrv) {
>> + fprintf(stderr, _("Only one seed device"));
>> + exit (EXIT_FAILURE);
>> + }
>> + sdrv = calloc (1, sizeof (struct drv));
>> + if (!sdrv) {
>> + perror ("malloc");
>> + exit (EXIT_FAILURE);
>> + }
>> + sdrv->type = drv_d;
>> + sdrv->nr_drives = -1;
>> + sdrv->d.guest = optarg;
>> + sdrv->next = NULL;
>> + break;
>> +
>> + case 'd':
>> + if (ddrv) {
>> + fprintf (stderr, _("Only one diff device"));
>> + exit (EXIT_FAILURE);
>> + }
>> + ddrv = calloc (1, sizeof (struct drv));
>> + if (!ddrv) {
>> + perror ("malloc");
>> + exit (EXIT_FAILURE);
>> + }
>> + ddrv->type = drv_d;
>> + ddrv->nr_drives = -1;
>> + ddrv->d.guest = optarg;
>> + ddrv->next = NULL;
>> + break;
>
> The problem with this syntax is that it forces libvirt to be used. In
> fact, you can only diff libvirt domains.
>
> This seems problematic.
>
> Why can't we use '-a' and '-d' options for one of the guests and some
> other options (eg. '--sa' and '--sd') for the other one?
Thank you for this suggestion, I'll try it then.
>
>> + case HELP_OPTION:
>> + diff_usage (EXIT_SUCCESS);
>> +
>> + default:
>> + diff_usage (EXIT_FAILURE);
>> + }
>> + }
>> +
>> + if (sdrv == NULL)
>> + diff_usage (EXIT_FAILURE);
>> + if (ddrv == NULL)
>> + diff_usage (EXIT_FAILURE);
>> +
>> + struct guestfs_add_domain_argv optargs = {
>> + .bitmask = 0,
>> + .libvirturi = libvirt_uri,
>> + .readonly = 1,
>> + .allowuuid = 1,
>> + .readonlydisk = "read",
>> + };
>> + nr = guestfs_add_domain_argv (sg, sdrv->d.guest, &optargs);
>> + if (nr == -1)
>> + exit (EXIT_FAILURE);
>> + sdrv->nr_drives = nr;
>> + nr = guestfs_add_domain_argv (dg, ddrv->d.guest, &optargs);
>> + if (nr == -1)
>> + exit (EXIT_FAILURE);
>> + ddrv->nr_drives = nr;
>> +
>> + if (guestfs_launch (sg) == -1)
>> + exit (EXIT_FAILURE);
>> + if (guestfs_launch (dg) == -1)
>> + exit (EXIT_FAILURE);
>> +
>> + diff_inspect_mount (sg);
>> + diff_inspect_mount (dg);
>> +
>> + char stempdir[] = "/tmp/sGuestXXXXXX";
>> + char dtempdir[] = "/tmp/dGuestXXXXXX";
>> +
>> + if (mkdtemp (stempdir) == NULL) {
>> + perror ("mkdtemp");
>> + exit (EXIT_FAILURE);
>> + }
>> + if (mkdtemp (dtempdir) == NULL) {
>> + perror ("mkdtemp");
>> + exit (EXIT_FAILURE);
>> + }
>> +
>> + if (guestfs_mount_local (sg, stempdir, -1) == -1)
>> + exit (EXIT_FAILURE);
>> + if (guestfs_mount_local (dg, dtempdir, -1) == -1)
>> + exit (EXIT_FAILURE);
>> +
>> + spid = fork ();
>> + if (spid == -1) {
>> + perror ("fork");
>> + exit (EXIT_FAILURE);
>> + }
>> + if (spid == 0) {
>> + if (guestfs_mount_local_run (sg) == -1)
>> + exit (EXIT_FAILURE);
>> + _exit (EXIT_SUCCESS);
>> + }
>> +
>> + dpid = fork();
>> + if (dpid == -1) {
>> + perror ("fork");
>> + exit (EXIT_FAILURE);
>> + }
>> + if (dpid == 0) {
>> + if (guestfs_mount_local_run (dg) == -1)
>> + exit (EXIT_FAILURE);
>> + _exit (EXIT_SUCCESS);
>> + }
>> +
>> + const char *dir = argv[optind];
>> + char system_arg[BUFSIZ];
>> + sprintf (system_arg, "diff -urN %s%s %s%s", stempdir, dir,
>> + dtempdir, dir);
>> + sleep (5);
>
> ?
I think I'm waiting for guest launching?
>
>> + if (system (system_arg) == -1)
>> + exit (EXIT_FAILURE);
>> +
>> + guestfs_umount_local (sg, GUESTFS_UMOUNT_LOCAL_RETRY, 1, -1);
>> + waitpid (spid, NULL, 0);
>> + guestfs_umount_local (dg, GUESTFS_UMOUNT_LOCAL_RETRY, 1, -1);
>> + waitpid (dpid, NULL, 0);
>
> You must check the return value of every call.
>
> Also you should remove the temporary directories.
OK, got it. thank you.
Wanlong Gao
More information about the Libguestfs
mailing list