[Libvirt-cim] [PATCH v2] Fix AppliedFilterList creation and deletion

Chip Vincent cvincent at linux.vnet.ibm.com
Thu Jan 26 14:58:21 UTC 2012


From: Chip Vincent <cvincent at 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 v1:
 - Fix leak and other comments
 - Fix all cases virDomainGetXML()
 - Fix NestedFilterList Create/Delete instance

Signed-off-by: Chip Vincent <cvincent at us.ibm.com>
---
 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         |   13 ++++-
 src/Virt_VSMigrationService.c       |    3 +-
 7 files changed, 78 insertions(+), 52 deletions(-)

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..b72c582 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,6 +387,11 @@ static CMPIStatus CreateInstance(
                 goto out;
         }
 
+        /* 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");
 
@@ -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




More information about the Libvirt-cim mailing list