[Libvirt-cim] [PATCH 3 of 3] [RFC] Add external check functionality to CheckIsVSMigratable()

[Jay Gagnon]

Dan Smith wrote:
> # HG changeset patch
> # User Dan Smith <danms at us.ibm.com>
> # Date 1205184074 25200
> # Node ID a9134d48bc71c419c78b7e58a71f873281fe84ac
> # Parent  8eb16f25ec12565c54e553972299fdbdda12533f
> [RFC] Add external check functionality to CheckIsVSMigratable()
>
> Changes:
>  - Fetch the array out of the MSD ourselves to avoid adding a special
>    case to get_msd_values()
>
> Signed-off-by: Dan Smith <danms at us.ibm.com>
>
> diff -r 8eb16f25ec12 -r a9134d48bc71 src/Virt_VSMigrationService.c
> --- a/src/Virt_VSMigrationService.c	Mon Mar 10 14:20:41 2008 -0700
> +++ b/src/Virt_VSMigrationService.c	Mon Mar 10 14:21:14 2008 -0700
> @@ -21,6 +21,13 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <sys/stat.h>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <dirent.h>
> +#include <errno.h>
>
>  #include <uuid/uuid.h>
>
> @@ -39,6 +46,8 @@
>  #include "Virt_VSMigrationService.h"
>  #include "Virt_HostSystem.h"
>  #include "Virt_VSMigrationSettingData.h"
> +
> +#include "config.h"
>
>  #define CIM_JOBSTATE_STARTING 3
>  #define CIM_JOBSTATE_RUNNING 4
> @@ -210,7 +219,6 @@ static CMPIStatus get_msd_values(const C
>                             "Failed to connect to remote host (%s)", uri);
>                  goto out;
>          }
> -
>   out:
>          free(uri);
>   
Now what did that blank line do to you to deserve that? Who knows how 
many compile cycles it's mutely stood witness as all the other lines get 
to become things like control structures and keywords. And what is its 
reward for a near eternity of silent service? A summary deletion, with a 
mere one-character obituary buried in a diff on a mailing list, read by 
few and noticed by fewer. Well, I noticed, and I won't stand for it!
>
> +static char **list_migration_checks(int *count)
> +{
> +        DIR *dir;
> +        struct dirent *de;
> +        char **list = NULL;
> +        int len = 0;
> +
> +        *count = 0;
> +
> +        dir = opendir(MIG_CHECKS_DIR);
> +        if (dir == NULL) {
> +                CU_DEBUG("Unable to open migration checks dir: %s (%s)",
> +                         MIG_CHECKS_DIR,
> +                         strerror(errno));
> +                *count = -1;
> +                return NULL;
> +        }
> +
> +        while ((de = readdir(dir)) != NULL) {
> +                int ret;
> +                char *path = NULL;
> +
> +                if (de->d_name[0] == '.')
> +                        continue;
>   
On top of killing the "." and ".." listings, this will also skip all 
hidden (.foo) files. This is of course fine by me, but just checking 
that this is the intended behavior.
> +static CMPIStatus _call_check(virDomainPtr dom,
> +                              const char *prog,
> +                              const char *param_path)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        pid_t pid;
> +        int i;
> +        int rc = -1;
> +
> +        pid = fork();
> +        if (pid == 0) {
> +                virConnectPtr conn = virDomainGetConnect(dom);
> +                const char *name = virDomainGetName(dom);
> +                const char *uri = virConnectGetURI(conn);
> +
> +                if (setpgrp() == -1)
> +                        perror("setpgrp");
> +
> +                execl(prog, prog, name, uri, param_path, NULL);
> +                CU_DEBUG("exec(%s) failed: %s", prog, strerror(errno));
> +                _exit(1);
> +        }
> +
> +        for (i = 0; i < (MIG_CHECKS_TIMEOUT * 4); i++) {
> +                int status;
> +                if (waitpid(pid, &status, WNOHANG) != pid) {
> +                        usleep(250000);
> +                } else {
> +                        rc = WEXITSTATUS(status);
> +                        break;
> +                }
> +        }
> +
> +        if (rc == -1) {
> +                CU_DEBUG("Killing off stale child %i", pid);
>   
One would think the passing of a child would garner a little more 
emotion, but given the treatment that blank line got, I suppose we 
should be thankful for what we can get.

The rest seems to be typically free of errors, and equally free of heart.

-- 

-Jay