2:01 p.m.
From: Chip Vincent <cvincent(a)us.ibm.com>
Fixes many small issues with the current AppliedFilterList provider.
1) Fix Create to properly return a complete object path and fix Delete to
properly parse that path.
2) Persist applied filer rules. Since it's not possible to dyanmically update
a single device, I've changed the code to modify and re-define the VM to
essentially add/remove ACL filter associations.
I also updated the code to minimize domain/device parsing overhead. For
some strange reason, our internal APIs sometimes take a virDomainPtr and
other times a struct domain * forcing providers who work with domains
*and* devices to parse everything twice. Until the internal APIs are
cleaned up, I simply parse everything once and then fetch the device
manually from the struct domain *.
3) Add VIR_DOMAIN_XML_INACTIVE to virDomainGetXML(). By default, libvirt only
returns the XML of the running domain. We need to fetch the *stored* XML
that will be used for the next boot so that all changes made while the VM
is running are preserved.
Changes from v3:
- Fix NestedFilterList DeleteInstance
- Fix remove_filter_ref() in acl_parsing.c (intend to refactor in
future patch)
Changes from v2:
- Return the correct reference in NestedFilterList
Changes from v1:
- Fix leak and other comments
- Fix all cases virDomainGetXML()
- Fix NestedFilterList Create/Delete instance
Signed-off-by: Chip Vincent <cvincent(a)us.ibm.com>
---
libxkutil/acl_parsing.c | 1 +
libxkutil/device_parsing.c | 6 ++-
src/Virt_AppliedFilterList.c | 97 +++++++++++++++++++----------------
src/Virt_ComputerSystem.c | 3 +-
src/Virt_ComputerSystemIndication.c | 3 +-
src/Virt_FilterList.c | 5 ++-
src/Virt_NestedFilterList.c | 19 +++++--
src/Virt_VSMigrationService.c | 3 +-
8 files changed, 82 insertions(+), 55 deletions(-)
diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c
index 5b6d7bb..9c4b4b2 100644
--- a/libxkutil/acl_parsing.c
+++ b/libxkutil/acl_parsing.c
@@ -652,6 +652,7 @@ int remove_filter_ref(struct acl_filter *filter, const char *name)
/* TODO: called infrequently, but needs optimization */
old_refs = filter->refs;
+ filter->ref_ct = 0;
for (i = 0; i < filter->ref_ct; i++) {
if (STREQC(old_refs[i], name)) {
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
index a1e8d6c..ff86f2a 100644
--- a/libxkutil/device_parsing.c
+++ b/libxkutil/device_parsing.c
@@ -996,7 +996,8 @@ int get_devices(virDomainPtr dom, struct virt_device **list, int
type)
char *xml;
int ret;
- xml = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_SECURE);
+ xml = virDomainGetXMLDesc(dom,
+ VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE);
if (xml == NULL)
return 0;
@@ -1241,7 +1242,8 @@ int get_dominfo(virDomainPtr dom, struct domain **dominfo)
char *xml;
int ret;
int start;
- xml = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_SECURE);
+ xml = virDomainGetXMLDesc(dom,
+ VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE);
if (xml == NULL)
return 0;
diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c
index bc31c14..538adf4 100644
--- a/src/Virt_AppliedFilterList.c
+++ b/src/Virt_AppliedFilterList.c
@@ -97,67 +97,60 @@ static CMPIrc cu_get_ref_path(const CMPIObjectPath *reference,
if ((s.rc != CMPI_RC_OK) || CMIsNullValue(value))
return CMPI_RC_ERR_NO_SUCH_PROPERTY;
- /* how to parse and object path? */
+ if ((value.type != CMPI_ref) || CMIsNullObject(value.value.ref))
+ return CMPI_RC_ERR_TYPE_MISMATCH;
+
+ *_reference = value.value.ref;
return CMPI_RC_OK;
}
-/* TODO: Port to libxkutil/device_parsing.c */
-static int update_device(virDomainPtr dom,
- struct virt_device *dev)
+static int update_domain(virConnectPtr conn,
+ struct domain *dominfo)
{
-#if LIBVIR_VERSION_NUMBER > 8000
char *xml = NULL;
- int flags = VIR_DOMAIN_DEVICE_MODIFY_CURRENT |
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- int ret = 0;
+ virDomainPtr dom = NULL;
- xml = device_to_xml(dev);
+ xml = system_to_xml(dominfo);
if (xml == NULL) {
- CU_DEBUG("Failed to get XML for device '%s'",
dev->id);
+ CU_DEBUG("Failed to get XML from domain %s",
dominfo->name);
goto out;
}
- if (virDomainUpdateDeviceFlags(dom, xml, flags) != 0) {
- CU_DEBUG("Failed to dynamically update device");
+ dom = virDomainDefineXML(conn, xml);
+ if (dom == NULL) {
+ CU_DEBUG("Failed to update domain %s", dominfo->name);
goto out;
}
- ret = 1;
out:
free(xml);
+ virDomainFree(dom);
- return ret;
-#else
return 0;
-#endif
}
-/* TODO: Port to libxkutil/device_parsing.c */
-static int get_device_by_devid(virDomainPtr dom,
+static int get_device_by_devid(struct domain *dominfo,
const char *devid,
- int type,
struct virt_device **dev)
{
- int i, ret = 0;
- struct virt_device *devices = NULL;
- int count = get_devices(dom, &devices, type);
+ int i;
+ struct virt_device *devices = dominfo->dev_net;
+ int count = dominfo->dev_net_ct;
+
+ if (dev == NULL)
+ return 0;
for (i = 0; i < count; i++) {
if (STREQC(devid, devices[i].id)) {
CU_DEBUG("Found '%s'", devices[i].id);
- *dev = virt_device_dup(&devices[i]);
- if (*dev != NULL)
- ret = 1;
-
- break;
+ *dev = &devices[i];
+ return 0;
}
}
- cleanup_virt_devices(&devices, count);
-
- return ret;
+ return 1;
}
/**
@@ -425,6 +418,8 @@ static CMPIStatus CreateInstance(
struct virt_device *device = NULL;
virConnectPtr conn = NULL;
virDomainPtr dom = NULL;
+ struct domain *dominfo = NULL;
+ CMPIObjectPath *_reference = NULL;
conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s);
if (conn == NULL)
@@ -487,8 +482,12 @@ static CMPIStatus CreateInstance(
goto out;
}
- get_device_by_devid(dom, net_name, CIM_RES_TYPE_NET, &device);
- if (device == NULL) {
+ if (get_dominfo(dom, &dominfo) == 0) {
+ CU_DEBUG("Failed to get dominfo");
+ goto out;
+ }
+
+ if (get_device_by_devid(dominfo, net_name, &device) != 0) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
"Dependent.Name object does not exist");
@@ -502,14 +501,19 @@ static CMPIStatus CreateInstance(
device->dev.net.filter_ref = strdup(filter_name);
- if (update_device(dom, device) == 0) {
+ if (update_domain(conn, dominfo) != 0) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Failed to update device");
+ "Failed to update domain");
goto out;
}
- CMReturnObjectPath(results, reference);
+ /* create new object path */
+ _reference = CMClone(reference, NULL);
+ CMAddKey(_reference, "Antecedent", (CMPIValue *)&antecedent,
CMPI_ref);
+ CMAddKey(_reference, "Dependent", (CMPIValue *)&dependent,
CMPI_ref);
+
+ CMReturnObjectPath(results, _reference);
CU_DEBUG("CreateInstance complete");
out:
@@ -542,6 +546,7 @@ static CMPIStatus DeleteInstance(
struct virt_device *device = NULL;
virConnectPtr conn = NULL;
virDomainPtr dom = NULL;
+ struct domain *dominfo = NULL;
conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s);
if (conn == NULL)
@@ -557,7 +562,7 @@ static CMPIStatus DeleteInstance(
goto out;
}
- if (cu_get_str_path(reference, "DeviceID",
+ if (cu_get_str_path(antecedent, "DeviceID",
&device_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
@@ -573,7 +578,7 @@ static CMPIStatus DeleteInstance(
goto out;
}
- if (cu_get_str_path(reference, "Name",
+ if (cu_get_str_path(dependent, "Name",
&filter_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
@@ -585,7 +590,7 @@ static CMPIStatus DeleteInstance(
if (filter == NULL) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Antecedent.Name object does not exist");
+ "Dependent.Name object does not exist");
goto out;
}
@@ -600,11 +605,15 @@ static CMPIStatus DeleteInstance(
goto out;
}
- get_device_by_devid(dom, net_name, CIM_RES_TYPE_NET, &device);
- if (device == NULL) {
+ if (get_dominfo(dom, &dominfo) == 0) {
+ CU_DEBUG("Failed to get dominfo");
+ goto out;
+ }
+
+ if (get_device_by_devid(dominfo, net_name, &device) != 0) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Dependent.Name object does not exist");
+ "Antecedent.Name object does not exist");
goto out;
}
@@ -613,14 +622,14 @@ static CMPIStatus DeleteInstance(
device->dev.net.filter_ref = NULL;
}
- if (update_device(dom, device) == 0) {
+ if (update_domain(conn, dominfo) != 0) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Failed to update device");
+ "Failed to update domain");
goto out;
}
- CU_DEBUG("CreateInstance complete");
+ CU_DEBUG("DeleteInstance complete");
out:
free(domain_name);
diff --git a/src/Virt_ComputerSystem.c b/src/Virt_ComputerSystem.c
index 582253a..e6c7e55 100644
--- a/src/Virt_ComputerSystem.c
+++ b/src/Virt_ComputerSystem.c
@@ -926,7 +926,8 @@ static CMPIStatus domain_reset(virDomainPtr dom)
return s;
}
- xml = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_SECURE);
+ xml = virDomainGetXMLDesc(dom,
+ VIR_DOMAIN_XML_INACTIVE |VIR_DOMAIN_XML_SECURE);
if (xml == NULL) {
CU_DEBUG("Unable to retrieve domain XML");
virt_set_status(_BROKER, &s,
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c
index eb1a71c..6ef2ddc 100644
--- a/src/Virt_ComputerSystemIndication.c
+++ b/src/Virt_ComputerSystemIndication.c
@@ -107,7 +107,8 @@ static int csi_dom_xml_set(csi_dom_xml_t *dom, virDomainPtr dom_ptr,
CMPIStatus
dom->name = strdup(name);
/* xml */
- dom->xml = virDomainGetXMLDesc(dom_ptr, VIR_DOMAIN_XML_SECURE);
+ dom->xml = virDomainGetXMLDesc(dom_ptr,
+ VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE);
if (dom->xml == NULL) {
cu_statusf(_BROKER, s,
CMPI_RC_ERR_FAILED,
diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c
index 35d18a9..5b1b6e8 100644
--- a/src/Virt_FilterList.c
+++ b/src/Virt_FilterList.c
@@ -358,7 +358,10 @@ static CMPIStatus DeleteInstance(
goto out;
}
- delete_filter(conn, filter);
+ if (delete_filter(conn, filter) != 0) {
+ CU_DEBUG("Failed to delete filter %s", filter->name);
+ goto out;
+ }
out:
cleanup_filters(&filter, 1);
diff --git a/src/Virt_NestedFilterList.c b/src/Virt_NestedFilterList.c
index 894cd7c..81c4408 100644
--- a/src/Virt_NestedFilterList.c
+++ b/src/Virt_NestedFilterList.c
@@ -98,7 +98,10 @@ static CMPIrc cu_get_ref_path(const CMPIObjectPath *reference,
if ((s.rc != CMPI_RC_OK) || CMIsNullValue(value))
return CMPI_RC_ERR_NO_SUCH_PROPERTY;
- /* how to parse and object path? */
+ if ((value.type != CMPI_ref) || CMIsNullObject(value.value.ref))
+ return CMPI_RC_ERR_TYPE_MISMATCH;
+
+ *_reference = value.value.ref;
return CMPI_RC_OK;
}
@@ -305,6 +308,7 @@ static CMPIStatus CreateInstance(
const char *child_name = NULL;
struct acl_filter *child_filter = NULL;
virConnectPtr conn = NULL;
+ CMPIObjectPath *_reference = NULL;
CU_DEBUG("Reference = %s", REF2STR(reference));
@@ -383,7 +387,12 @@ static CMPIStatus CreateInstance(
goto out;
}
- CMReturnObjectPath(results, reference);
+ /* create new object path */
+ _reference = CMClone(reference, NULL);
+ CMAddKey(_reference, "Antecedent", (CMPIValue *)&antecedent,
CMPI_ref);
+ CMAddKey(_reference, "Dependent", (CMPIValue *)&dependent,
CMPI_ref);
+
+ CMReturnObjectPath(results, _reference);
CU_DEBUG("CreateInstance completed");
out:
@@ -423,7 +432,7 @@ static CMPIStatus DeleteInstance(
goto out;
}
- if (cu_get_str_path(reference, "Name", &parent_name) != CMPI_RC_OK)
{
+ 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");
@@ -446,7 +455,7 @@ static CMPIStatus DeleteInstance(
goto out;
}
- if (cu_get_str_path(reference, "Name", &child_name) != CMPI_RC_OK)
{
+ 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");
@@ -475,7 +484,7 @@ static CMPIStatus DeleteInstance(
goto out;
}
- CU_DEBUG("CreateInstance completed");
+ CU_DEBUG("DeleteInstance completed");
out:
cleanup_filters(&parent_filter, 1);
diff --git a/src/Virt_VSMigrationService.c b/src/Virt_VSMigrationService.c
index d393787..76e3d25 100644
--- a/src/Virt_VSMigrationService.c
+++ b/src/Virt_VSMigrationService.c
@@ -1070,7 +1070,8 @@ static CMPIStatus prepare_migrate(virDomainPtr dom,
{
CMPIStatus s = {CMPI_RC_OK, NULL};
- *xml = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_SECURE);
+ *xml = virDomainGetXMLDesc(dom,
+ VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE);
if (*xml == NULL) {
virt_set_status(_BROKER, &s,
--
1.7.1