[PATCH] [RFC] Transition back to connect_by_classname()
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1195080791 28800 # Node ID cfb48c67f1e611f10003a6c17ba458080f9d76f4 # Parent c9e977e820e99070f0c0c0397061229f7382ac7b [RFC] Transition back to connect_by_classname() Previously, we used the incoming reference for all operations to determine the URI to pass to libvirt during a connect operation. For some reason, we transitioned away from that to a detection-based mechanism, which would not support multiple technologies to be managed from the provider simultaneously. I think that we have enough other infrastructure in place now that this is no longer a problem. As a test, I offer this patch with a few key instances of lv_connect() replaced with connect_by_classname() for discussion. I have tested that enumeration of Xen_ComputerSystem, KVM_ComputerSystem, and CIM_ComputerSystem are exclusive, and do not return any duplicate results. Further, I have confirmed that resolving Xen_SystemDevice against a KVM_ComputerSystem instance does not work, and that no duplicate results are returned in the case of CIM_SystemDevice. If this looks okay to people (and wider testing supports my findings), I propose we start to convert uses of lv_connect() back to per-class connections until we encounter a problem. Comments welcome. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r c9e977e820e9 -r cfb48c67f1e6 libxkutil/misc_util.c --- a/libxkutil/misc_util.c Wed Nov 14 13:06:06 2007 +0100 +++ b/libxkutil/misc_util.c Wed Nov 14 14:53:11 2007 -0800 @@ -64,6 +64,8 @@ virConnectPtr connect_by_classname(const return NULL; } + CU_DEBUG("Connecting to libvirt with uri `%s'", uri); + conn = virConnectOpen(uri); if (!conn) { CMSetStatusWithChars(broker, s, @@ -334,28 +336,21 @@ bool provider_is_responsible(const CMPIB const CMPIObjectPath *reference, CMPIStatus *status) { - const char *dft_pfx; char *pfx; - bool rc = false; + bool rc = true; CMSetStatus(status, CMPI_RC_OK); pfx = class_prefix_name(CLASSNAME(reference)); - if (STREQC(pfx, "CIM")) + if (STREQC(pfx, "CIM")) { cu_statusf(broker, status, CMPI_RC_ERR_FAILED, "Please exactly specify the class (check CIMOM behavior!): %s", CLASSNAME(reference)); - - dft_pfx = default_prefix(); - if (dft_pfx == NULL) - goto out; - - if (STREQC(pfx, dft_pfx)) - rc = true; - - out: + rc = false; + } + free(pfx); return rc; } diff -r c9e977e820e9 -r cfb48c67f1e6 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Wed Nov 14 13:06:06 2007 +0100 +++ b/src/Virt_ComputerSystem.c Wed Nov 14 14:53:11 2007 -0800 @@ -346,7 +346,7 @@ static CMPIStatus return_enum_domains(co if (!provider_is_responsible(_BROKER, reference, &s)) return s; - conn = lv_connect(_BROKER, &s); + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); if (conn == NULL) return s; @@ -386,7 +386,7 @@ static CMPIStatus get_domain(const CMPIO return s; } - conn = lv_connect(_BROKER, &s); + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); if (conn == NULL) return s; @@ -609,7 +609,7 @@ static CMPIStatus __state_change(char *n virDomainPtr dom = NULL; virDomainInfo info; - conn = lv_connect(_BROKER, &s); + conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); if (conn == NULL) goto out; diff -r c9e977e820e9 -r cfb48c67f1e6 src/Virt_SystemDevice.c --- a/src/Virt_SystemDevice.c Wed Nov 14 13:06:06 2007 +0100 +++ b/src/Virt_SystemDevice.c Wed Nov 14 14:53:11 2007 -0800 @@ -61,6 +61,7 @@ static int get_dom_devices(const char *n static int get_dom_devices(const char *name, struct inst_list *list, int type, + const char *host_cn, const char *ns) { virConnectPtr conn = NULL; @@ -68,7 +69,7 @@ static int get_dom_devices(const char *n CMPIStatus s; int ret = 0; - conn = lv_connect(_BROKER, &s); + conn = connect_by_classname(_BROKER, host_cn, &s); if (conn == NULL) goto out; @@ -87,17 +88,19 @@ static int get_dom_devices(const char *n static int get_all_devices(const char *name, struct inst_list *list, - char *ns) + const char *host_cn, + const char *ns) { int i; for (i = 0; i < DEV_TYPE_COUNT; i++) - get_dom_devices(name, list, device_types[i], ns); + get_dom_devices(name, list, device_types[i], host_cn, ns); return i; } static CMPIInstance *host_instance(char *name, + const char *host_cn, const char *ns) { CMPIInstance *inst = NULL; @@ -114,7 +117,7 @@ static CMPIInstance *host_instance(char if ((s.rc != CMPI_RC_OK) || CMIsNullObject(op)) goto out; - conn = lv_connect(_BROKER, &s); + conn = connect_by_classname(_BROKER, host_cn, &s); if (conn == NULL) goto out; @@ -173,9 +176,16 @@ static CMPIStatus sys_to_dev(const CMPIO type = device_type_from_classname(info->result_class); - ret = get_dom_devices(host, list, type, NAMESPACE(ref)); + ret = get_dom_devices(host, + list, + type, + CLASSNAME(ref), + NAMESPACE(ref)); } else { - ret = get_all_devices(host, list, NAMESPACE(ref)); + ret = get_all_devices(host, + list, + CLASSNAME(ref), + NAMESPACE(ref)); } if (ret >= 0) { @@ -218,6 +228,7 @@ static CMPIStatus dev_to_sys(const CMPIO } sys = host_instance(host, + CLASSNAME(ref), NAMESPACE(ref)); if (sys == NULL)
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1195080791 28800 # Node ID cfb48c67f1e611f10003a6c17ba458080f9d76f4 # Parent c9e977e820e99070f0c0c0397061229f7382ac7b [RFC] Transition back to connect_by_classname()
Previously, we used the incoming reference for all operations to determine the URI to pass to libvirt during a connect operation. For some reason, we transitioned away from that to a detection-based mechanism, which would not support multiple technologies to be managed from the provider simultaneously.
I think that we have enough other infrastructure in place now that this is no longer a problem. As a test, I offer this patch with a few key instances of lv_connect() replaced with connect_by_classname() for discussion.
I have tested that enumeration of Xen_ComputerSystem, KVM_ComputerSystem, and CIM_ComputerSystem are exclusive, and do not return any duplicate results.
I'm very sorry to tell you, that it does not work with sfcb. [heidineu@localhost ~]$ wbemein http://localhost/root/virt:CIM_ComputerSystem * * wbemein: Cim: (1) CIM_ERR_FAILED: Unable to connect to hypervisor * while the following debug messages are given out: misc_util.c(67): Connecting to libvirt with uri `xen' misc_util.c(67): Connecting to libvirt with uri `qemu:///system' misc_util.c(76): Connected to libvirt with uri `qemu:///system' For me this looks like, that the provider has a problem with being called multiple times (in parallel), because one connection was established with success.
Further, I have confirmed that resolving Xen_SystemDevice against a KVM_ComputerSystem instance does not work, and that no duplicate results are returned in the case of CIM_SystemDevice.
If this looks okay to people (and wider testing supports my findings), I propose we start to convert uses of lv_connect() back to per-class connections until we encounter a problem.
Comments welcome.
-- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck@linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
HE> I'm very sorry to tell you, that it does not work with sfcb. Well, for what it's worth, I can't get it to fail in this way under sfcb. However, I can get pegasus to exhibit this behavior *some* of the time when forceProviderProcesses=true. It's about 25% of the time, and only seems to fail on the first connection attempt. After this, both connections succeed 100% of the time (AFAICT). I feel confident that this is a libvirt problem, as I've been able to reproduce it (very) occasionally with the attached test program. If I introduce a delay in one of the paths (i.e. KVM) I can't get it to fail, which indicates to me that it is a race condition somewhere. I will follow up on this and try to figure out the problem, but I don't think it should block our conversion to by-classname libvirt connections. It would be good if some other people test this patch and report on its behavior with this wbemcli command: HE> [heidineu@localhost ~]$ wbemein http://localhost/root/virt:CIM_ComputerSystem Reports of libvirt version, bitness, etc would be useful. I'm seeing this on libvirt-0.3.3 on an x86_64 machine. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (2)
-
Dan Smith -
Heidi Eckhart