9:49 a.m.
Hi Guys,
Recently, I has been working on integrate MOM into VDSM. MOM needs to
use VDSM API to interact with it. But currently, it requires the
instance of clientIF to use vdsm API. Passing clientIF to MOM is not a
good choice since it's a vdsm internal object. So I try to remove the
parameter 'cif' from the interface definition and change to access the
globally unique clientIF instance in API.py.
To get the instance of clientIF, I add a decorator to clientIF to
change it into singleton. Actually, clientIF has been working as a
global single instance already. We just don't have an interface to get
it and so passing it as parameter instead. I think using singleton to
get the instance of clientIF is more clean.
Dan and Saggi already gave some comments in
http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But I
think we need more discussion on it, so I post it here because gerrit
is not the appropriate to discuss a design issue.
Thanks !
Mark.
10:01 a.m.
On Wed, May 30, 2012 at 10:49:29PM +0800, Mark Wu wrote:
Hi Guys,
Recently, I has been working on integrate MOM into VDSM. MOM needs
to use VDSM API to interact with it. But currently, it requires the
instance of clientIF to use vdsm API. Passing clientIF to MOM is
not a good choice since it's a vdsm internal object. So I try to
remove the parameter 'cif' from the interface definition and change
to access the globally unique clientIF instance in API.py.
Please remind me - why don't we continue to pass the clientIF instance,
even if it means mentioning it each and every time an API.py object is
created? It may be annoying (and thus serve as a reminder that we should
probably retire much of clientIF...), but it should work.
To get the instance of clientIF, I add a decorator to clientIF to
change it into singleton. Actually, clientIF has been working as a
global single instance already. We just don't have an interface to
get it and so passing it as parameter instead. I think using
singleton to get the instance of clientIF is more clean.
Dan and Saggi already gave some comments in
http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But
I think we need more discussion on it, so I post it here because
gerrit is not the appropriate to discuss a design issue.
Thanks !
Mark.
8:03 a.m.
On 05/30/2012 11:01 PM, Dan Kenigsberg wrote:
On Wed, May 30, 2012 at 10:49:29PM +0800, Mark Wu wrote:
> Hi Guys,
>
> Recently, I has been working on integrate MOM into VDSM. MOM needs
> to use VDSM API to interact with it. But currently, it requires the
> instance of clientIF to use vdsm API. Passing clientIF to MOM is
> not a good choice since it's a vdsm internal object. So I try to
> remove the parameter 'cif' from the interface definition and change
> to access the globally unique clientIF instance in API.py.
Please remind me - why don't we continue to pass the clientIF instance,
even if it means mentioning it each and every time an API.py object is
created? It may be annoying (and thus serve as a reminder that we should
probably retire much of clientIF...), but it should work.
In the old MOM integration patch, I passed the clientIF instance to MOM
by the following method:
vdsmInterface.setConnection(self._cif)
Here's your comments on the patch:
"_cif is not the proper API to interact with Vdsm. API.py is. Please
change MOM to conform to this, if possible.
I think that mom should receive an API object (even API.Global()!) that
it needs for its operation. Even passing BindingXMLRPC() object is more
APIish than the internal clientIF object."
So I try to remove cif from API definition to make MOM can call the VDSM
API without having clientIF.
> To get the instance of clientIF, I add a decorator to clientIF
to
> change it into singleton. Actually, clientIF has been working as a
> global single instance already. We just don't have an interface to
> get it and so passing it as parameter instead. I think using
> singleton to get the instance of clientIF is more clean.
>
> Dan and Saggi already gave some comments in
> http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But
> I think we need more discussion on it, so I post it here because
> gerrit is not the appropriate to discuss a design issue.
>
> Thanks !
> Mark.
>
8:08 a.m.
On Thu, May 31, 2012 at 09:03:37PM +0800, Mark Wu wrote:
On 05/30/2012 11:01 PM, Dan Kenigsberg wrote:
>On Wed, May 30, 2012 at 10:49:29PM +0800, Mark Wu wrote:
>>Hi Guys,
>>
>>Recently, I has been working on integrate MOM into VDSM. MOM needs
>>to use VDSM API to interact with it. But currently, it requires the
>>instance of clientIF to use vdsm API. Passing clientIF to MOM is
>>not a good choice since it's a vdsm internal object. So I try to
>>remove the parameter 'cif' from the interface definition and change
>>to access the globally unique clientIF instance in API.py.
>Please remind me - why don't we continue to pass the clientIF instance,
>even if it means mentioning it each and every time an API.py object is
>created? It may be annoying (and thus serve as a reminder that we should
>probably retire much of clientIF...), but it should work.
>
In the old MOM integration patch, I passed the clientIF instance to
MOM by the following method:
vdsmInterface.setConnection(self._cif)
Here's your comments on the patch:
"_cif is not the proper API to interact with Vdsm. API.py is. Please
change MOM to conform to this, if possible.
I think that mom should receive an API object (even API.Global()!)
that it needs for its operation. Even passing BindingXMLRPC() object
is more APIish than the internal clientIF object."
Please do not blame me! ;-)
I do not mind passing an API.Global() that happens to hold an internal
private reference to _clientIF. I just want that if we find the way to
obliterate clientIF, we won't need to send a patch to MOM, too.
So I try to remove cif from API definition to make MOM can call the
VDSM API without having clientIF.
I do not understand - MOM could receive an API object, it does not have
to construct it by itself.
Regards,
Dan.
9:07 a.m.
On Thu, May 31, 2012 at 04:08:52PM +0300, Dan Kenigsberg wrote:
On Thu, May 31, 2012 at 09:03:37PM +0800, Mark Wu wrote:
> On 05/30/2012 11:01 PM, Dan Kenigsberg wrote:
> >On Wed, May 30, 2012 at 10:49:29PM +0800, Mark Wu wrote:
> >>Hi Guys,
> >>
> >>Recently, I has been working on integrate MOM into VDSM. MOM needs
> >>to use VDSM API to interact with it. But currently, it requires the
> >>instance of clientIF to use vdsm API. Passing clientIF to MOM is
> >>not a good choice since it's a vdsm internal object. So I try to
> >>remove the parameter 'cif' from the interface definition and change
> >>to access the globally unique clientIF instance in API.py.
> >Please remind me - why don't we continue to pass the clientIF instance,
> >even if it means mentioning it each and every time an API.py object is
> >created? It may be annoying (and thus serve as a reminder that we should
> >probably retire much of clientIF...), but it should work.
> >
> In the old MOM integration patch, I passed the clientIF instance to
> MOM by the following method:
> vdsmInterface.setConnection(self._cif)
>
> Here's your comments on the patch:
>
> "_cif is not the proper API to interact with Vdsm. API.py is. Please
> change MOM to conform to this, if possible.
>
> I think that mom should receive an API object (even API.Global()!)
> that it needs for its operation. Even passing BindingXMLRPC() object
> is more APIish than the internal clientIF object."
Please do not blame me! ;-)
I do not mind passing an API.Global() that happens to hold an internal
private reference to _clientIF. I just want that if we find the way to
obliterate clientIF, we won't need to send a patch to MOM, too.
>
> So I try to remove cif from API definition to make MOM can call the
> VDSM API without having clientIF.
I do not understand - MOM could receive an API object, it does not have
to construct it by itself.
Today, the API consists of several, unlinked objects so passing a single
API.Global() would not be enough. We either need to allow MOM to construct its
own API objects or produce them by calling methods in API.Global(). Personally,
I think the code would be cleaner if clientIF is a singleton (Mark's latest
patch) as opposed to adding factory methods to API.Global().
--
Adam Litke <agl(a)us.ibm.com>
IBM Linux Technology Center
12:59 a.m.
On 05/31/2012 09:08 PM, Dan Kenigsberg wrote:
On Thu, May 31, 2012 at 09:03:37PM +0800, Mark Wu wrote:
> On 05/30/2012 11:01 PM, Dan Kenigsberg wrote:
>> On Wed, May 30, 2012 at 10:49:29PM +0800, Mark Wu wrote:
>>> Hi Guys,
>>>
>>> Recently, I has been working on integrate MOM into VDSM. MOM needs
>>> to use VDSM API to interact with it. But currently, it requires the
>>> instance of clientIF to use vdsm API. Passing clientIF to MOM is
>>> not a good choice since it's a vdsm internal object. So I try to
>>> remove the parameter 'cif' from the interface definition and change
>>> to access the globally unique clientIF instance in API.py.
>> Please remind me - why don't we continue to pass the clientIF instance,
>> even if it means mentioning it each and every time an API.py object is
>> created? It may be annoying (and thus serve as a reminder that we should
>> probably retire much of clientIF...), but it should work.
>>
> In the old MOM integration patch, I passed the clientIF instance to
> MOM by the following method:
> vdsmInterface.setConnection(self._cif)
>
> Here's your comments on the patch:
>
> "_cif is not the proper API to interact with Vdsm. API.py is. Please
> change MOM to conform to this, if possible.
>
> I think that mom should receive an API object (even API.Global()!)
> that it needs for its operation. Even passing BindingXMLRPC() object
> is more APIish than the internal clientIF object."
Please do not blame me! ;-)
No body dares to blame the VDSM maintainer! :P
I do not mind passing an API.Global() that happens to hold an
internal
private reference to _clientIF. I just want that if we find the way to
obliterate clientIF, we won't need to send a patch to MOM, too.
It seems I misunderstood your previous comments. I am sorry for that.
> So I try to remove cif from API definition to make MOM can call
the
> VDSM API without having clientIF.
I do not understand - MOM could receive an API object, it does not have
to construct it by itself.
yes, we can pass an API.Global() instance to MOM, but
MOM also needs to
create API.VM instance. Without clientIF, I could pass a wrapped VM class
to MOM. But it's ugly. So I try to remove clientIF in API exposed
interface, and make the caller can use vdsm API just after importing the
module. It could be cleaner. What's your opinion?
Thanks a lot!!
Mark
Regards,
Dan.
5:25 a.m.
On Fri, Jun 01, 2012 at 01:59:25PM +0800, Mark Wu wrote:
On 05/31/2012 09:08 PM, Dan Kenigsberg wrote:
>On Thu, May 31, 2012 at 09:03:37PM +0800, Mark Wu wrote:
>>On 05/30/2012 11:01 PM, Dan Kenigsberg wrote:
>>>On Wed, May 30, 2012 at 10:49:29PM +0800, Mark Wu wrote:
>>>>Hi Guys,
>>>>
>>>>Recently, I has been working on integrate MOM into VDSM. MOM needs
>>>>to use VDSM API to interact with it. But currently, it requires the
>>>>instance of clientIF to use vdsm API. Passing clientIF to MOM is
>>>>not a good choice since it's a vdsm internal object. So I try to
>>>>remove the parameter 'cif' from the interface definition and
change
>>>>to access the globally unique clientIF instance in API.py.
>>>Please remind me - why don't we continue to pass the clientIF instance,
>>>even if it means mentioning it each and every time an API.py object is
>>>created? It may be annoying (and thus serve as a reminder that we should
>>>probably retire much of clientIF...), but it should work.
>>>
>>In the old MOM integration patch, I passed the clientIF instance to
>>MOM by the following method:
>>vdsmInterface.setConnection(self._cif)
>>
>>Here's your comments on the patch:
>>
>>"_cif is not the proper API to interact with Vdsm. API.py is. Please
>>change MOM to conform to this, if possible.
>>
>>I think that mom should receive an API object (even API.Global()!)
>>that it needs for its operation. Even passing BindingXMLRPC() object
>>is more APIish than the internal clientIF object."
>Please do not blame me! ;-)
No body dares to blame the VDSM maintainer! :P
>I do not mind passing an API.Global() that happens to hold an internal
>private reference to _clientIF. I just want that if we find the way to
>obliterate clientIF, we won't need to send a patch to MOM, too.
It seems I misunderstood your previous comments. I am sorry for that.
I'm even sorrier for not being clear enough!
>>So I try to remove cif from API definition to make MOM can
call the
>>VDSM API without having clientIF.
>I do not understand - MOM could receive an API object, it does
not have
>to construct it by itself.
yes, we can pass an API.Global() instance to MOM, but MOM also needs
to create API.VM instance. Without clientIF, I could pass a wrapped
VM class
to MOM. But it's ugly.
I admit it is not pretty. How about adding factory functions such as
Global.constructVm() ? I think it would appease the no-singleton gods.
So I try to remove clientIF in API exposed
interface, and make the caller can use vdsm API just after importing
the module. It could be cleaner. What's your opinion?
Thanks a lot!!
Thank you. Either way I kinda fear the first out-of-prject dependecy on
API.py. The latter API is not yet stable nor complete. Brace yourself to
guard against surprises, and to move along as the API changes. I cannot
stress enough the need for a unit test for every functionality required
by mom.
Regards,
Dan.
10:02 a.m.
If you don't want to add it in a parameter then you already suspect that you are doing
something wrong. Using a singleton instead of passing a parameter doesn't make the
dependency not there. It's just obscures it.
I might not fully understand what you want to do but I think what you want is to have MOM
expect a certain interface.
Then have an adapter class bridging the two interfaces.
The pass the wrapped CIF to MOM.
----- Original Message -----
From: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>
To: vdsm-devel(a)lists.fedorahosted.org
Cc: "Dan Kenigsberg" <danken(a)redhat.com>, "Saggi Mizrahi"
<smizrahi(a)redhat.com>, "Adam Litke" <agl(a)us.ibm.com>, "Ryan
Harper" <ryanh(a)us.ibm.com>
Sent: Wednesday, May 30, 2012 10:49:29 AM
Subject: VDSM API/clientIF instance design issue
Hi Guys,
Recently, I has been working on integrate MOM into VDSM. MOM needs
to
use VDSM API to interact with it. But currently, it requires the
instance of clientIF to use vdsm API. Passing clientIF to MOM is not
a
good choice since it's a vdsm internal object. So I try to remove
the
parameter 'cif' from the interface definition and change to access
the
globally unique clientIF instance in API.py.
To get the instance of clientIF, I add a decorator to clientIF to
change it into singleton. Actually, clientIF has been working as a
global single instance already. We just don't have an interface to
get
it and so passing it as parameter instead. I think using singleton
to
get the instance of clientIF is more clean.
Dan and Saggi already gave some comments in
http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But
I
think we need more discussion on it, so I post it here because
gerrit
is not the appropriate to discuss a design issue.
Thanks !
Mark.
1:53 p.m.
On Wed, May 30, 2012 at 11:02:50AM -0400, Saggi Mizrahi wrote:
If you don't want to add it in a parameter then you already
suspect that you are doing something wrong. Using a singleton instead of passing a
parameter doesn't make the dependency not there. It's just obscures it.
Right. Passing around an internal 'singleton' (whether wrapped or not) is not
the correct course.
I might not fully understand what you want to do but I think what you
want is to have MOM expect a certain interface.
Then have an adapter class bridging the two interfaces.
The pass the wrapped CIF to MOM.
It seems that your suggestion obscures the dependency at least as badly as a
singleton interface does. Today clientIF is a singleton class in all but the
name. It is basically a global container that stores global objects
(vmContainer, locks, etc) and global threads (irs, hostStatsThread, etc). The
fact that a reference gets passed around doesn't really make it any cleaner. We
can all agree that clientIF (in its current form) has overstayed its welcome.
Mark and Xu He Jie have some ideas on how to remove it and plan to tackle that
work. That being said, MOM is an important feature for oVirt 3.1. Do we really
want it to drop out of the release in order to focus on clientIF cleanups (which
certainly will not make 3.1)?
Using a singleton simply means that API objects can be instantiated without the
caller needing to be aware of clientIF at all. In my opinion, this is a cleanup
because we are free to refactor clientIF without affecting the bindings or MOM
one bit.
----- Original Message -----
> From: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>
> To: vdsm-devel(a)lists.fedorahosted.org
> Cc: "Dan Kenigsberg" <danken(a)redhat.com>, "Saggi Mizrahi"
<smizrahi(a)redhat.com>, "Adam Litke" <agl(a)us.ibm.com>, "Ryan
> Harper" <ryanh(a)us.ibm.com>
> Sent: Wednesday, May 30, 2012 10:49:29 AM
> Subject: VDSM API/clientIF instance design issue
>
> Hi Guys,
>
> Recently, I has been working on integrate MOM into VDSM. MOM needs
> to
> use VDSM API to interact with it. But currently, it requires the
> instance of clientIF to use vdsm API. Passing clientIF to MOM is not
> a
> good choice since it's a vdsm internal object. So I try to remove
> the
> parameter 'cif' from the interface definition and change to access
> the
> globally unique clientIF instance in API.py.
>
> To get the instance of clientIF, I add a decorator to clientIF to
> change it into singleton. Actually, clientIF has been working as a
> global single instance already. We just don't have an interface to
> get
> it and so passing it as parameter instead. I think using singleton
> to
> get the instance of clientIF is more clean.
>
> Dan and Saggi already gave some comments in
> http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But
> I
> think we need more discussion on it, so I post it here because
> gerrit
> is not the appropriate to discuss a design issue.
>
> Thanks !
> Mark.
>
>
--
Adam Litke <agl(a)us.ibm.com>
IBM Linux Technology Center
2 p.m.
On Wed, May 30, 2012 at 11:02:50AM -0400, Saggi Mizrahi wrote:
If you don't want to add it in a parameter then you already
suspect that you are doing something wrong. Using a singleton instead of passing a
parameter doesn't make the dependency not there. It's just obscures it.
Right. Passing around an internal 'singleton' (whether wrapped or not) is not
the correct course.
I might not fully understand what you want to do but I think what you
want is to have MOM expect a certain interface.
Then have an adapter class bridging the two interfaces.
The pass the wrapped CIF to MOM.
It seems that your suggestion obscures the dependency at least as badly as a
singleton interface does. Today clientIF is a singleton class in all but the
name. It is basically a global container that stores global objects
(vmContainer, locks, etc) and global threads (irs, hostStatsThread, etc). The
fact that a reference gets passed around doesn't really make it any cleaner. We
can all agree that clientIF (in its current form) has overstayed its welcome.
Mark and Xu He Jie have some ideas on how to remove it and plan to tackle that
work. That being said, MOM is an important feature for oVirt 3.1. Do we really
want it to drop out of the release in order to focus on clientIF cleanups (which
certainly will not make 3.1)?
Using a singleton simply means that API objects can be instantiated without the
caller needing to be aware of clientIF at all. In my opinion, this is a cleanup
because we are free to refactor clientIF without affecting the bindings or MOM
one bit.
----- Original Message -----
> From: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>
> To: vdsm-devel(a)lists.fedorahosted.org
> Cc: "Dan Kenigsberg" <danken(a)redhat.com>, "Saggi Mizrahi"
<smizrahi(a)redhat.com>, "Adam Litke" <agl(a)us.ibm.com>, "Ryan
> Harper" <ryanh(a)us.ibm.com>
> Sent: Wednesday, May 30, 2012 10:49:29 AM
> Subject: VDSM API/clientIF instance design issue
>
> Hi Guys,
>
> Recently, I has been working on integrate MOM into VDSM. MOM needs
> to
> use VDSM API to interact with it. But currently, it requires the
> instance of clientIF to use vdsm API. Passing clientIF to MOM is not
> a
> good choice since it's a vdsm internal object. So I try to remove
> the
> parameter 'cif' from the interface definition and change to access
> the
> globally unique clientIF instance in API.py.
>
> To get the instance of clientIF, I add a decorator to clientIF to
> change it into singleton. Actually, clientIF has been working as a
> global single instance already. We just don't have an interface to
> get
> it and so passing it as parameter instead. I think using singleton
> to
> get the instance of clientIF is more clean.
>
> Dan and Saggi already gave some comments in
> http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But
> I
> think we need more discussion on it, so I post it here because
> gerrit
> is not the appropriate to discuss a design issue.
>
> Thanks !
> Mark.
>
>
--
Adam Litke <agl(a)us.ibm.com>
IBM Linux Technology Center
10:26 a.m.
Do I get it right that host-side of MOM communicates with guest side
over network interface? If so, why isn't it following best practices of
qemu/kvm world and why it doesn't communicate over its own virtio-serial
port?
David
Mark Wu píše v St 30. 05. 2012 v 22:49 +0800:
Hi Guys,
Recently, I has been working on integrate MOM into VDSM. MOM needs to
use VDSM API to interact with it. But currently, it requires the
instance of clientIF to use vdsm API. Passing clientIF to MOM is not a
good choice since it's a vdsm internal object. So I try to remove the
parameter 'cif' from the interface definition and change to access the
globally unique clientIF instance in API.py.
To get the instance of clientIF, I add a decorator to clientIF to
change it into singleton. Actually, clientIF has been working as a
global single instance already. We just don't have an interface to get
it and so passing it as parameter instead. I think using singleton to
get the instance of clientIF is more clean.
Dan and Saggi already gave some comments in
http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But I
think we need more discussion on it, so I post it here because gerrit
is not the appropriate to discuss a design issue.
Thanks !
Mark.
_______________________________________________
vdsm-devel mailing list
vdsm-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-devel
--
David Jaša, RHCE
SPICE QE based in Brno
GPG Key: 22C33E24
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24
2:07 p.m.
On Wed, May 30, 2012 at 05:26:47PM +0200, David Jaša wrote:
Do I get it right that host-side of MOM communicates with guest side
over network interface? If so, why isn't it following best practices of
qemu/kvm world and why it doesn't communicate over its own virtio-serial
port?
No that is not correct for oVirt's usage of MOM. MOM has several ways of
acquiring guest statistics: Client/server over TCP, libvirt (via.
virDomainMemoryStats), qemu-ga (client/server over virtio-serial), etc. For
vdsm, memory stats are acquired via the vdsm api itself (API.py:VM.getStats()).
VDSM acquires these stats via the ovirt-guest-agent over virto-serial.
David
Mark Wu píše v St 30. 05. 2012 v 22:49 +0800:
> Hi Guys,
>
> Recently, I has been working on integrate MOM into VDSM. MOM needs to
> use VDSM API to interact with it. But currently, it requires the
> instance of clientIF to use vdsm API. Passing clientIF to MOM is not a
> good choice since it's a vdsm internal object. So I try to remove the
> parameter 'cif' from the interface definition and change to access the
> globally unique clientIF instance in API.py.
>
> To get the instance of clientIF, I add a decorator to clientIF to
> change it into singleton. Actually, clientIF has been working as a
> global single instance already. We just don't have an interface to get
> it and so passing it as parameter instead. I think using singleton to
> get the instance of clientIF is more clean.
>
> Dan and Saggi already gave some comments in
> http://gerrit.ovirt.org/#change,4839 Thanks for the reviewing! But I
> think we need more discussion on it, so I post it here because gerrit
> is not the appropriate to discuss a design issue.
>
> Thanks !
> Mark.
>
> _______________________________________________
> vdsm-devel mailing list
> vdsm-devel(a)lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/vdsm-devel
--
David Jaša, RHCE
SPICE QE based in Brno
GPG Key: 22C33E24
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24
_______________________________________________
vdsm-devel mailing list
vdsm-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-devel
--
Adam Litke <agl(a)us.ibm.com>
IBM Linux Technology Center
4370
days inactive
4372
days old
vdsm-devel@lists.fedorahosted.org
11 comments
5 participants
participants (5)
-
agl＠us.ibm.com -
Dan Kenigsberg -
David Jaša -
smizrahi＠redhat.com -
wudxw＠linux.vnet.ibm.com