7:25 p.m.
This set adds external check support to VSMigration service
Any executable in the MIG_CHECK_DIR is executed during the CheckVSIsMigratable()
method. If it returns 0, the check continues, nonzero and an error is
reported to the client. If the client puts anything in the CheckParams[] field,
the method writes these out to a temporary file and passes the filename on the
command-line to the check programs.
Migration checks are given MIG_CHECK_TIMEOUT (=10 sec by default) to get their
business done, otherwise they are killed and we report failure. This avoids
letting the checks block the CIMOM for too long.
Comments appreciated.
7:25 p.m.
New subject: [PATCH 1 of 3] [RFC] Add optional check parameter field to migration schema
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1205173462 25200
# Node ID bdfe49f4387203f179d52f4e2e3bd8c2416e4629
# Parent 29e665fff096426ced0f3c7c9ddc7c0924927a07
[RFC] Add optional check parameter field to migration schema
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 29e665fff096 -r bdfe49f43872 schema/VSMigrationSettingData.mof
--- a/schema/VSMigrationSettingData.mof Mon Mar 10 10:56:14 2008 -0700
+++ b/schema/VSMigrationSettingData.mof Mon Mar 10 11:24:22 2008 -0700
@@ -11,6 +11,7 @@ class Xen_VirtualSystemMigrationSettingD
[ ValueMap
{"0","1","2","3","4","5","6"},
Values { "Unknown", "Other", "SSH", "TLS",
"TLS Strict", "TCP", "UNIX" }]
uint16 TransportType;
+ string CheckParameters[];
};
[Provider("cmpi::Virt_VSMigrationSettingData")]
@@ -18,4 +19,5 @@ class KVM_VirtualSystemMigrationSettingD
[ ValueMap
{"0","1","2","3","4","5","6"},
Values { "Unknown", "Other", "SSH", "TLS",
"TLS Strict", "TCP", "UNIX" }]
uint16 TransportType;
+ string CheckParameters[];
};
7:25 p.m.
New subject: [PATCH 2 of 3] [RFC] Add configure support for migration check settings
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1205173468 25200
# Node ID 35861307381c7f9bd604c0080e24453d85e75f96
# Parent bdfe49f4387203f179d52f4e2e3bd8c2416e4629
[RFC] Add configure support for migration check settings
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r bdfe49f43872 -r 35861307381c configure.ac
--- a/configure.ac Mon Mar 10 11:24:22 2008 -0700
+++ b/configure.ac Mon Mar 10 11:24:28 2008 -0700
@@ -72,6 +72,20 @@ AC_ARG_WITH(html-dir,
[test "x$withval" != "x" &&
HTML_DIR="$withval"],
[HTML_DIR='$(datadir)/doc/$(PACKAGE)-$(VERSION)/html'])
AC_SUBST(HTML_DIR)
+
+AC_ARG_WITH([migrate_check_timeout],
+ [ --with-migrate_check_timeout=SECS Max runtime allowed for external migration
checks],
+ [test "x$withval" != "x" &&
MIG_CHECKS_TIMEOUT="$withval"],
+ [MIG_CHECKS_TIMEOUT=10])
+AC_SUBST(MIG_CHECKS_TIMEOUT)
+AC_DEFINE_UNQUOTED(MIG_CHECKS_TIMEOUT, $MIG_CHECKS_TIMEOUT, [External migration check
timeout])
+
+AC_ARG_WITH([migrate_check_dir],
+ [ --with-migrate_check_dir=dir Location of external migration checks],
+ [test "x$withval" != "x" &&
MIG_CHECKS_DIR="$withval"],
+ [MIG_CHECKS_DIR=/usr/libexec/extchecks])
+AC_SUBST(MIG_CHECKS_DIR)
+AC_DEFINE_UNQUOTED(MIG_CHECKS_DIR, "$MIG_CHECKS_DIR", [External migration check
timeout])
# Autogenerate the autoconf header file to store build settings
AC_CONFIG_HEADER([config.h])
10:26 p.m.
New subject: [PATCH 2 of 3] [RFC] Add configure support for migration check settings
DS> + [MIG_CHECKS_DIR=/usr/libexec/extchecks])
I should point out that this was just thrown in there to have
something in place. I'm not sure what a good location would be.
Perhaps $prefix/libexec/libvirt-cim/extchecks ?
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
11 p.m.
New subject: [PATCH 2 of 3] [RFC] Add configure support for migration check settings
Dan Smith wrote:
DS> + [MIG_CHECKS_DIR=/usr/libexec/extchecks])
I should point out that this was just thrown in there to have
something in place. I'm not sure what a good location would be.
Perhaps $prefix/libexec/libvirt-cim/extchecks ?
I think having libvirt-cim in the path is a good way to go. It's more
descriptive that way.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
11:25 a.m.
New subject: [PATCH 2 of 3] [RFC] Add configure support for migration check settings
Kaitlin Rupert wrote:
Dan Smith wrote:
> DS> + [MIG_CHECKS_DIR=/usr/libexec/extchecks])
>
> I should point out that this was just thrown in there to have
> something in place. I'm not sure what a good location would be.
> Perhaps $prefix/libexec/libvirt-cim/extchecks ?
>
>
I think having libvirt-cim in the path is a good way to go. It's more
descriptive that way.
I agree.
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
7:54 a.m.
New subject: [PATCH 2 of 3] [RFC] Add configure support for migration check settings
On Wed, Mar 12, 2008 at 11:25:25AM +0100, Heidi Eckhart wrote:
Kaitlin Rupert wrote:
>Dan Smith wrote:
>>DS> + [MIG_CHECKS_DIR=/usr/libexec/extchecks])
>>
>>I should point out that this was just thrown in there to have
>>something in place. I'm not sure what a good location would be.
>>Perhaps $prefix/libexec/libvirt-cim/extchecks ?
>>
>>
>
>I think having libvirt-cim in the path is a good way to go. It's more
>descriptive that way.
I agree.
I agree too,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
2:29 p.m.
New subject: [PATCH 2 of 3] [RFC] Add configure support for migration check settings
Daniel Veillard wrote:
On Wed, Mar 12, 2008 at 11:25:25AM +0100, Heidi Eckhart wrote:
> Kaitlin Rupert wrote:
>
>> Dan Smith wrote:
>>
>>> DS> + [MIG_CHECKS_DIR=/usr/libexec/extchecks])
>>>
>>> I should point out that this was just thrown in there to have
>>> something in place. I'm not sure what a good location would be.
>>> Perhaps $prefix/libexec/libvirt-cim/extchecks ?
>>>
>>>
>>>
>> I think having libvirt-cim in the path is a good way to go. It's more
>> descriptive that way.
>>
> I agree.
>
I agree too,
Daniel
I refuse to be the only person on the list who doesn't explicitly agree
with this.
--
-Jay
7:25 p.m.
New subject: [PATCH 3 of 3] [RFC] Add external check functionality to CheckIsVSMigratable()
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1205173468 25200
# Node ID bc3883ff55307b2e1836cb5083adbd1c26e4cadf
# Parent 35861307381c7f9bd604c0080e24453d85e75f96
[RFC] Add external check functionality to CheckIsVSMigratable()
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 35861307381c -r bc3883ff5530 src/Virt_VSMigrationService.c
--- a/src/Virt_VSMigrationService.c Mon Mar 10 11:24:28 2008 -0700
+++ b/src/Virt_VSMigrationService.c Mon Mar 10 11:24:28 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
@@ -216,15 +225,254 @@ static CMPIStatus check_hver(virConnectP
return s;
}
+static bool is_valid_check(const char *path)
+{
+ struct stat s;
+
+ if (stat(path, &s) != 0)
+ return false;
+
+ if (!S_ISREG(s.st_mode))
+ return false;
+
+ if ((s.st_mode & S_IXUSR) ||
+ (s.st_mode & S_IXGRP) ||
+ (s.st_mode & S_IXOTH))
+ return true;
+ else
+ return false;
+}
+
+static void free_list(char **list, int count)
+{
+ int i;
+
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < count; i++)
+ free(list[i]);
+
+ free(list);
+}
+
+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;
+
+ if (*count == len) {
+ char **tmp;
+
+ len = (len * 2) + 1;
+ tmp = realloc(list, sizeof(char *) * len);
+ if (tmp == NULL) {
+ CU_DEBUG("Failed to alloc check list");
+ goto error;
+ }
+
+ list = tmp;
+ }
+
+ ret = asprintf(&path,
+ "%s/%s",
+ MIG_CHECKS_DIR,
+ de->d_name);
+ if (ret == -1) {
+ CU_DEBUG("Failed to alloc path for check");
+ goto error;
+ }
+
+ if (is_valid_check(path)) {
+ list[*count] = path;
+ (*count) += 1;
+ } else {
+ CU_DEBUG("Invalid check program: %s", path);
+ free(path);
+ }
+ }
+
+ closedir(dir);
+
+ return list;
+ error:
+ closedir(dir);
+
+ free_list(list, *count);
+ *count = 0;
+
+ return NULL;
+}
+
+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);
+ killpg(pid, SIGKILL);
+ waitpid(pid, NULL, WNOHANG);
+ }
+
+ if (rc != 0) {
+ char *name = strdup(prog);
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Migration check `%s' failed",
+ basename(name));
+ free(name);
+ }
+
+ return s;
+}
+
+static CMPIStatus call_external_checks(virDomainPtr dom,
+ const char *param_path)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ char **list = NULL;
+ int count = 0;
+ int i;
+
+ list = list_migration_checks(&count);
+ if (count < 0) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Unable to execute migration checks");
+ goto out;
+ } else if (list == NULL) {
+ goto out;
+ }
+
+ for (i = 0; i < count; i++) {
+ CU_DEBUG("Calling migration check: %s", list[i]);
+ s = _call_check(dom, list[i], param_path);
+ if (s.rc != CMPI_RC_OK) {
+ CU_DEBUG("...Failed");
+ break;
+ } else
+ CU_DEBUG("...OK");
+ }
+ out:
+ free_list(list, count);
+
+ return s;
+}
+
+static char *write_params(CMPIArray *array)
+{
+ int i;
+ int fd;
+ char *filename = strdup("/tmp/libvirtcim_mig.XXXXXX");
+ FILE *file = NULL;
+
+ if (filename == NULL) {
+ CU_DEBUG("Unable to get temporary file");
+ return NULL;
+ }
+
+ fd = mkstemp(filename);
+ if (fd < 0) {
+ CU_DEBUG("Unable to get temporary file: %s", strerror(errno));
+ free(filename);
+ filename = NULL;
+ goto out;
+ }
+
+ file = fdopen(fd, "w");
+ if (file == NULL) {
+ CU_DEBUG("Unable to open temporary file: %s",
strerror(errno));
+ free(filename);
+ filename = NULL;
+ goto out;
+ }
+
+ for (i = 0; i < CMGetArrayCount(array, NULL); i++) {
+ CMPIData d;
+ CMPIStatus s;
+
+ d = CMGetArrayElementAt(array, i, &s);
+ if ((s.rc != CMPI_RC_OK) || CMIsNullValue(d)) {
+ CU_DEBUG("Unable to get array[%i]: %s",
+ i,
+ CMGetCharPtr(s.msg));
+ continue;
+ }
+
+ fprintf(file, "%s\n", CMGetCharPtr(d.value.string));
+ }
+
+ out:
+ if (file != NULL)
+ fclose(file);
+
+ close(fd);
+
+ return filename;
+}
+
static CMPIStatus get_msd_check_values(const CMPIObjectPath *ref,
const char *destination,
const CMPIArgs *argsin,
- virConnectPtr *conn)
+ virConnectPtr *conn,
+ char **path_to_params)
{
CMPIStatus s;
CMPIInstance *msd;
uint16_t uri_type;
char *uri = NULL;
+ CMPIArray *array;
+
+ *path_to_params = NULL;
s = get_msd(ref, argsin, &msd);
if (s.rc != CMPI_RC_OK)
@@ -251,6 +499,8 @@ static CMPIStatus get_msd_check_values(c
goto out;
}
+ if (cu_get_array_prop(msd, "CheckParameters", &array) ==
CMPI_RC_OK)
+ *path_to_params = write_params(array);
out:
free(uri);
@@ -270,8 +520,9 @@ static CMPIStatus vs_migratable(const CM
uint32_t retcode = 1;
CMPIBoolean isMigratable = 0;
virDomainPtr dom = NULL;
-
- s = get_msd_check_values(ref, destination, argsin, &dconn);
+ char *path = NULL;
+
+ s = get_msd_check_values(ref, destination, argsin, &dconn, &path);
if (s.rc != CMPI_RC_OK)
goto out;
@@ -292,6 +543,10 @@ static CMPIStatus vs_migratable(const CM
}
s = check_caps(conn, dconn);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
+
+ s = call_external_checks(dom, path);
if (s.rc != CMPI_RC_OK)
goto out;
@@ -309,6 +564,10 @@ static CMPIStatus vs_migratable(const CM
virDomainFree(dom);
virConnectClose(conn);
virConnectClose(dconn);
+
+ if (path != NULL)
+ unlink(path);
+ free(path);
return s;
}
6035
days inactive
6038
days old
8 comments
5 participants
participants (5)
-
Dan Smith -
Daniel Veillard -
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert