7:24 p.m.
This adds MSD support to the check functions, as well as adding a check
to make sure the domain in question actually exists on the source system.
7:24 p.m.
New subject: [PATCH 1 of 2] Make the migration check functions aware of MSD
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1205171774 25200
# Node ID 05cbd18bf3644b6e1b32b37c30310fb1b1e23a44
# Parent 1d2157cd04e4d46b50d9e6115e0927d8b0a33b8c
Make the migration check functions aware of MSD
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 1d2157cd04e4 -r 05cbd18bf364 src/Virt_VSMigrationService.c
--- a/src/Virt_VSMigrationService.c Mon Mar 10 14:40:47 2008 +0100
+++ b/src/Virt_VSMigrationService.c Mon Mar 10 10:56:14 2008 -0700
@@ -68,6 +68,63 @@ struct migration_job {
char uuid[33];
};
+static CMPIStatus get_msd(const CMPIObjectPath *ref,
+ const CMPIArgs *argsin,
+ CMPIInstance **msd)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ int ret;
+
+ ret = cu_get_inst_arg(argsin, "MigrationSettingData", msd);
+ if ((ret == CMPI_RC_OK) && (*msd != NULL))
+ goto out;
+
+ s = get_migration_sd(ref, msd, _BROKER, false);
+ if ((s.rc != CMPI_RC_OK) || (*msd == NULL)) {
+ cu_statusf(_BROKER, &s,
+ s.rc,
+ "Unable to get default setting data values");
+ goto out;
+ }
+ CU_DEBUG("Using default values for MigrationSettingData param");
+
+ out:
+ return s;
+}
+
+static CMPIStatus get_migration_type(CMPIInstance *msd,
+ uint16_t *type)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ int ret;
+
+ ret = cu_get_u16_prop(msd, "MigrationType", type);
+ if (ret != CMPI_RC_OK) {
+ cu_statusf(_BROKER, &s,
+ ret,
+ "Invalid MigrationType value");
+ }
+
+ return s;
+}
+
+static CMPIStatus get_migration_uri(CMPIInstance *msd,
+ uint16_t *uri)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ int ret;
+
+ ret = cu_get_u16_prop(msd, "TransportType", uri);
+ if (ret == CMPI_RC_OK)
+ goto out;
+
+ CU_DEBUG("Using default TransportType: %d", CIM_MIGRATE_URI_SSH);
+ *uri = CIM_MIGRATE_URI_SSH;
+
+ out:
+ return s;
+}
+
static char *dest_uri(const char *cn,
const char *dest,
uint16_t transport)
@@ -159,39 +216,67 @@ static CMPIStatus check_hver(virConnectP
return s;
}
+static CMPIStatus get_msd_check_values(const CMPIObjectPath *ref,
+ const char *destination,
+ const CMPIArgs *argsin,
+ virConnectPtr *conn)
+{
+ CMPIStatus s;
+ CMPIInstance *msd;
+ uint16_t uri_type;
+ char *uri = NULL;
+
+ s = get_msd(ref, argsin, &msd);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
+
+ s = get_migration_uri(msd, &uri_type);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
+
+ uri = dest_uri(CLASSNAME(ref), destination, uri_type);
+ if (uri == NULL) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to construct a valid libvirt URI");
+ goto out;
+ }
+
+ *conn = virConnectOpen(uri);
+ if (*conn == NULL) {
+ CU_DEBUG("Failed to connect to remote host (%s)", uri);
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to connect to remote host (%s)", uri);
+ goto out;
+ }
+
+ out:
+ free(uri);
+
+ return s;
+}
+
static CMPIStatus vs_migratable(const CMPIObjectPath *ref,
const char *domain,
const char *destination,
const CMPIResult *results,
+ const CMPIArgs *argsin,
CMPIArgs *argsout)
{
CMPIStatus s;
- char *uri = NULL;
virConnectPtr conn = NULL;
virConnectPtr dconn = NULL;
uint32_t retcode = 1;
CMPIBoolean isMigratable = 0;
- uri = dest_uri(CLASSNAME(ref), destination, CIM_MIGRATE_URI_SSH);
- if (uri == NULL) {
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Failed to construct a valid libvirt URI");
- goto out;
- }
+ s = get_msd_check_values(ref, destination, argsin, &dconn);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s);
if (conn == NULL)
goto out;
-
- dconn = virConnectOpen(uri);
- if (dconn == NULL) {
- CU_DEBUG("Failed to connect to remote host (%s)", uri);
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Failed to connect to remote host (%s)", uri);
- goto out;
- }
s = check_hver(conn, dconn);
if (s.rc != CMPI_RC_OK)
@@ -212,7 +297,6 @@ static CMPIStatus vs_migratable(const CM
CMAddArg(argsout, "IsMigratable",
(CMPIValue *)&isMigratable, CMPI_boolean);
- free(uri);
virConnectClose(conn);
virConnectClose(dconn);
@@ -250,7 +334,7 @@ static CMPIStatus vs_migratable_host(CMP
return s;
}
- return vs_migratable(ref, name, dhost, results, argsout);
+ return vs_migratable(ref, name, dhost, results, argsin, argsout);
}
static CMPIStatus vs_migratable_system(CMPIMethodMI *self,
@@ -293,7 +377,7 @@ static CMPIStatus vs_migratable_system(C
return s;
}
- return vs_migratable(ref, name, dname, results, argsout);
+ return vs_migratable(ref, name, dname, results, argsin, argsout);
}
static const char *ind_type_to_name(int ind_type)
@@ -771,66 +855,9 @@ static CMPIStatus migrate_create_job_ins
return s;
}
-static CMPIStatus get_msd(const CMPIObjectPath *ref,
- const CMPIArgs *argsin,
- CMPIInstance **msd)
-{
- CMPIStatus s = {CMPI_RC_OK, NULL};
- int ret;
-
- ret = cu_get_inst_arg(argsin, "MigrationSettingData", msd);
- if ((ret == CMPI_RC_OK) && (*msd != NULL))
- goto out;
-
- s = get_migration_sd(ref, msd, _BROKER, false);
- if ((s.rc != CMPI_RC_OK) || (*msd == NULL)) {
- cu_statusf(_BROKER, &s,
- s.rc,
- "Unable to get default setting data values");
- goto out;
- }
- CU_DEBUG("Using default values for MigrationSettingData param");
-
- out:
- return s;
-}
-
-static CMPIStatus get_migration_type(CMPIInstance *msd,
- uint16_t *type)
-{
- CMPIStatus s = {CMPI_RC_OK, NULL};
- int ret;
-
- ret = cu_get_u16_prop(msd, "MigrationType", type);
- if (ret != CMPI_RC_OK) {
- cu_statusf(_BROKER, &s,
- ret,
- "Invalid MigrationType value");
- }
-
- return s;
-}
-
-static CMPIStatus get_migration_uri(CMPIInstance *msd,
- uint16_t *uri)
-{
- CMPIStatus s = {CMPI_RC_OK, NULL};
- int ret;
-
- ret = cu_get_u16_prop(msd, "TransportType", uri);
- if (ret == CMPI_RC_OK)
- goto out;
-
- CU_DEBUG("Using default TransportType: %d", CIM_MIGRATE_URI_SSH);
- *uri = CIM_MIGRATE_URI_SSH;
-
- out:
- return s;
-}
-
-static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
- const CMPIArgs *argsin,
- struct migration_job *job)
+static CMPIStatus get_msd_job_values(const CMPIObjectPath *ref,
+ const CMPIArgs *argsin,
+ struct migration_job *job)
{
CMPIStatus s;
CMPIInstance *msd;
@@ -901,7 +928,7 @@ static CMPIStatus migrate_do(const CMPIO
goto out;
}
- s = get_msd_values(ref, argsin, job);
+ s = get_msd_job_values(ref, argsin, job);
if (s.rc != CMPI_RC_OK)
goto out;
8:58 p.m.
New subject: [PATCH 1 of 2] Make the migration check functions aware of MSD
+static CMPIStatus get_msd_check_values(const CMPIObjectPath *ref,
-
-static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
- const CMPIArgs *argsin,
- struct migration_job *job)
+static CMPIStatus get_msd_job_values(const CMPIObjectPath *ref,
+ const CMPIArgs *argsin,
+ struct migration_job *job)
{
I was thinking maybe you could condense these functions some. Instead
of having a transport field in the job struct, you could have a uri field.
So, you could have a function that is called get_msd_values() that has
everything from get_msd_check_values() except the virConnectPtr /
connect_by_classname() part. get_msd_check_values() would just call
get_msd_values() plus the connect_by_classname() bit.
Then the check functions could call get_msd_check_values() and the
migrate functions could call get_msd_values().
Probably not worth the reordering and work though.
Otherwise, I have no issues here. This set tested fine on my system.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
9:50 p.m.
New subject: [PATCH 1 of 2] Make the migration check functions aware of MSD
KR> So, you could have a function that is called get_msd_values() that has
KR> everything from get_msd_check_values() except the virConnectPtr /
KR> connect_by_classname() part. get_msd_check_values() would just call
KR> get_msd_values() plus the connect_by_classname() bit.
There is also the array processing that the check version does. I
experimented with this a little and we ended up having a little
overlap and then a special case for both. Given the rather small size
and the possibility for more special cases later, it didn't seem worth
the mess of combining them.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
7:24 p.m.
New subject: [PATCH 2 of 2] Add lookup of domain to make sure it exists as part of the check
# HG changeset patch
# User Dan Smith <danms(a)us.ibm.com>
# Date 1205171774 25200
# Node ID 29e665fff096426ced0f3c7c9ddc7c0924927a07
# Parent 05cbd18bf3644b6e1b32b37c30310fb1b1e23a44
Add lookup of domain to make sure it exists as part of the check
Signed-off-by: Dan Smith <danms(a)us.ibm.com>
diff -r 05cbd18bf364 -r 29e665fff096 src/Virt_VSMigrationService.c
--- a/src/Virt_VSMigrationService.c Mon Mar 10 10:56:14 2008 -0700
+++ b/src/Virt_VSMigrationService.c Mon Mar 10 10:56:14 2008 -0700
@@ -269,6 +269,7 @@ static CMPIStatus vs_migratable(const CM
virConnectPtr dconn = NULL;
uint32_t retcode = 1;
CMPIBoolean isMigratable = 0;
+ virDomainPtr dom = NULL;
s = get_msd_check_values(ref, destination, argsin, &dconn);
if (s.rc != CMPI_RC_OK)
@@ -281,6 +282,14 @@ static CMPIStatus vs_migratable(const CM
s = check_hver(conn, dconn);
if (s.rc != CMPI_RC_OK)
goto out;
+
+ dom = virDomainLookupByName(conn, domain);
+ if (dom == NULL) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_NOT_FOUND,
+ "No such domain");
+ goto out;
+ }
s = check_caps(conn, dconn);
if (s.rc != CMPI_RC_OK)
@@ -297,6 +306,7 @@ static CMPIStatus vs_migratable(const CM
CMAddArg(argsout, "IsMigratable",
(CMPIValue *)&isMigratable, CMPI_boolean);
+ virDomainFree(dom);
virConnectClose(conn);
virConnectClose(dconn);
11:20 a.m.
New subject: [PATCH 2 of 2] Add lookup of domain to make sure it exists as part of the check
Dan Smith wrote:
@@ -281,6 +282,14 @@ static CMPIStatus vs_migratable(const CM
s = check_hver(conn, dconn);
if (s.rc != CMPI_RC_OK)
goto out;
+
+ dom = virDomainLookupByName(conn, domain);
+ if (dom == NULL) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_NOT_FOUND,
+ "No such domain");
+ goto out;
+ }
Sorry, I know my comment comes a bit late, as the patch is already
checked in.
I think this check is not enough. To check if a certain
Xen/KVM_ComputerSystem or the Xen/KVM_HostSystem object path is correct
specified by the client, you should use the corresponding (new ;))
interfaces (get_domain_by_ref, get_host), as they check the complete
object path for correctness. It raises the number of cycles a bit, but
makes the behavior of all providers consistent. Thanks.
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
2:54 p.m.
New subject: [PATCH 2 of 2] Add lookup of domain to make sure it exists as part of the check
HE> Sorry, I know my comment comes a bit late, as the patch is already
HE> checked in.
That's okay :)
HE> I think this check is not enough. To check if a certain
HE> Xen/KVM_ComputerSystem or the Xen/KVM_HostSystem object path is
HE> correct specified by the client, you should use the corresponding
HE> (new ;)) interfaces (get_domain_by_ref, get_host), as they check
HE> the complete object path for correctness. It raises the number of
HE> cycles a bit, but makes the behavior of all providers
HE> consistent.
Well, the reason I added this is twofold: (1) because of the fact that
we would fail late if the domain doesn't exist, and (2) because I use
the dom pointer later for the external check code.
So, I'm more than happy to add in the more rigorous checks, but we
need this part as well :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
3:54 p.m.
New subject: [PATCH 2 of 2] Add lookup of domain to make sure it exists as part of the check
Dan Smith wrote:
HE> Sorry, I know my comment comes a bit late, as the patch is
already
HE> checked in.
That's okay :)
HE> I think this check is not enough. To check if a certain
HE> Xen/KVM_ComputerSystem or the Xen/KVM_HostSystem object path is
HE> correct specified by the client, you should use the corresponding
HE> (new ;)) interfaces (get_domain_by_ref, get_host), as they check
HE> the complete object path for correctness. It raises the number of
HE> cycles a bit, but makes the behavior of all providers
HE> consistent.
Well, the reason I added this is twofold: (1) because of the fact that
we would fail late if the domain doesn't exist, and (2) because I use
the dom pointer later for the external check code.
So, I'm more than happy to add in the more rigorous checks, but we
need this part as well :)
ok, fine for me :)
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
6032
days inactive
6034
days old
7 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert