10:18 a.m.
Hi All,
Please express your opinion if you have any -
Currently Disk BE has a plugged property, which should be a property of the relationship
between vm(or template) and a disk.
I plan to remove this property from the Disk entity, and add new entity, called
DeviceDisk.
This should inherit from Disk and contain the vm/template guid and the plugged property at
first round.
At second round it'll also contain the readOnly property, for RO disks, TBD right
after.
Appreciate any input,
Vered
10:22 a.m.
Vered,
VmDevice has "isPlugged" field,
Why not have somehow in your inheritence (either Disk or a subclass) a field :
"VmDevice device"
and have isPlugged method called "device.isPlugged()" ?
Then you can also add the readOnly property which is not represented at VmDevice.
Does this sound logical to you?
----- Original Message -----
From: "Vered Volansky" <vered(a)redhat.com>
To: engine-devel(a)ovirt.org
Sent: Monday, May 27, 2013 6:18:58 PM
Subject: [Engine-devel] Disk BE very small refactoring
Hi All,
Please express your opinion if you have any -
Currently Disk BE has a plugged property, which should be a property of the
relationship between vm(or template) and a disk.
I plan to remove this property from the Disk entity, and add new entity,
called DeviceDisk.
This should inherit from Disk and contain the vm/template guid and the
plugged property at first round.
At second round it'll also contain the readOnly property, for RO disks, TBD
right after.
Appreciate any input,
Vered
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
10:42 a.m.
----- Original Message -----
From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
To: "Vered Volansky" <vered(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Monday, May 27, 2013 6:22:58 PM
Subject: Re: [Engine-devel] Disk BE very small refactoring
Vered,
VmDevice has "isPlugged" field,
Why not have somehow in your inheritence (either Disk or a subclass) a field
: "VmDevice device"
and have isPlugged method called "device.isPlugged()" ?
Then you can also add the readOnly property which is not represented at
VmDevice.
Does this sound logical to you?
Vered, I forgot sharable disk may be associated with two VMs, hence having a field of
VmDevice may be problematic.
However, maybe we need some interface (i.e - Device) which BothVmDevice and your class (or
maybe an object of some class that will be contained in your class) will implement?
----- Original Message -----
> From: "Vered Volansky" <vered(a)redhat.com>
> To: engine-devel(a)ovirt.org
> Sent: Monday, May 27, 2013 6:18:58 PM
> Subject: [Engine-devel] Disk BE very small refactoring
>
> Hi All,
>
> Please express your opinion if you have any -
>
> Currently Disk BE has a plugged property, which should be a property of the
> relationship between vm(or template) and a disk.
> I plan to remove this property from the Disk entity, and add new entity,
> called DeviceDisk.
> This should inherit from Disk and contain the vm/template guid and the
> plugged property at first round.
> At second round it'll also contain the readOnly property, for RO disks, TBD
> right after.
>
> Appreciate any input,
> Vered
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:09 a.m.
----- Original Message -----
----- Original Message -----
> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> To: "Vered Volansky" <vered(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Monday, May 27, 2013 6:22:58 PM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
> Vered,
> VmDevice has "isPlugged" field,
> Why not have somehow in your inheritence (either Disk or a subclass) a
> field
> : "VmDevice device"
> and have isPlugged method called "device.isPlugged()" ?
>
> Then you can also add the readOnly property which is not represented at
> VmDevice.
>
>
> Does this sound logical to you?
Vered, I forgot sharable disk may be associated with two VMs, hence having a
field of VmDevice may be problematic.
In this case there will be 2 VmDevice entities for the same disk, IIUC.
Why not just rely on the field there? Why does it need to be "flattened" on the
Disk BE?
However, maybe we need some interface (i.e - Device) which
BothVmDevice and
your class (or maybe an object of some class that will be contained in your
class) will implement?
>
> ----- Original Message -----
> > From: "Vered Volansky" <vered(a)redhat.com>
> > To: engine-devel(a)ovirt.org
> > Sent: Monday, May 27, 2013 6:18:58 PM
> > Subject: [Engine-devel] Disk BE very small refactoring
> >
> > Hi All,
> >
> > Please express your opinion if you have any -
> >
> > Currently Disk BE has a plugged property, which should be a property of
> > the
> > relationship between vm(or template) and a disk.
> > I plan to remove this property from the Disk entity, and add new entity,
> > called DeviceDisk.
> > This should inherit from Disk and contain the vm/template guid and the
> > plugged property at first round.
> > At second round it'll also contain the readOnly property, for RO disks,
> > TBD
> > right after.
> >
> > Appreciate any input,
> > Vered
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:24 a.m.
----- Original Message -----
From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
To: "Vered Volansky" <vered(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Monday, May 27, 2013 6:22:58 PM
Subject: Re: [Engine-devel] Disk BE very small refactoring
Vered,
VmDevice has "isPlugged" field,
Why not have somehow in your inheritence (either Disk or a subclass) a field
: "VmDevice device"
disk id is the device id, no need for field to represent the relation.
the combination of disk-id and a specific entity (vm/template) will get you the other
info
and have isPlugged method called "device.isPlugged()" ?
Then you can also add the readOnly property which is not represented at
VmDevice.
Does this sound logical to you?
----- Original Message -----
> From: "Vered Volansky" <vered(a)redhat.com>
> To: engine-devel(a)ovirt.org
> Sent: Monday, May 27, 2013 6:18:58 PM
> Subject: [Engine-devel] Disk BE very small refactoring
>
> Hi All,
>
> Please express your opinion if you have any -
>
> Currently Disk BE has a plugged property, which should be a property of the
> relationship between vm(or template) and a disk.
> I plan to remove this property from the Disk entity, and add new entity,
> called DeviceDisk.
> This should inherit from Disk and contain the vm/template guid and the
> plugged property at first round.
> At second round it'll also contain the readOnly property, for RO disks, TBD
> right after.
>
> Appreciate any input,
> Vered
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
5:49 a.m.
I think the main problem is that we abuse the business entity to act as
an O/R mapping class to the DB and also to be used as a business entity
for presentation purposes.
I understand how Yair thought isPlugged could be fetched from vmDevice,
this is a confusing design, and it is just one example and more to come.
I suggest that if we already thinking of changing the class hierarchy,
we can start by implementing a package for presentation classes for
transient classes such as this instead enforcing complex hierarchy.
The query class will fetch all the data from the DB and initialized the
transient class and send it to the client.
I think it could be a good start and will solve many issues we might
encounter in the future.
Regards,
Maor
On 05/28/2013 11:24 AM, Omer Frenkel wrote:
----- Original Message -----
> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> To: "Vered Volansky" <vered(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Monday, May 27, 2013 6:22:58 PM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
> Vered,
> VmDevice has "isPlugged" field,
> Why not have somehow in your inheritence (either Disk or a subclass) a field
> : "VmDevice device"
disk id is the device id, no need for field to represent the relation.
the combination of disk-id and a specific entity (vm/template) will get you the other
info
> and have isPlugged method called "device.isPlugged()" ?
>
> Then you can also add the readOnly property which is not represented at
> VmDevice.
>
>
> Does this sound logical to you?
>
> ----- Original Message -----
>> From: "Vered Volansky" <vered(a)redhat.com>
>> To: engine-devel(a)ovirt.org
>> Sent: Monday, May 27, 2013 6:18:58 PM
>> Subject: [Engine-devel] Disk BE very small refactoring
>>
>> Hi All,
>>
>> Please express your opinion if you have any -
>>
>> Currently Disk BE has a plugged property, which should be a property of the
>> relationship between vm(or template) and a disk.
>> I plan to remove this property from the Disk entity, and add new entity,
>> called DeviceDisk.
>> This should inherit from Disk and contain the vm/template guid and the
>> plugged property at first round.
>> At second round it'll also contain the readOnly property, for RO disks, TBD
>> right after.
>>
>> Appreciate any input,
>> Vered
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
12:40 p.m.
I had a problem, didn't see all the replies till now.
I'll add some more info as to why we want to do this like we do.
It all started from adding the readOnly property to disks. It should have been handled
like plugged is handled, yet plugged is a hack, and if we don't change that now,
we'll only keep on adding unreasonable hacks.
So from the start -
Disk currently:
- Sometimes represents a disk in a vm context and sometimes not.
- Holds plugged property, which is only relevant when a disk is in a vm context, which
already suggests this is not the natural place for it.
- Also holds bootable and interface, which cause limitations of use, but are not so
obviously related to the relationship between Disk and Vm as plugged.
- Can be shared between several vm's, to some plugged and to some not plugged.
- Will soon be optionally RO in one VM and RW in another, which is exactly the same as
plugged, and therefore plugged issue should be fixed first.
Every column in that shows a disk in the UI receives a Disk entity, and show its contents,
while plugged/unplugged is ignored when not in a VM context.
The way things are now, using a VmDevice in the where we need it to show plug status,
we'll also have to use it in all other columns, which is irrelevant and just totally
not related.
So using VmDevice in UI is a no go.
The UI is the main limitation forcing us to use something that extends Disk, and what I
described below is the easiest thing to implement in the time restrictions we have without
changing the entire system.
I think this answers all the questions not already answered by others.
Regarding Maor suggestion - might be a good idea, but not in this scope or time-frame.
If there is any other/unanswered issue or objection to the design change please share.
I appreciate your inputs,
Vered
----- Original Message -----
From: "Maor Lipchuk" <mlipchuk(a)redhat.com>
To: "Vered Volansky" <vered(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Tuesday, May 28, 2013 1:49:46 PM
Subject: Re: [Engine-devel] Disk BE very small refactoring
I think the main problem is that we abuse the business entity to act as
an O/R mapping class to the DB and also to be used as a business entity
for presentation purposes.
I understand how Yair thought isPlugged could be fetched from vmDevice,
this is a confusing design, and it is just one example and more to come.
I suggest that if we already thinking of changing the class hierarchy,
we can start by implementing a package for presentation classes for
transient classes such as this instead enforcing complex hierarchy.
The query class will fetch all the data from the DB and initialized the
transient class and send it to the client.
I think it could be a good start and will solve many issues we might
encounter in the future.
Regards,
Maor
On 05/28/2013 11:24 AM, Omer Frenkel wrote:
>
>
> ----- Original Message -----
>> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
>> To: "Vered Volansky" <vered(a)redhat.com>
>> Cc: engine-devel(a)ovirt.org
>> Sent: Monday, May 27, 2013 6:22:58 PM
>> Subject: Re: [Engine-devel] Disk BE very small refactoring
>>
>> Vered,
>> VmDevice has "isPlugged" field,
>> Why not have somehow in your inheritence (either Disk or a subclass) a
>> field
>> : "VmDevice device"
>
> disk id is the device id, no need for field to represent the relation.
> the combination of disk-id and a specific entity (vm/template) will get you
> the other info
>
>> and have isPlugged method called "device.isPlugged()" ?
>>
>> Then you can also add the readOnly property which is not represented at
>> VmDevice.
>>
>>
>> Does this sound logical to you?
>>
>> ----- Original Message -----
>>> From: "Vered Volansky" <vered(a)redhat.com>
>>> To: engine-devel(a)ovirt.org
>>> Sent: Monday, May 27, 2013 6:18:58 PM
>>> Subject: [Engine-devel] Disk BE very small refactoring
>>>
>>> Hi All,
>>>
>>> Please express your opinion if you have any -
>>>
>>> Currently Disk BE has a plugged property, which should be a property of
>>> the
>>> relationship between vm(or template) and a disk.
>>> I plan to remove this property from the Disk entity, and add new entity,
>>> called DeviceDisk.
>>> This should inherit from Disk and contain the vm/template guid and the
>>> plugged property at first round.
>>> At second round it'll also contain the readOnly property, for RO disks,
>>> TBD
>>> right after.
>>>
>>> Appreciate any input,
>>> Vered
>>> _______________________________________________
>>> Engine-devel mailing list
>>> Engine-devel(a)ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:29 a.m.
----- Original Message -----
From: "Vered Volansky" <vered(a)redhat.com>
To: "Maor Lipchuk" <mlipchuk(a)redhat.com>, "Yair Zaslavsky"
<yzaslavs(a)redhat.com>, "Omer Frenkel"
<ofrenkel(a)redhat.com>, "Mike Kolesnik" <mkolesni(a)redhat.com>,
"Allon Mureinik" <amureini(a)redhat.com>, "Daniel Erez"
<derez(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Tuesday, May 28, 2013 8:40:38 PM
Subject: Re: [Engine-devel] Disk BE very small refactoring
I had a problem, didn't see all the replies till now.
I'll add some more info as to why we want to do this like we do.
It all started from adding the readOnly property to disks. It should have
been handled like plugged is handled, yet plugged is a hack, and if we don't
change that now, we'll only keep on adding unreasonable hacks.
please explain why you think plugged is a hack? what is wrong with it?
So from the start -
Disk currently:
- Sometimes represents a disk in a vm context and sometimes not.
- Holds plugged property, which is only relevant when a disk is in a vm
context, which already suggests this is not the natural place for it.
- Also holds bootable and interface, which cause limitations of use, but are
not so obviously related to the relationship between Disk and Vm as plugged.
- Can be shared between several vm's, to some plugged and to some not
plugged.
- Will soon be optionally RO in one VM and RW in another, which is exactly
the same as plugged, and therefore plugged issue should be fixed first.
Every column in that shows a disk in the UI receives a Disk entity, and show
its contents, while plugged/unplugged is ignored when not in a VM context.
The way things are now, using a VmDevice in the where we need it to show plug
status, we'll also have to use it in all other columns, which is irrelevant
and just totally not related.
So using VmDevice in UI is a no go.
The UI is the main limitation forcing us to use something that extends Disk,
and what I described below is the easiest thing to implement in the time
restrictions we have without changing the entire system.
I think this answers all the questions not already answered by others.
Regarding Maor suggestion - might be a good idea, but not in this scope or
time-frame.
If there is any other/unanswered issue or objection to the design change
please share.
I appreciate your inputs,
Vered
sorry i didnt understand why the current disk object isnt good enough,
you get a disk, some of its properties are valid only in some situations.
i think its easier to use instead of couple of different objects representing the same
entity in different situations.
----- Original Message -----
> From: "Maor Lipchuk" <mlipchuk(a)redhat.com>
> To: "Vered Volansky" <vered(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Tuesday, May 28, 2013 1:49:46 PM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
> I think the main problem is that we abuse the business entity to act as
> an O/R mapping class to the DB and also to be used as a business entity
> for presentation purposes.
>
> I understand how Yair thought isPlugged could be fetched from vmDevice,
> this is a confusing design, and it is just one example and more to come.
>
> I suggest that if we already thinking of changing the class hierarchy,
> we can start by implementing a package for presentation classes for
> transient classes such as this instead enforcing complex hierarchy.
>
> The query class will fetch all the data from the DB and initialized the
> transient class and send it to the client.
> I think it could be a good start and will solve many issues we might
> encounter in the future.
>
> Regards,
> Maor
>
> On 05/28/2013 11:24 AM, Omer Frenkel wrote:
> >
> >
> > ----- Original Message -----
> >> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> >> To: "Vered Volansky" <vered(a)redhat.com>
> >> Cc: engine-devel(a)ovirt.org
> >> Sent: Monday, May 27, 2013 6:22:58 PM
> >> Subject: Re: [Engine-devel] Disk BE very small refactoring
> >>
> >> Vered,
> >> VmDevice has "isPlugged" field,
> >> Why not have somehow in your inheritence (either Disk or a subclass) a
> >> field
> >> : "VmDevice device"
> >
> > disk id is the device id, no need for field to represent the relation.
> > the combination of disk-id and a specific entity (vm/template) will get
> > you
> > the other info
> >
> >> and have isPlugged method called "device.isPlugged()" ?
> >>
> >> Then you can also add the readOnly property which is not represented at
> >> VmDevice.
> >>
> >>
> >> Does this sound logical to you?
> >>
> >> ----- Original Message -----
> >>> From: "Vered Volansky" <vered(a)redhat.com>
> >>> To: engine-devel(a)ovirt.org
> >>> Sent: Monday, May 27, 2013 6:18:58 PM
> >>> Subject: [Engine-devel] Disk BE very small refactoring
> >>>
> >>> Hi All,
> >>>
> >>> Please express your opinion if you have any -
> >>>
> >>> Currently Disk BE has a plugged property, which should be a property
of
> >>> the
> >>> relationship between vm(or template) and a disk.
> >>> I plan to remove this property from the Disk entity, and add new
> >>> entity,
> >>> called DeviceDisk.
> >>> This should inherit from Disk and contain the vm/template guid and the
> >>> plugged property at first round.
> >>> At second round it'll also contain the readOnly property, for RO
disks,
> >>> TBD
> >>> right after.
> >>>
> >>> Appreciate any input,
> >>> Vered
> >>> _______________________________________________
> >>> Engine-devel mailing list
> >>> Engine-devel(a)ovirt.org
> >>> http://lists.ovirt.org/mailman/listinfo/engine-devel
> >>>
> >> _______________________________________________
> >> Engine-devel mailing list
> >> Engine-devel(a)ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/engine-devel
> >>
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
>
>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
4:57 a.m.
----- Original Message -----
From: "Omer Frenkel" <ofrenkel(a)redhat.com>
To: "Vered Volansky" <vered(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Thursday, May 30, 2013 11:29:40 AM
Subject: Re: [Engine-devel] Disk BE very small refactoring
----- Original Message -----
> From: "Vered Volansky" <vered(a)redhat.com>
> To: "Maor Lipchuk" <mlipchuk(a)redhat.com>, "Yair
Zaslavsky"
> <yzaslavs(a)redhat.com>, "Omer Frenkel"
> <ofrenkel(a)redhat.com>, "Mike Kolesnik" <mkolesni(a)redhat.com>,
"Allon
> Mureinik" <amureini(a)redhat.com>, "Daniel Erez"
> <derez(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Tuesday, May 28, 2013 8:40:38 PM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
> I had a problem, didn't see all the replies till now.
>
> I'll add some more info as to why we want to do this like we do.
> It all started from adding the readOnly property to disks. It should have
> been handled like plugged is handled, yet plugged is a hack, and if we
> don't
> change that now, we'll only keep on adding unreasonable hacks.
>
please explain why you think plugged is a hack? what is wrong with it?
You have is_readonly and is_plugged in the device level , this is far from being a hack
and managed in the correct place since both may apply to multiple device types (disk, nics
....)
> So from the start -
> Disk currently:
> - Sometimes represents a disk in a vm context and sometimes not.
> - Holds plugged property, which is only relevant when a disk is in a vm
> context, which already suggests this is not the natural place for it.
> - Also holds bootable and interface, which cause limitations of use, but
> are
> not so obviously related to the relationship between Disk and Vm as
> plugged.
> - Can be shared between several vm's, to some plugged and to some not
> plugged.
> - Will soon be optionally RO in one VM and RW in another, which is exactly
> the same as plugged, and therefore plugged issue should be fixed first.
>
> Every column in that shows a disk in the UI receives a Disk entity, and
> show
> its contents, while plugged/unplugged is ignored when not in a VM context.
> The way things are now, using a VmDevice in the where we need it to show
> plug
> status, we'll also have to use it in all other columns, which is irrelevant
> and just totally not related.
> So using VmDevice in UI is a no go.
>
> The UI is the main limitation forcing us to use something that extends
> Disk,
> and what I described below is the easiest thing to implement in the time
> restrictions we have without changing the entire system.
>
> I think this answers all the questions not already answered by others.
> Regarding Maor suggestion - might be a good idea, but not in this scope or
> time-frame.
>
> If there is any other/unanswered issue or objection to the design change
> please share.
>
> I appreciate your inputs,
> Vered
>
sorry i didnt understand why the current disk object isnt good enough,
you get a disk, some of its properties are valid only in some situations.
i think its easier to use instead of couple of different objects representing
the same entity in different situations.
I totally agree with Omer, please specify what is impossible or hard to achieve with the
current schema
>
> ----- Original Message -----
> > From: "Maor Lipchuk" <mlipchuk(a)redhat.com>
> > To: "Vered Volansky" <vered(a)redhat.com>
> > Cc: engine-devel(a)ovirt.org
> > Sent: Tuesday, May 28, 2013 1:49:46 PM
> > Subject: Re: [Engine-devel] Disk BE very small refactoring
> >
> > I think the main problem is that we abuse the business entity to act as
> > an O/R mapping class to the DB and also to be used as a business entity
> > for presentation purposes.
> >
> > I understand how Yair thought isPlugged could be fetched from vmDevice,
> > this is a confusing design, and it is just one example and more to come.
> >
> > I suggest that if we already thinking of changing the class hierarchy,
> > we can start by implementing a package for presentation classes for
> > transient classes such as this instead enforcing complex hierarchy.
> >
> > The query class will fetch all the data from the DB and initialized the
> > transient class and send it to the client.
> > I think it could be a good start and will solve many issues we might
> > encounter in the future.
> >
> > Regards,
> > Maor
> >
> > On 05/28/2013 11:24 AM, Omer Frenkel wrote:
> > >
> > >
> > > ----- Original Message -----
> > >> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> > >> To: "Vered Volansky" <vered(a)redhat.com>
> > >> Cc: engine-devel(a)ovirt.org
> > >> Sent: Monday, May 27, 2013 6:22:58 PM
> > >> Subject: Re: [Engine-devel] Disk BE very small refactoring
> > >>
> > >> Vered,
> > >> VmDevice has "isPlugged" field,
> > >> Why not have somehow in your inheritence (either Disk or a subclass)
a
> > >> field
> > >> : "VmDevice device"
> > >
> > > disk id is the device id, no need for field to represent the relation.
> > > the combination of disk-id and a specific entity (vm/template) will get
> > > you
> > > the other info
> > >
> > >> and have isPlugged method called "device.isPlugged()" ?
> > >>
> > >> Then you can also add the readOnly property which is not represented
> > >> at
> > >> VmDevice.
> > >>
> > >>
> > >> Does this sound logical to you?
> > >>
> > >> ----- Original Message -----
> > >>> From: "Vered Volansky" <vered(a)redhat.com>
> > >>> To: engine-devel(a)ovirt.org
> > >>> Sent: Monday, May 27, 2013 6:18:58 PM
> > >>> Subject: [Engine-devel] Disk BE very small refactoring
> > >>>
> > >>> Hi All,
> > >>>
> > >>> Please express your opinion if you have any -
> > >>>
> > >>> Currently Disk BE has a plugged property, which should be a
property
> > >>> of
> > >>> the
> > >>> relationship between vm(or template) and a disk.
> > >>> I plan to remove this property from the Disk entity, and add new
> > >>> entity,
> > >>> called DeviceDisk.
> > >>> This should inherit from Disk and contain the vm/template guid
and
> > >>> the
> > >>> plugged property at first round.
> > >>> At second round it'll also contain the readOnly property, for
RO
> > >>> disks,
> > >>> TBD
> > >>> right after.
> > >>>
> > >>> Appreciate any input,
> > >>> Vered
> > >>> _______________________________________________
> > >>> Engine-devel mailing list
> > >>> Engine-devel(a)ovirt.org
> > >>> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >>>
> > >> _______________________________________________
> > >> Engine-devel mailing list
> > >> Engine-devel(a)ovirt.org
> > >> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >>
> > > _______________________________________________
> > > Engine-devel mailing list
> > > Engine-devel(a)ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >
> >
> >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
4:02 a.m.
----- Original Message -----
From: "Eli Mesika" <emesika(a)redhat.com>
To: "Vered Volansky" <vered(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Thursday, May 30, 2013 12:57:26 PM
Subject: Re: [Engine-devel] Disk BE very small refactoring
----- Original Message -----
> From: "Omer Frenkel" <ofrenkel(a)redhat.com>
> To: "Vered Volansky" <vered(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Thursday, May 30, 2013 11:29:40 AM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
>
>
> ----- Original Message -----
> > From: "Vered Volansky" <vered(a)redhat.com>
> > To: "Maor Lipchuk" <mlipchuk(a)redhat.com>, "Yair
Zaslavsky"
> > <yzaslavs(a)redhat.com>, "Omer Frenkel"
> > <ofrenkel(a)redhat.com>, "Mike Kolesnik"
<mkolesni(a)redhat.com>, "Allon
> > Mureinik" <amureini(a)redhat.com>, "Daniel Erez"
> > <derez(a)redhat.com>
> > Cc: engine-devel(a)ovirt.org
> > Sent: Tuesday, May 28, 2013 8:40:38 PM
> > Subject: Re: [Engine-devel] Disk BE very small refactoring
> >
> > I had a problem, didn't see all the replies till now.
> >
> > I'll add some more info as to why we want to do this like we do.
> > It all started from adding the readOnly property to disks. It should have
> > been handled like plugged is handled, yet plugged is a hack, and if we
> > don't
> > change that now, we'll only keep on adding unreasonable hacks.
> >
>
> please explain why you think plugged is a hack? what is wrong with it?
You have is_readonly and is_plugged in the device level , this is far from
being a hack and managed in the correct place since both may apply to
multiple device types (disk, nics ....)
There's no problem with device level,
only I can't use VmDevice (which is where this naturally belongs) in UI since it will
cause a major change we don't want right now.
Not only that, VmDevice does hold this info, but not other relevant info used in UI and
available in Disk, so using VmDevice on it's own is not only to extensive a change,
but also by far will not end there.
>
> > So from the start -
> > Disk currently:
> > - Sometimes represents a disk in a vm context and sometimes not.
> > - Holds plugged property, which is only relevant when a disk is in a vm
> > context, which already suggests this is not the natural place for it.
> > - Also holds bootable and interface, which cause limitations of use, but
> > are
> > not so obviously related to the relationship between Disk and Vm as
> > plugged.
> > - Can be shared between several vm's, to some plugged and to some not
> > plugged.
> > - Will soon be optionally RO in one VM and RW in another, which is
> > exactly
> > the same as plugged, and therefore plugged issue should be fixed first.
> >
> > Every column in that shows a disk in the UI receives a Disk entity, and
> > show
> > its contents, while plugged/unplugged is ignored when not in a VM
> > context.
> > The way things are now, using a VmDevice in the where we need it to show
> > plug
> > status, we'll also have to use it in all other columns, which is
> > irrelevant
> > and just totally not related.
> > So using VmDevice in UI is a no go.
> >
> > The UI is the main limitation forcing us to use something that extends
> > Disk,
> > and what I described below is the easiest thing to implement in the time
> > restrictions we have without changing the entire system.
> >
> > I think this answers all the questions not already answered by others.
> > Regarding Maor suggestion - might be a good idea, but not in this scope
> > or
> > time-frame.
> >
> > If there is any other/unanswered issue or objection to the design change
> > please share.
> >
> > I appreciate your inputs,
> > Vered
> >
>
> sorry i didnt understand why the current disk object isnt good enough,
> you get a disk, some of its properties are valid only in some situations.
> i think its easier to use instead of couple of different objects
> representing
> the same entity in different situations.
I totally agree with Omer, please specify what is impossible or hard to
achieve with the current schema
No problem with DB schema, please see my answer in
engine-devel, I'm not sure you're directly cc'ed.
>
> >
> > ----- Original Message -----
> > > From: "Maor Lipchuk" <mlipchuk(a)redhat.com>
> > > To: "Vered Volansky" <vered(a)redhat.com>
> > > Cc: engine-devel(a)ovirt.org
> > > Sent: Tuesday, May 28, 2013 1:49:46 PM
> > > Subject: Re: [Engine-devel] Disk BE very small refactoring
> > >
> > > I think the main problem is that we abuse the business entity to act as
> > > an O/R mapping class to the DB and also to be used as a business entity
> > > for presentation purposes.
> > >
> > > I understand how Yair thought isPlugged could be fetched from vmDevice,
> > > this is a confusing design, and it is just one example and more to
> > > come.
> > >
> > > I suggest that if we already thinking of changing the class hierarchy,
> > > we can start by implementing a package for presentation classes for
> > > transient classes such as this instead enforcing complex hierarchy.
> > >
> > > The query class will fetch all the data from the DB and initialized the
> > > transient class and send it to the client.
> > > I think it could be a good start and will solve many issues we might
> > > encounter in the future.
> > >
> > > Regards,
> > > Maor
> > >
> > > On 05/28/2013 11:24 AM, Omer Frenkel wrote:
> > > >
> > > >
> > > > ----- Original Message -----
> > > >> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> > > >> To: "Vered Volansky" <vered(a)redhat.com>
> > > >> Cc: engine-devel(a)ovirt.org
> > > >> Sent: Monday, May 27, 2013 6:22:58 PM
> > > >> Subject: Re: [Engine-devel] Disk BE very small refactoring
> > > >>
> > > >> Vered,
> > > >> VmDevice has "isPlugged" field,
> > > >> Why not have somehow in your inheritence (either Disk or a
subclass)
> > > >> a
> > > >> field
> > > >> : "VmDevice device"
> > > >
> > > > disk id is the device id, no need for field to represent the
> > > > relation.
> > > > the combination of disk-id and a specific entity (vm/template) will
> > > > get
> > > > you
> > > > the other info
> > > >
> > > >> and have isPlugged method called "device.isPlugged()"
?
> > > >>
> > > >> Then you can also add the readOnly property which is not
represented
> > > >> at
> > > >> VmDevice.
> > > >>
> > > >>
> > > >> Does this sound logical to you?
> > > >>
> > > >> ----- Original Message -----
> > > >>> From: "Vered Volansky" <vered(a)redhat.com>
> > > >>> To: engine-devel(a)ovirt.org
> > > >>> Sent: Monday, May 27, 2013 6:18:58 PM
> > > >>> Subject: [Engine-devel] Disk BE very small refactoring
> > > >>>
> > > >>> Hi All,
> > > >>>
> > > >>> Please express your opinion if you have any -
> > > >>>
> > > >>> Currently Disk BE has a plugged property, which should be a
> > > >>> property
> > > >>> of
> > > >>> the
> > > >>> relationship between vm(or template) and a disk.
> > > >>> I plan to remove this property from the Disk entity, and add
new
> > > >>> entity,
> > > >>> called DeviceDisk.
> > > >>> This should inherit from Disk and contain the vm/template
guid and
> > > >>> the
> > > >>> plugged property at first round.
> > > >>> At second round it'll also contain the readOnly property,
for RO
> > > >>> disks,
> > > >>> TBD
> > > >>> right after.
> > > >>>
> > > >>> Appreciate any input,
> > > >>> Vered
> > > >>> _______________________________________________
> > > >>> Engine-devel mailing list
> > > >>> Engine-devel(a)ovirt.org
> > > >>> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > >>>
> > > >> _______________________________________________
> > > >> Engine-devel mailing list
> > > >> Engine-devel(a)ovirt.org
> > > >> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > >>
> > > > _______________________________________________
> > > > Engine-devel mailing list
> > > > Engine-devel(a)ovirt.org
> > > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > >
> > >
> > >
> > > _______________________________________________
> > > Engine-devel mailing list
> > > Engine-devel(a)ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
4:19 a.m.
----- Original Message -----
From: "Vered Volansky" <vered(a)redhat.com>
To: "Eli Mesika" <emesika(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Sunday, June 2, 2013 12:02:55 PM
Subject: Re: [Engine-devel] Disk BE very small refactoring
----- Original Message -----
> From: "Eli Mesika" <emesika(a)redhat.com>
> To: "Vered Volansky" <vered(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Thursday, May 30, 2013 12:57:26 PM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
>
>
> ----- Original Message -----
> > From: "Omer Frenkel" <ofrenkel(a)redhat.com>
> > To: "Vered Volansky" <vered(a)redhat.com>
> > Cc: engine-devel(a)ovirt.org
> > Sent: Thursday, May 30, 2013 11:29:40 AM
> > Subject: Re: [Engine-devel] Disk BE very small refactoring
> >
> >
> >
> > ----- Original Message -----
> > > From: "Vered Volansky" <vered(a)redhat.com>
> > > To: "Maor Lipchuk" <mlipchuk(a)redhat.com>, "Yair
Zaslavsky"
> > > <yzaslavs(a)redhat.com>, "Omer Frenkel"
> > > <ofrenkel(a)redhat.com>, "Mike Kolesnik"
<mkolesni(a)redhat.com>, "Allon
> > > Mureinik" <amureini(a)redhat.com>, "Daniel Erez"
> > > <derez(a)redhat.com>
> > > Cc: engine-devel(a)ovirt.org
> > > Sent: Tuesday, May 28, 2013 8:40:38 PM
> > > Subject: Re: [Engine-devel] Disk BE very small refactoring
> > >
> > > I had a problem, didn't see all the replies till now.
> > >
> > > I'll add some more info as to why we want to do this like we do.
> > > It all started from adding the readOnly property to disks. It should
> > > have
> > > been handled like plugged is handled, yet plugged is a hack, and if we
> > > don't
> > > change that now, we'll only keep on adding unreasonable hacks.
> > >
> >
> > please explain why you think plugged is a hack? what is wrong with it?
>
> You have is_readonly and is_plugged in the device level , this is far from
> being a hack and managed in the correct place since both may apply to
> multiple device types (disk, nics ....)
There's no problem with device level, only I can't use VmDevice (which is
where this naturally belongs) in UI since it will cause a major change we
don't want right now.
Not only that, VmDevice does hold this info, but not other relevant info used
in UI and available in Disk, so using VmDevice on it's own is not only to
extensive a change, but also by far will not end there.
>
> >
> > > So from the start -
> > > Disk currently:
> > > - Sometimes represents a disk in a vm context and sometimes not.
> > > - Holds plugged property, which is only relevant when a disk is in a vm
> > > context, which already suggests this is not the natural place for it.
> > > - Also holds bootable and interface, which cause limitations of use,
> > > but
> > > are
> > > not so obviously related to the relationship between Disk and Vm as
> > > plugged.
> > > - Can be shared between several vm's, to some plugged and to some not
> > > plugged.
> > > - Will soon be optionally RO in one VM and RW in another, which is
> > > exactly
> > > the same as plugged, and therefore plugged issue should be fixed first.
> > >
> > > Every column in that shows a disk in the UI receives a Disk entity, and
> > > show
> > > its contents, while plugged/unplugged is ignored when not in a VM
> > > context.
> > > The way things are now, using a VmDevice in the where we need it to
> > > show
> > > plug
> > > status, we'll also have to use it in all other columns, which is
> > > irrelevant
> > > and just totally not related.
> > > So using VmDevice in UI is a no go.
> > >
> > > The UI is the main limitation forcing us to use something that extends
> > > Disk,
> > > and what I described below is the easiest thing to implement in the
> > > time
> > > restrictions we have without changing the entire system.
> > >
> > > I think this answers all the questions not already answered by others.
> > > Regarding Maor suggestion - might be a good idea, but not in this scope
> > > or
> > > time-frame.
> > >
> > > If there is any other/unanswered issue or objection to the design
> > > change
> > > please share.
> > >
> > > I appreciate your inputs,
> > > Vered
> > >
> >
> > sorry i didnt understand why the current disk object isnt good enough,
> > you get a disk, some of its properties are valid only in some situations.
> > i think its easier to use instead of couple of different objects
> > representing
> > the same entity in different situations.
>
> I totally agree with Omer, please specify what is impossible or hard to
> achieve with the current schema
No problem with DB schema, please see my answer in engine-devel, I'm not sure
you're directly cc'ed.
The way I see it, it's a design issue.
A disk is different from other devices as it can be attached to multiple VMs.
I.e., the same Disk record in the database is associated with either 0 device records (a
floating disk), 1 device record (a normal disk) or multiple device records (shared disk).
The java object has a few properties that really belong to the disk (e.g., alias, size,
wipeAfterDelete), some that belong to the device (plugged, readOnly in the near future)
and some the belong to ALL the VMs it's attached to (e.g., vmNames).
This way, the object is unusable - depending on which query you use to load it, you'll
have a different set of properties set to null, which is unacceptable in modular code.
The suggestion here is simple - instead of needing to comment or somehow no what
properties a function is allowed to use, make it explicit my moving these properties to a
sensible object.
>
> >
> > >
> > > ----- Original Message -----
> > > > From: "Maor Lipchuk" <mlipchuk(a)redhat.com>
> > > > To: "Vered Volansky" <vered(a)redhat.com>
> > > > Cc: engine-devel(a)ovirt.org
> > > > Sent: Tuesday, May 28, 2013 1:49:46 PM
> > > > Subject: Re: [Engine-devel] Disk BE very small refactoring
> > > >
> > > > I think the main problem is that we abuse the business entity to act
> > > > as
> > > > an O/R mapping class to the DB and also to be used as a business
> > > > entity
> > > > for presentation purposes.
> > > >
> > > > I understand how Yair thought isPlugged could be fetched from
> > > > vmDevice,
> > > > this is a confusing design, and it is just one example and more to
> > > > come.
> > > >
> > > > I suggest that if we already thinking of changing the class
> > > > hierarchy,
> > > > we can start by implementing a package for presentation classes for
> > > > transient classes such as this instead enforcing complex hierarchy.
> > > >
> > > > The query class will fetch all the data from the DB and initialized
> > > > the
> > > > transient class and send it to the client.
> > > > I think it could be a good start and will solve many issues we might
> > > > encounter in the future.
> > > >
> > > > Regards,
> > > > Maor
> > > >
> > > > On 05/28/2013 11:24 AM, Omer Frenkel wrote:
> > > > >
> > > > >
> > > > > ----- Original Message -----
> > > > >> From: "Yair Zaslavsky"
<yzaslavs(a)redhat.com>
> > > > >> To: "Vered Volansky" <vered(a)redhat.com>
> > > > >> Cc: engine-devel(a)ovirt.org
> > > > >> Sent: Monday, May 27, 2013 6:22:58 PM
> > > > >> Subject: Re: [Engine-devel] Disk BE very small refactoring
> > > > >>
> > > > >> Vered,
> > > > >> VmDevice has "isPlugged" field,
> > > > >> Why not have somehow in your inheritence (either Disk or a
> > > > >> subclass)
> > > > >> a
> > > > >> field
> > > > >> : "VmDevice device"
> > > > >
> > > > > disk id is the device id, no need for field to represent the
> > > > > relation.
> > > > > the combination of disk-id and a specific entity (vm/template)
will
> > > > > get
> > > > > you
> > > > > the other info
> > > > >
> > > > >> and have isPlugged method called
"device.isPlugged()" ?
> > > > >>
> > > > >> Then you can also add the readOnly property which is not
> > > > >> represented
> > > > >> at
> > > > >> VmDevice.
> > > > >>
> > > > >>
> > > > >> Does this sound logical to you?
> > > > >>
> > > > >> ----- Original Message -----
> > > > >>> From: "Vered Volansky"
<vered(a)redhat.com>
> > > > >>> To: engine-devel(a)ovirt.org
> > > > >>> Sent: Monday, May 27, 2013 6:18:58 PM
> > > > >>> Subject: [Engine-devel] Disk BE very small refactoring
> > > > >>>
> > > > >>> Hi All,
> > > > >>>
> > > > >>> Please express your opinion if you have any -
> > > > >>>
> > > > >>> Currently Disk BE has a plugged property, which should
be a
> > > > >>> property
> > > > >>> of
> > > > >>> the
> > > > >>> relationship between vm(or template) and a disk.
> > > > >>> I plan to remove this property from the Disk entity, and
add new
> > > > >>> entity,
> > > > >>> called DeviceDisk.
> > > > >>> This should inherit from Disk and contain the
vm/template guid
> > > > >>> and
> > > > >>> the
> > > > >>> plugged property at first round.
> > > > >>> At second round it'll also contain the readOnly
property, for RO
> > > > >>> disks,
> > > > >>> TBD
> > > > >>> right after.
> > > > >>>
> > > > >>> Appreciate any input,
> > > > >>> Vered
> > > > >>> _______________________________________________
> > > > >>> Engine-devel mailing list
> > > > >>> Engine-devel(a)ovirt.org
> > > > >>> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > > >>>
> > > > >> _______________________________________________
> > > > >> Engine-devel mailing list
> > > > >> Engine-devel(a)ovirt.org
> > > > >> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > > >>
> > > > > _______________________________________________
> > > > > Engine-devel mailing list
> > > > > Engine-devel(a)ovirt.org
> > > > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > Engine-devel mailing list
> > > > Engine-devel(a)ovirt.org
> > > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > >
> > >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:57 a.m.
----- Original Message -----
From: "Omer Frenkel" <ofrenkel(a)redhat.com>
To: "Vered Volansky" <vered(a)redhat.com>
Cc: "Maor Lipchuk" <mlipchuk(a)redhat.com>, "Yair Zaslavsky"
<yzaslavs(a)redhat.com>, "Mike Kolesnik"
<mkolesni(a)redhat.com>, "Allon Mureinik" <amureini(a)redhat.com>,
"Daniel Erez" <derez(a)redhat.com>,
engine-devel(a)ovirt.org
Sent: Thursday, May 30, 2013 11:29:40 AM
Subject: Re: [Engine-devel] Disk BE very small refactoring
----- Original Message -----
> From: "Vered Volansky" <vered(a)redhat.com>
> To: "Maor Lipchuk" <mlipchuk(a)redhat.com>, "Yair
Zaslavsky"
> <yzaslavs(a)redhat.com>, "Omer Frenkel"
> <ofrenkel(a)redhat.com>, "Mike Kolesnik" <mkolesni(a)redhat.com>,
"Allon
> Mureinik" <amureini(a)redhat.com>, "Daniel Erez"
> <derez(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Tuesday, May 28, 2013 8:40:38 PM
> Subject: Re: [Engine-devel] Disk BE very small refactoring
>
> I had a problem, didn't see all the replies till now.
>
> I'll add some more info as to why we want to do this like we do.
> It all started from adding the readOnly property to disks. It should have
> been handled like plugged is handled, yet plugged is a hack, and if we
> don't
> change that now, we'll only keep on adding unreasonable hacks.
>
please explain why you think plugged is a hack? what is wrong with it?
Plugged is a
property of a disk-vm.
Were the disk only be able to be plugged to one VM, there would be no problem with that.
But since the disk can be plugged to several VMs, the plugged property should be on the
relationship between the two, and not selectively on disk.
Since this is the current case, there's a query that fills in this data in disk under
the VM's context, which can be changed when the VM's context is different, which
is a hack.
> So from the start -
> Disk currently:
> - Sometimes represents a disk in a vm context and sometimes not.
> - Holds plugged property, which is only relevant when a disk is in a vm
> context, which already suggests this is not the natural place for it.
> - Also holds bootable and interface, which cause limitations of use, but
> are
> not so obviously related to the relationship between Disk and Vm as
> plugged.
> - Can be shared between several vm's, to some plugged and to some not
> plugged.
> - Will soon be optionally RO in one VM and RW in another, which is exactly
> the same as plugged, and therefore plugged issue should be fixed first.
>
> Every column in that shows a disk in the UI receives a Disk entity, and
> show
> its contents, while plugged/unplugged is ignored when not in a VM context.
> The way things are now, using a VmDevice in the where we need it to show
> plug
> status, we'll also have to use it in all other columns, which is irrelevant
> and just totally not related.
> So using VmDevice in UI is a no go.
>
> The UI is the main limitation forcing us to use something that extends
> Disk,
> and what I described below is the easiest thing to implement in the time
> restrictions we have without changing the entire system.
>
> I think this answers all the questions not already answered by others.
> Regarding Maor suggestion - might be a good idea, but not in this scope or
> time-frame.
>
> If there is any other/unanswered issue or objection to the design change
> please share.
>
> I appreciate your inputs,
> Vered
>
sorry i didnt understand why the current disk object isnt good enough,
you get a disk, some of its properties are valid only in some situations.
i think its easier to use instead of couple of different objects representing
the same entity in different situations.
This just doesn't belong there,
there's a workaround to make it work when it's indeed needed - by the
GetAllDisksByVmIdQuery.
In between the value there means nothing, the fact that we successfully avoid falling into
that pitfall doesn't mean it should remain this way.
More importantly, this is not done here, we're adding other properties (currently RO)
that face the same handling onless this is resolved now.
This is the best resolution we came up with under the circumstances.
Are you putting you foot down on this?
>
> ----- Original Message -----
> > From: "Maor Lipchuk" <mlipchuk(a)redhat.com>
> > To: "Vered Volansky" <vered(a)redhat.com>
> > Cc: engine-devel(a)ovirt.org
> > Sent: Tuesday, May 28, 2013 1:49:46 PM
> > Subject: Re: [Engine-devel] Disk BE very small refactoring
> >
> > I think the main problem is that we abuse the business entity to act as
> > an O/R mapping class to the DB and also to be used as a business entity
> > for presentation purposes.
> >
> > I understand how Yair thought isPlugged could be fetched from vmDevice,
> > this is a confusing design, and it is just one example and more to come.
> >
> > I suggest that if we already thinking of changing the class hierarchy,
> > we can start by implementing a package for presentation classes for
> > transient classes such as this instead enforcing complex hierarchy.
> >
> > The query class will fetch all the data from the DB and initialized the
> > transient class and send it to the client.
> > I think it could be a good start and will solve many issues we might
> > encounter in the future.
> >
> > Regards,
> > Maor
> >
> > On 05/28/2013 11:24 AM, Omer Frenkel wrote:
> > >
> > >
> > > ----- Original Message -----
> > >> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> > >> To: "Vered Volansky" <vered(a)redhat.com>
> > >> Cc: engine-devel(a)ovirt.org
> > >> Sent: Monday, May 27, 2013 6:22:58 PM
> > >> Subject: Re: [Engine-devel] Disk BE very small refactoring
> > >>
> > >> Vered,
> > >> VmDevice has "isPlugged" field,
> > >> Why not have somehow in your inheritence (either Disk or a subclass)
a
> > >> field
> > >> : "VmDevice device"
> > >
> > > disk id is the device id, no need for field to represent the relation.
> > > the combination of disk-id and a specific entity (vm/template) will get
> > > you
> > > the other info
> > >
> > >> and have isPlugged method called "device.isPlugged()" ?
> > >>
> > >> Then you can also add the readOnly property which is not represented
> > >> at
> > >> VmDevice.
> > >>
> > >>
> > >> Does this sound logical to you?
> > >>
> > >> ----- Original Message -----
> > >>> From: "Vered Volansky" <vered(a)redhat.com>
> > >>> To: engine-devel(a)ovirt.org
> > >>> Sent: Monday, May 27, 2013 6:18:58 PM
> > >>> Subject: [Engine-devel] Disk BE very small refactoring
> > >>>
> > >>> Hi All,
> > >>>
> > >>> Please express your opinion if you have any -
> > >>>
> > >>> Currently Disk BE has a plugged property, which should be a
property
> > >>> of
> > >>> the
> > >>> relationship between vm(or template) and a disk.
> > >>> I plan to remove this property from the Disk entity, and add new
> > >>> entity,
> > >>> called DeviceDisk.
> > >>> This should inherit from Disk and contain the vm/template guid
and
> > >>> the
> > >>> plugged property at first round.
> > >>> At second round it'll also contain the readOnly property, for
RO
> > >>> disks,
> > >>> TBD
> > >>> right after.
> > >>>
> > >>> Appreciate any input,
> > >>> Vered
> > >>> _______________________________________________
> > >>> Engine-devel mailing list
> > >>> Engine-devel(a)ovirt.org
> > >>> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >>>
> > >> _______________________________________________
> > >> Engine-devel mailing list
> > >> Engine-devel(a)ovirt.org
> > >> http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >>
> > > _______________________________________________
> > > Engine-devel mailing list
> > > Engine-devel(a)ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >
> >
> >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
>
4221
days inactive
4227
days old
11 comments
7 participants
participants (7)
-
Allon Mureinik -
Eli Mesika -
Maor Lipchuk -
Mike Kolesnik -
Omer Frenkel -
Vered Volansky -
Yair Zaslavsky