[PATCH] VirtualSystemManagementService: Avoid extra connection to libvirt
src/Virt_VirtualSystemManagementService.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) # HG changeset patch # User Eduardo Lima (Etrunko) <eblima@br.ibm.com> # Date 1317242639 10800 # Node ID e02b7fef37d7f1f6a18d68991dc1409eef8905ec # Parent 942e9fa22bcb2681884cb39e1dcfc459c67ce197 VirtualSystemManagementService: Avoid extra connection to libvirt Function update_device_info() has been called in after a creating a connection to libvirt, while the itself creates a new connection. Moving the function call a few lines above adresses this issue. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -2331,6 +2331,8 @@ return s; } + update_dominfo(dominfo, refcn); + conn = connect_by_classname(_BROKER, refcn, &s); if (conn == NULL) { CU_DEBUG("Failed to connect"); @@ -2347,8 +2349,6 @@ goto out; } - update_dominfo(dominfo, refcn); - if (!domain_online(dom)) { CU_DEBUG("VS `%s' not online; skipping dynamic update", dominfo->name);
Actually, this change only prevents a 'nested' connection. There are still 2 connects; one in update_dominfo() and one in _resource_dynamic(). Also, moving the call to update_dominfo() up negates the check done by the code block that begins with 'dom = virDomainLookupByName(conn, dominfo->name);' I don't see the real value of the change. Am I missing something? On 09/28/2011 04:44 PM, Eduardo Lima (Etrunko) wrote:
src/Virt_VirtualSystemManagementService.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317242639 10800 # Node ID e02b7fef37d7f1f6a18d68991dc1409eef8905ec # Parent 942e9fa22bcb2681884cb39e1dcfc459c67ce197 VirtualSystemManagementService: Avoid extra connection to libvirt
Function update_device_info() has been called in after a creating a connection to libvirt, while the itself creates a new connection. Moving the function call a few lines above adresses this issue.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -2331,6 +2331,8 @@ return s; }
+ update_dominfo(dominfo, refcn); + conn = connect_by_classname(_BROKER, refcn,&s); if (conn == NULL) { CU_DEBUG("Failed to connect"); @@ -2347,8 +2349,6 @@ goto out; }
- update_dominfo(dominfo, refcn); - if (!domain_online(dom)) { CU_DEBUG("VS `%s' not online; skipping dynamic update", dominfo->name);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
libvirt-cim-bounces@redhat.com wrote on 10/02/2011 12:41:09 PM:
Chip Vincent <cvincent@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
10/02/11 12:41 PM
Please respond to cvincent@linux.vnet.ibm.com; Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
Subject
Re: [Libvirt-cim] [PATCH] VirtualSystemManagementService: Avoid extra connection to libvirt
Actually, this change only prevents a 'nested' connection. There are still 2 connects; one in update_dominfo() and one in _resource_dynamic().
There should not be 2 connects here. One connection will be created in update_dominfo which will be closed on exiting that function and a new one created later in _resource_dynamic().
Also, moving the call to update_dominfo() up negates the check done by the code block that begins with 'dom = virDomainLookupByName(conn, dominfo->name);'
There is a similar check in update_dominfo too. In my opinion this patch makes sense. -Sharad Mishra
I don't see the real value of the change. Am I missing something?
On 09/28/2011 04:44 PM, Eduardo Lima (Etrunko) wrote:
src/Virt_VirtualSystemManagementService.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317242639 10800 # Node ID e02b7fef37d7f1f6a18d68991dc1409eef8905ec # Parent 942e9fa22bcb2681884cb39e1dcfc459c67ce197 VirtualSystemManagementService: Avoid extra connection to libvirt
Function update_device_info() has been called in after a creating a connection to libvirt, while the itself creates a new connection. Moving the function call a few lines above adresses this issue.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/ Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -2331,6 +2331,8 @@ return s; }
+ update_dominfo(dominfo, refcn); + conn = connect_by_classname(_BROKER, refcn,&s); if (conn == NULL) { CU_DEBUG("Failed to connect"); @@ -2347,8 +2349,6 @@ goto out; }
- update_dominfo(dominfo, refcn); - if (!domain_online(dom)) { CU_DEBUG("VS `%s' not online; skipping dynamic
update",
dominfo->name);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
On 10/03/2011 02:23 PM, Sharad Mishra wrote:
Re: [Libvirt-cim] [PATCH] VirtualSystemManagementService: Avoid extra connection to libvirt
Actually, this change only prevents a 'nested' connection. There are still 2 connects; one in update_dominfo() and one in _resource_dynamic().
There should not be 2 connects here. One connection will be created in update_dominfo which will be closed on exiting that function and a new one created later in _resource_dynamic().
Also, moving the call to update_dominfo() up negates the check done by the code block that begins with 'dom = virDomainLookupByName(conn, dominfo->name);'
There is a similar check in update_dominfo too.
In my opinion this patch makes sense.
-Sharad Mishra
Yes, that was the rationale of the patch. In fact, the update_dominfo function does not make any use of the values in _resource_dynamic. Best regards, Etrunko -- Eduardo de Barros Lima Software Engineer, Open Virtualization Linux Technology Center - IBM/Brazil eblima@br.ibm.com
Okay. Pushed. On 10/06/2011 04:43 PM, Eduardo Lima (Etrunko) wrote:
On 10/03/2011 02:23 PM, Sharad Mishra wrote:
Re: [Libvirt-cim] [PATCH] VirtualSystemManagementService: Avoid extra connection to libvirt
Actually, this change only prevents a 'nested' connection. There are still 2 connects; one in update_dominfo() and one in _resource_dynamic().
There should not be 2 connects here. One connection will be created in update_dominfo which will be closed on exiting that function and a new one created later in _resource_dynamic().
Also, moving the call to update_dominfo() up negates the check done by the code block that begins with 'dom = virDomainLookupByName(conn, dominfo->name);'
There is a similar check in update_dominfo too.
In my opinion this patch makes sense.
-Sharad Mishra
Yes, that was the rationale of the patch. In fact, the update_dominfo function does not make any use of the values in _resource_dynamic.
Best regards, Etrunko
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
participants (3)
-
Chip Vincent -
Eduardo Lima (Etrunko) -
Sharad Mishra