# HG changeset patch # User Chip Vincent <cvincent(a)us.ibm.com&gt; # Date 1311193039 14400 # Node ID c4fae0f5cc8f21f0d66077df7693158c10633ece # Parent 6056961c3c5347d3b8375039767d7bc78fa97eb5 Fix crash when creating ACL filter lists Specifically, AppliedFilterList and NestedFilterList CreateInstance(). For AppliedFilterList: The Antecedent and Dependent were backwards and the provider would crash since no libvirt connection was made before calling get_filter_by_name(). Also added some additional error checking in acl_parsing.c. It appears libvirt w/ qemu does not support dynamic updating of virtual net devices in 0.8.1 so CreateInstance() will still fail (but not crash) in that version. Looking for alternatives to provide dynamic support and will post in a follow-on patch. For NestedFilterList: The code was trying to extract the 'Name' property from the instance reference, rather than the Antecedent and Dependent properties. Also fixed an issue when creating the filter XML. NOTE: I'm still seeing sparatic cores in various providers when doing 'deep' or 'broad' associators calls (not using the -ac). However, there is no indication that bug is tied to this feature. For example, I cannot get the ACL providers to crash when using 'wbemcli ai -ac'. Signed-off-by: Chip Vincent <cvincent(a)us.ibm.com&gt; diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c --- a/libxkutil/acl_parsing.c +++ b/libxkutil/acl_parsing.c @@ -429,6 +429,7 @@ int get_filter_from_xml(const char *xml, struct acl_filter **filter) { xmlDoc *xmldoc = NULL; + int ret = 0; if (xml == NULL || filter == NULL) return 0; @@ -440,15 +441,19 @@ goto err; *filter = malloc(sizeof(**filter)); + if (*filter == NULL) + goto err; memset(*filter, 0, sizeof(**filter)); parse_acl_filter(xmldoc->children, *filter); + ret = 1; + err: xmlSetGenericErrorFunc(NULL, NULL); xmlFreeDoc(xmldoc); - return 1; + return ret; } int get_filter_by_name( @@ -464,6 +469,8 @@ return 0; vfilter = virNWFilterLookupByName(conn, name); + if (vfilter == NULL) + return 0; xml = virNWFilterGetXMLDesc(vfilter, 0); @@ -472,9 +479,7 @@ if (xml == NULL) return 0; - get_filter_from_xml(xml, filter); - - return 1; + return get_filter_from_xml(xml, filter); #else return 0; #endif @@ -493,6 +498,8 @@ return 0; vfilter = virNWFilterLookupByUUIDString(conn, uuid); + if (vfilter == NULL) + return 0; xml = virNWFilterGetXMLDesc(vfilter, 0); @@ -581,31 +588,27 @@ int update_filter(virConnectPtr conn, struct acl_filter *filter) { - if (delete_filter(conn, filter) == 0 || - create_filter(conn, filter) == 0) - return 0; - - return 1; + return create_filter(conn, filter); } int delete_filter(virConnectPtr conn, struct acl_filter *filter) { #if LIBVIR_VERSION_NUMBER > 8000 + int ret = 0; virNWFilterPtr vfilter = NULL; if (filter == NULL) return 0; - vfilter = virNWFilterLookupByUUIDString(conn, filter->uuid); + vfilter = virNWFilterLookupByName(conn, filter->name); if (vfilter == NULL) return 0; - if (virNWFilterUndefine(vfilter) != 0) { - virNWFilterFree(vfilter); - return 0; - } + ret = virNWFilterUndefine(vfilter); - return 1; + virNWFilterFree(vfilter); + + return ret == 0 ? 1 : 0; #else return 0; #endif diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -1375,14 +1375,16 @@ if (xmlNewProp(root, BAD_CAST "name", BAD_CAST filter->name) == NULL) goto out; - if (filter->chain != NULL) + if (filter->chain != NULL) { if (xmlNewProp(root, BAD_CAST "chain", BAD_CAST filter->chain) == NULL) goto out; + } if (filter->uuid != NULL) { - tmp = xmlNewChild(root, NULL, BAD_CAST "uuid", NULL); - if (xmlNewProp(tmp, NULL, BAD_CAST filter->uuid) == NULL) + tmp = xmlNewChild(root, NULL, BAD_CAST "uuid", + BAD_CAST filter->uuid); + if (tmp == NULL) goto out; } @@ -1406,7 +1408,7 @@ msg = NULL; /* no errors */ out: - CU_DEBUG("Filter XML: %s", msg); + CU_DEBUG("Filter XML: %s", xml); xmlFreeNode(root); diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c --- a/src/Virt_AppliedFilterList.c +++ b/src/Virt_AppliedFilterList.c @@ -30,6 +30,7 @@ #include "acl_parsing.h" #include "misc_util.h" #include "cs_util.h" +#include "xmlgen.h" #include "Virt_Device.h" #include "Virt_FilterList.h" @@ -111,20 +112,14 @@ VIR_DOMAIN_DEVICE_MODIFY_CONFIG; int ret = 0; - /** device_to_xml() is not exported, so this function needs - * to be moved - */ - - /* xml = device_to_xml(dev); */ - + xml = device_to_xml(dev); if (xml == NULL) { CU_DEBUG("Failed to get XML for device '%s'", dev->id); goto out; } if (virDomainUpdateDeviceFlags(dom, xml, flags) != 0) { - CU_DEBUG("Failed to dynamically update device:"); - CU_DEBUG("%s", xml); + CU_DEBUG("Failed to dynamically update device"); goto out; } @@ -430,6 +425,10 @@ virConnectPtr conn = NULL; virDomainPtr dom = NULL; + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out; + CU_DEBUG("Reference = %s", REF2STR(reference)); if (cu_get_ref_prop(instance, "Antecedent", @@ -440,18 +439,13 @@ goto out; } - if (cu_get_str_path(reference, "Name", &filter_name) != CMPI_RC_OK) { + CU_DEBUG("Antecedent = %s", REF2STR(antecedent)); + + if (cu_get_str_path(antecedent, "DeviceID", + &device_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, - "Unable to get Antecedent.Name property"); - goto out; - } - - get_filter_by_name(conn, filter_name, &filter); - if (filter == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Antecedent.Name object does not exist"); + "Unable to get Antecedent.DeviceID property"); goto out; } @@ -463,15 +457,23 @@ goto out; } - if (cu_get_str_path(reference, "DeviceID", - &device_name) != CMPI_RC_OK) { + CU_DEBUG("Dependent = %s", REF2STR(dependent)); + + if (cu_get_str_path(dependent, "Name", + &filter_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, - "Unable to get Dependent.DeviceID property"); + "Unable to get Dependent.Name property"); goto out; } - CU_DEBUG("DeviceID = %s", device_name); + get_filter_by_name(conn, filter_name, &filter); + if (filter == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Antecedent.Name object does not exist"); + goto out; + } if (parse_fq_devid(device_name, &domain_name, &net_name) == 0) { CU_DEBUG("Failed to parse devid"); @@ -539,6 +541,10 @@ virConnectPtr conn = NULL; virDomainPtr dom = NULL; + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out; + CU_DEBUG("Reference = %s", REF2STR(reference)); if (cu_get_ref_path(reference, "Antecedent", @@ -549,18 +555,11 @@ goto out; } - if (cu_get_str_path(reference, "Name", &filter_name) != CMPI_RC_OK) { + if (cu_get_str_path(reference, "DeviceID", + &device_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, - "Unable to get Antecedent.Name property"); - goto out; - } - - get_filter_by_name(conn, filter_name, &filter); - if (filter == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Antecedent.Name object does not exist"); + "Unable to get Antecedent.DeviceID property"); goto out; } @@ -572,15 +571,21 @@ goto out; } - if (cu_get_str_path(reference, "DeviceID", - &device_name) != CMPI_RC_OK) { + if (cu_get_str_path(reference, "Name", + &filter_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, - "Unable to get Dependent.DeviceID property"); + "Unable to get Dependent.Name property"); goto out; } - CU_DEBUG("DeviceID = %s", device_name); + get_filter_by_name(conn, filter_name, &filter); + if (filter == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Antecedent.Name object does not exist"); + goto out; + } if (parse_fq_devid(device_name, &domain_name, &net_name) == 0) { CU_DEBUG("Failed to parse devid"); diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c --- a/src/Virt_FilterList.c +++ b/src/Virt_FilterList.c @@ -269,6 +269,8 @@ CMPIInstance *_instance = NULL; virConnectPtr conn = NULL; + CU_DEBUG("Reference = %s", REF2STR(reference)); + /**Get Name from instance rather than reference since keys * are set by this provider, not the client. */ @@ -335,6 +337,8 @@ struct acl_filter *filter = NULL; virConnectPtr conn = NULL; + CU_DEBUG("Reference = %s", REF2STR(reference)); + if (cu_get_str_path(reference, "Name", &name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, diff --git a/src/Virt_NestedFilterList.c b/src/Virt_NestedFilterList.c --- a/src/Virt_NestedFilterList.c +++ b/src/Virt_NestedFilterList.c @@ -323,13 +323,17 @@ goto out; } - if (cu_get_str_path(reference, "Name", &parent_name) != CMPI_RC_OK) { + CU_DEBUG("Antecedent = %s", REF2STR(antecedent)); + + if (cu_get_str_path(antecedent, "Name", &parent_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Unable to get Antecedent.Name property"); goto out; } + CU_DEBUG("Antecedent.Name = %s", parent_name); + get_filter_by_name(conn, parent_name, &parent_filter); if (parent_filter == NULL) { cu_statusf(_BROKER, &s, @@ -346,13 +350,17 @@ goto out; } - if (cu_get_str_path(reference, "Name", &child_name) != CMPI_RC_OK) { + CU_DEBUG("Dependent = %s", REF2STR(dependent)); + + if (cu_get_str_path(dependent, "Name", &child_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Unable to get Dependent.Name property"); goto out; } + CU_DEBUG("Dependent.Name = %s", child_name); + get_filter_by_name(conn, child_name, &child_filter); if (child_filter == NULL) { cu_statusf(_BROKER, &s, @@ -368,6 +376,9 @@ goto out; } + CU_DEBUG("filter appended, parent_filter->name = %s", + parent_filter->name); + if (update_filter(conn, parent_filter) == 0) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED,