[PATCH 0 of 4] Add reference validation to VSMigrationService and VSMigrateCap.
More reference validation updates. This patch set is dependant on the get_vsm_cap() reference validation set.
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1203123381 28800 # Node ID 9b93bc85520bc6a9cb486066e83acd8af2432c6a # Parent db0f626f74052bc78be3d62e9b530f6af6d7ae3b Add reference validation to get_migration_service(). Adds reference validation to VirtualSystemMigrationService. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r db0f626f7405 -r 9b93bc85520b src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Fri Feb 15 13:50:26 2008 -0800 +++ b/src/Virt_VSMigrationService.c Fri Feb 15 16:56:21 2008 -0800 @@ -755,12 +755,23 @@ STDIM_MethodMIStub(, Virt_VSMigrationSer CMPIStatus get_migration_service(const CMPIObjectPath *ref, CMPIInstance **_inst, - const CMPIBroker *broker) + const CMPIBroker *broker, + bool is_get_inst) { CMPIInstance *inst; CMPIStatus s = {CMPI_RC_OK, NULL}; + virConnectPtr conn = NULL; const char *name = NULL; const char *ccname = NULL; + + conn = connect_by_classname(broker, CLASSNAME(ref), &s); + if (conn == NULL) { + if (is_get_inst) + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + } inst = get_typed_instance(broker, CLASSNAME(ref), @@ -793,6 +804,12 @@ CMPIStatus get_migration_service(const C CMSetProperty(inst, "SystemCreationClassName", (CMPIValue *)ccname, CMPI_chars); + if (is_get_inst) { + s = cu_validate_ref(broker, ref, inst); + if (s.rc != CMPI_RC_OK) + goto out; + } + cu_statusf(broker, &s, CMPI_RC_OK, ""); @@ -800,24 +817,28 @@ CMPIStatus get_migration_service(const C *_inst = inst; out: + virConnectClose(conn); + return s; } static CMPIStatus return_vsms(const CMPIObjectPath *ref, const CMPIResult *results, - bool name_only) + bool name_only, + bool is_get_inst) { CMPIInstance *inst; CMPIStatus s; - s = get_migration_service(ref, &inst, _BROKER); - if (s.rc == CMPI_RC_OK) { - if (name_only) - cu_return_instance_name(results, inst); - else - CMReturnInstance(results, inst); - } - + s = get_migration_service(ref, &inst, _BROKER, is_get_inst); + if ((s.rc != CMPI_RC_OK) || (inst == NULL)) + goto out; + + if (name_only) + cu_return_instance_name(results, inst); + else + CMReturnInstance(results, inst); + out: return s; } @@ -826,7 +847,7 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *ref) { - return return_vsms(ref, results, true); + return return_vsms(ref, results, true, false, false); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -836,7 +857,7 @@ static CMPIStatus EnumInstances(CMPIInst const char **properties) { - return return_vsms(ref, results, false); + return return_vsms(ref, results, false, false, false); } @@ -846,24 +867,7 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *ref, const char **properties) { - CMPIInstance *inst; - CMPIStatus s; - const char *prop; - - s = get_migration_service(ref, &inst, _BROKER); - if (s.rc != CMPI_RC_OK) - return s; - - prop = cu_compare_ref(ref, inst); - if (prop != NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "No such instance (%s)", prop); - } else { - CMReturnInstance(results, inst); - } - - return s; + return return_vsms(ref, results, false, true, true); } DEFAULT_CI(); diff -r db0f626f7405 -r 9b93bc85520b src/Virt_VSMigrationService.h --- a/src/Virt_VSMigrationService.h Fri Feb 15 13:50:26 2008 -0800 +++ b/src/Virt_VSMigrationService.h Fri Feb 15 16:56:21 2008 -0800 @@ -21,7 +21,8 @@ CMPIStatus get_migration_service(const CMPIObjectPath *reference, CMPIInstance **_inst, - const CMPIBroker *broker); + const CMPIBroker *broker, + bool is_get_inst); /* * Local Variables:
Kaitlin Rupert wrote:
static CMPIStatus return_vsms(const CMPIObjectPath *ref, const CMPIResult *results, - bool name_only) + bool name_only, + bool is_get_inst) { CMPIInstance *inst;
The same reason here ... please init inst=NULL to avoid seg faulting the return instance functions.
CMPIStatus s;
- s = get_migration_service(ref, &inst, _BROKER); - if (s.rc == CMPI_RC_OK) { - if (name_only) - cu_return_instance_name(results, inst); - else - CMReturnInstance(results, inst); - } - + s = get_migration_service(ref, &inst, _BROKER, is_get_inst); + if ((s.rc != CMPI_RC_OK) || (inst == NULL)) + goto out; + + if (name_only) + cu_return_instance_name(results, inst); + else + CMReturnInstance(results, inst); + out: return s; }
-- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1203123478 28800 # Node ID fe50fd4e9cfa5b858c9c5c6ce312031ca9af7dee # Parent 9b93bc85520bc6a9cb486066e83acd8af2432c6a Update calls to get_migration_service() to use new param. This param indicates whether the reference should be validated, or if an instance (without ref validation) should be returned. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 9b93bc85520b -r fe50fd4e9cfa src/Virt_ElementCapabilities.c --- a/src/Virt_ElementCapabilities.c Fri Feb 15 16:56:21 2008 -0800 +++ b/src/Virt_ElementCapabilities.c Fri Feb 15 16:57:58 2008 -0800 @@ -70,12 +70,16 @@ static CMPIStatus validate_caps_get_serv if ((s.rc != CMPI_RC_OK) || (_inst == NULL)) goto out; - s = get_migration_service(ref, &_inst, _BROKER); + s = get_migration_service(ref, &_inst, _BROKER, false); } else cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, "Not found"); + + if ((s.rc != CMPI_RC_OK) || (_inst == NULL)) + goto out; + *inst = _inst; out: free(classname); @@ -99,7 +103,7 @@ static CMPIStatus validate_service_get_c s = get_vsm_cap(_BROKER, ref, &_inst, false); } else if (STREQC(classname, "VirtualSystemMigrationService")) { - s = get_migration_service(ref, &_inst, _BROKER); + s = get_migration_service(ref, &_inst, _BROKER, true); if ((s.rc != CMPI_RC_OK) || (_inst == NULL)) goto out; diff -r 9b93bc85520b -r fe50fd4e9cfa src/Virt_HostedService.c --- a/src/Virt_HostedService.c Fri Feb 15 16:56:21 2008 -0800 +++ b/src/Virt_HostedService.c Fri Feb 15 16:57:58 2008 -0800 @@ -51,7 +51,7 @@ static CMPIStatus validate_service_ref(c } else if (STREQC(classname, "ResourcePoolConfigurationService")) { s = get_rpcs(ref, &inst, _BROKER, true); } else if (STREQC(classname, "VirtualSystemMigrationService")) { - s = get_migration_service(ref, &inst, _BROKER); + s = get_migration_service(ref, &inst, _BROKER, true); } if (s.rc != CMPI_RC_OK) @@ -117,7 +117,7 @@ static CMPIStatus host_to_service(const if (!CMIsNullObject(inst)) inst_list_add(list, inst); - s = get_migration_service(ref, &inst, _BROKER); + s = get_migration_service(ref, &inst, _BROKER, false); if (s.rc != CMPI_RC_OK) return s; if (!CMIsNullObject(inst)) diff -r 9b93bc85520b -r fe50fd4e9cfa src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Fri Feb 15 16:56:21 2008 -0800 +++ b/src/Virt_VSMigrationService.c Fri Feb 15 16:57:58 2008 -0800 @@ -847,7 +847,7 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *ref) { - return return_vsms(ref, results, true, false, false); + return return_vsms(ref, results, true, false); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -857,7 +857,7 @@ static CMPIStatus EnumInstances(CMPIInst const char **properties) { - return return_vsms(ref, results, false, false, false); + return return_vsms(ref, results, false, false); } @@ -867,7 +867,7 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *ref, const char **properties) { - return return_vsms(ref, results, false, true, true); + return return_vsms(ref, results, false, true); } DEFAULT_CI();
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1203123517 28800 # Node ID f074335959a5eebb4b1a6300f421fbc2a94e3f2a # Parent fe50fd4e9cfa5b858c9c5c6ce312031ca9af7dee Add support for ref checking to VSMigrationCapabilities. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r fe50fd4e9cfa -r f074335959a5 src/Virt_VSMigrationCapabilities.c --- a/src/Virt_VSMigrationCapabilities.c Fri Feb 15 16:57:58 2008 -0800 +++ b/src/Virt_VSMigrationCapabilities.c Fri Feb 15 16:58:37 2008 -0800 @@ -81,10 +81,21 @@ static CMPIStatus set_method_properties( CMPIStatus get_migration_caps(const CMPIObjectPath *ref, CMPIInstance **_inst, - const CMPIBroker *broker) + const CMPIBroker *broker, + bool is_get_inst) { CMPIInstance *inst; CMPIStatus s; + virConnectPtr conn = NULL; + + conn = connect_by_classname(broker, CLASSNAME(ref), &s); + if (conn == NULL) { + if (is_get_inst) + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + } inst = get_typed_instance(broker, CLASSNAME(ref), @@ -102,28 +113,42 @@ CMPIStatus get_migration_caps(const CMPI s = set_method_properties(broker, inst); - if (s.rc == CMPI_RC_OK) - *_inst = inst; + if (s.rc != CMPI_RC_OK) + goto out; + + if (is_get_inst) { + s = cu_validate_ref(broker, ref, inst); + if (s.rc != CMPI_RC_OK) + goto out; + } + + *_inst = inst; + + out: + + virConnectClose(conn); return s; } static CMPIStatus return_vsmc(const CMPIObjectPath *ref, const CMPIResult *results, - bool name_only) + bool name_only, + bool is_get_inst) { CMPIInstance *inst; CMPIStatus s; - s = get_migration_caps(ref, &inst, _BROKER); - - if (s.rc == CMPI_RC_OK) { - if (name_only) - cu_return_instance_name(results, inst); - else - CMReturnInstance(results, inst); - } - + s = get_migration_caps(ref, &inst, _BROKER, is_get_inst); + if ((s.rc != CMPI_RC_OK) || (inst == NULL)) + goto out; + + if (name_only) + cu_return_instance_name(results, inst); + else + CMReturnInstance(results, inst); + + out: return s; } @@ -132,7 +157,7 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *ref) { - return return_vsmc(ref, results, true); + return return_vsmc(ref, results, true, false); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -142,7 +167,7 @@ static CMPIStatus EnumInstances(CMPIInst const char **properties) { - return return_vsmc(ref, results, false); + return return_vsmc(ref, results, false, false); } @@ -152,24 +177,7 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *ref, const char **properties) { - CMPIInstance *inst; - CMPIStatus s; - const char *prop; - - s = get_migration_caps(ref, &inst, _BROKER); - if (s.rc != CMPI_RC_OK) - return s; - - prop = cu_compare_ref(ref, inst); - if (prop != NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "No such instance (%s)", prop); - } else { - CMReturnInstance(results, inst); - } - - return s; + return return_vsmc(ref, results, false, true); } DEFAULT_CI(); diff -r fe50fd4e9cfa -r f074335959a5 src/Virt_VSMigrationCapabilities.h --- a/src/Virt_VSMigrationCapabilities.h Fri Feb 15 16:57:58 2008 -0800 +++ b/src/Virt_VSMigrationCapabilities.h Fri Feb 15 16:58:37 2008 -0800 @@ -21,7 +21,8 @@ CMPIStatus get_migration_caps(const CMPIObjectPath *reference, CMPIInstance **_inst, - const CMPIBroker *broker); + const CMPIBroker *broker, + bool is_get_inst); /* * Local Variables:
Kaitlin Rupert wrote:
@@ -81,10 +81,21 @@ static CMPIStatus set_method_properties(
CMPIStatus get_migration_caps(const CMPIObjectPath *ref, CMPIInstance **_inst, - const CMPIBroker *broker) + const CMPIBroker *broker, + bool is_get_inst) { CMPIInstance *inst; CMPIStatus s;
Please initialize the Status s to become absolutely sure, that a valid status is returned.
+ virConnectPtr conn = NULL; + + conn = connect_by_classname(broker, CLASSNAME(ref), &s); + if (conn == NULL) { + if (is_get_inst) + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + }
inst = get_typed_instance(broker, CLASSNAME(ref),
As the connection is established at this point in time and through all the other code we use pfx_from_conn(conn) in that case, it might be good to also use this here instead of CLASSNAME(ref). But that's more a convenience thing.
static CMPIStatus return_vsmc(const CMPIObjectPath *ref, const CMPIResult *results, - bool name_only) + bool name_only, + bool is_get_inst) { CMPIInstance *inst;
This is causing the reproducible seg fault on my system. Please initialize inst = NULL. In the case of a Xen request on a KVM only system this uninitialized inst is disorienting the check for inst ==NULL and afterwards seg faulting the return instance functions - as inst is only containing garbage.
CMPIStatus s;
- s = get_migration_caps(ref, &inst, _BROKER); - - if (s.rc == CMPI_RC_OK) { - if (name_only) - cu_return_instance_name(results, inst); - else - CMReturnInstance(results, inst); - } - + s = get_migration_caps(ref, &inst, _BROKER, is_get_inst); + if ((s.rc != CMPI_RC_OK) || (inst == NULL)) + goto out; + + if (name_only) + cu_return_instance_name(results, inst); + else + CMReturnInstance(results, inst); + + out: return s; }
-- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
+ }
inst = get_typed_instance(broker, CLASSNAME(ref),
As the connection is established at this point in time and through all the other code we use pfx_from_conn(conn) in that case, it might be good to also use this here instead of CLASSNAME(ref). But that's more a convenience thing.
Good point. =)
static CMPIStatus return_vsmc(const CMPIObjectPath *ref, const CMPIResult *results, - bool name_only) + bool name_only, + bool is_get_inst) { CMPIInstance *inst;
This is causing the reproducible seg fault on my system. Please initialize inst = NULL. In the case of a Xen request on a KVM only system this uninitialized inst is disorienting the check for inst ==NULL and afterwards seg faulting the return instance functions - as inst is only containing garbage.
Thanks for tracking this down! I'll send a new patch to fix this and the other issues you mentioned. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1203123548 28800 # Node ID c7b46732e06984c108bf4991e9f216ba60cffd67 # Parent f074335959a5eebb4b1a6300f421fbc2a94e3f2a Update the get_migration_caps() calls to include new param. This extra param indicates that the reference need to be validated. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r f074335959a5 -r c7b46732e069 src/Virt_ElementCapabilities.c --- a/src/Virt_ElementCapabilities.c Fri Feb 15 16:58:37 2008 -0800 +++ b/src/Virt_ElementCapabilities.c Fri Feb 15 16:59:08 2008 -0800 @@ -66,7 +66,7 @@ static CMPIStatus validate_caps_get_serv s = get_vsms(ref, &_inst, _BROKER, false); } else if (STREQC(classname, "VirtualSystemMigrationCapabilities")) { - s = get_migration_caps(ref, &_inst, _BROKER); + s = get_migration_caps(ref, &_inst, _BROKER, true); if ((s.rc != CMPI_RC_OK) || (_inst == NULL)) goto out; @@ -107,7 +107,7 @@ static CMPIStatus validate_service_get_c if ((s.rc != CMPI_RC_OK) || (_inst == NULL)) goto out; - s = get_migration_caps(ref, &_inst, _BROKER); + s = get_migration_caps(ref, &_inst, _BROKER, false); } else cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, @@ -139,7 +139,7 @@ static CMPIStatus sys_to_cap(const CMPIO if (s.rc == CMPI_RC_OK) inst_list_add(list, inst); - s = get_migration_caps(ref, &inst, _BROKER); + s = get_migration_caps(ref, &inst, _BROKER, false); if (s.rc == CMPI_RC_OK) inst_list_add(list, inst);
Kaitlin Rupert wrote:
More reference validation updates.
This patch set is dependant on the get_vsm_cap() reference validation set.
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
I applied patch set "Add reference validation to get_vsm_cap()." and this one and run into the following problems: wbemein http://localhost/root/virt:Virt_VirtualSystemMigrationCapabilities is seg faulting sfcb with misc_util.c(74): Unable to connect to `xen' -#- Virt_VSMigrationCapabilities - 29723 provider exiting due to a SIGSEGV signal wbemein http://localhost/root/virt:CIM_VirtualSystemMigrationService is hanging sfcb misc_util.c(74): Unable to connect to `xen' -#- Virt_VSMigrationService - 29631 provider exiting due to a SIGSEGV signal -#- Serious provider id / provider process mismatch --- terminating process. Did I miss some patch to apply ? -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
HE> is seg faulting sfcb with HE> misc_util.c(74): Unable to connect to `xen' HE> -#- Virt_VSMigrationCapabilities - 29723 provider exiting due to a HE> SIGSEGV signal I don't see a segfault on SFCB. It returns the VSMCaps object as expected, for both Xen and KVM. If I stop xend, I just get a KVM object back (as expected), and no crash. HE> is hanging sfcb HE> misc_util.c(74): Unable to connect to `xen' HE> -#- Virt_VSMigrationService - 29631 provider exiting due to a SIGSEGV signal HE> -#- Serious provider id / provider process mismatch --- terminating process. I don't see this hang either. The providers return the expected result. Perhaps you have an old version of libcmpiutil, or just need to reinstall all the providers cleanly? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> is seg faulting sfcb with HE> misc_util.c(74): Unable to connect to `xen' HE> -#- Virt_VSMigrationCapabilities - 29723 provider exiting due to a HE> SIGSEGV signal
I don't see a segfault on SFCB. It returns the VSMCaps object as expected, for both Xen and KVM. If I stop xend, I just get a KVM object back (as expected), and no crash.
I did see a segfault once using Pegasus just now. I'm wondering if this is related to a race condition with libvirt. Does this query segfault for you every time?
HE> is hanging sfcb HE> misc_util.c(74): Unable to connect to `xen' HE> -#- Virt_VSMigrationService - 29631 provider exiting due to a SIGSEGV signal HE> -#- Serious provider id / provider process mismatch --- terminating process.
I don't see this hang either. The providers return the expected result.
Perhaps you have an old version of libcmpiutil, or just need to reinstall all the providers cleanly?
-- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
Kaitlin Rupert wrote:
Dan Smith wrote:
HE> is seg faulting sfcb with HE> misc_util.c(74): Unable to connect to `xen' HE> -#- Virt_VSMigrationCapabilities - 29723 provider exiting due to a HE> SIGSEGV signal
I don't see a segfault on SFCB. It returns the VSMCaps object as expected, for both Xen and KVM. If I stop xend, I just get a KVM object back (as expected), and no crash.
I did see a segfault once using Pegasus just now. I'm wondering if this is related to a race condition with libvirt.
Does this query segfault for you every time?
I will try to to find out what's going wrong on my system and will come back ... hopefully by end of my day. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert