On 05/16/2012 01:16 PM, Gilad Chaplik wrote: > Hi All, > > I am adding the ability to import a VM or a Template to a storage-domain, > when this VM or Template already exists in the destination storage domain. > Until now, Backend failed this. Now I want to enable the user to specify > that he wishes this VM/Template will be created again by a different name, > i.e - cloned. > > [feature page: http://www.ovirt.org/wiki/Features/ImportMoreThanOnce] > > I plan to achieve this using a new parameter, but I want to reach an agreement > about this parameter's name. I thought simply to call it "clone". > > Another thing that I'll do in the patch-set is add the currently-missing ability > to specify whether the snapshots of the VM, which is being imported, will > be collapsed into a single snapshot (we have this ability in GUI). I am also > deliberating about the name of this parameter. I thought about > "collapse_snapshots" (same as in GUI). > > Does anyone think "clone" and "collapse_snapshots" are inappropriate and has /clone/ already in-use (used to clone vm from template), <vm> <disks> <clone>true|false</clone> ..., you can simply say if imported vm has <name> element, this is import+clone, otherwise import,

as about collapse_snapshots, i don't mind, but this should be done in the way <clone> is implemented in <disks> collection

> better suggestions? > > Thanks, > Gilad -- Michael Pasternak RedHat, ENG-Virtualization R&D

On 05/16/2012 02:49 PM, Ori Liel wrote: >> On 05/16/2012 01:16 PM, Gilad Chaplik wrote: >>> Hi All, >>> >>> I am adding the ability to import a VM or a Template to a storage-domain, >>> when this VM or Template already exists in the destination storage domain. >>> Until now, Backend failed this. Now I want to enable the user to specify >>> that he wishes this VM/Template will be created again by a different name, >>> i.e - cloned. >>> >>> [feature page: http://www.ovirt.org/wiki/Features/ImportMoreThanOnce] >>> >>> I plan to achieve this using a new parameter, but I want to reach an agreement >>> about this parameter's name. I thought simply to call it "clone". >>> >>> Another thing that I'll do in the patch-set is add the currently-missing ability >>> to specify whether the snapshots of the VM, which is being imported, will >>> be collapsed into a single snapshot (we have this ability in GUI). I am also >>> deliberating about the name of this parameter. I thought about >>> "collapse_snapshots" (same as in GUI). >>> >>> Does anyone think "clone" and "collapse_snapshots" are inappropriate and has >> >> /clone/ already in-use (used to clone vm from template), >> >> <vm> >> <disks> >> <clone>true|false</clone> >> ..., >> >> you can simply say if imported vm has <name> element, this is import+clone, otherwise import, > > If in the future we will want to enable overriding a VM's params on import, this will be confusing > (because a user might want to import a VM and change it's name - but not clone it if it already exists). the concept of /import/ is copy vm from sd1 to sd2, then you can change vm meta as needed,

besides we already using <name> element existence/absence as action 'determinator' in other places in api.

> >> as about collapse_snapshots, i don't mind, but this should be done in the way <clone> is implemented >> in <disks> collection > > Semantically, a snapshot is a point in time of a VM. It not only associated any more only with the VM's > disks; it includes the VM's meta-data as well. For this reason, maybe the parameter collapse_snapshots > should not be in <disks> collection (although, technically, the collapse will be done on disks) obviously i meant <snapshots>, thanks. > >> >> >>> better suggestions? >>> >>> Thanks, >>> Gilad >> >> >> -- >> >> Michael Pasternak >> RedHat, ENG-Virtualization R&D -- Michael Pasternak RedHat, ENG-Virtualization R&D

From: "Ori Liel" <oliel(a)redhat.com&gt; To: "Michael Pasternak" <mpastern(a)redhat.com&gt; Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, "Itamar Heim" <iheim(a)redhat.com&gt;, "Doron Fediuck" <dfediuck(a)redhat.com&gt;, "Gilad Chaplik" <gchaplik(a)redhat.com&gt; Sent: Wednesday, May 16, 2012 2:49:17 PM Subject: Re: restapi: New params for import VM/Template >On 05/16/2012 01:16 PM, Gilad Chaplik wrote: >> Hi All, >> >> I am adding the ability to import a VM or a Template to a >> storage-domain, >> when this VM or Template already exists in the destination storage >> domain. >> Until now, Backend failed this. Now I want to enable the user to >> specify >> that he wishes this VM/Template will be created again by a >> different name, >> i.e - cloned. >> >> [feature page: >> http://www.ovirt.org/wiki/Features/ImportMoreThanOnce] >> >> I plan to achieve this using a new parameter, but I want to reach >> an agreement >> about this parameter's name. I thought simply to call it "clone". >> >> Another thing that I'll do in the patch-set is add the >> currently-missing ability >> to specify whether the snapshots of the VM, which is being >> imported, will >> be collapsed into a single snapshot (we have this ability in GUI). >> I am also >> deliberating about the name of this parameter. I thought about >> "collapse_snapshots" (same as in GUI). >> >> Does anyone think "clone" and "collapse_snapshots" are >> inappropriate and has > >/clone/ already in-use (used to clone vm from template),

> ><vm> > <disks> > <clone>true|false</clone> >..., > >you can simply say if imported vm has <name> element, this is >import+clone, otherwise import, If in the future we will want to enable overriding a VM's params on import, this will be confusing (because a user might want to import a VM and change it's name - but not clone it if it already exists).

>as about collapse_snapshots, i don't mind, but this should be done >in the way <clone> is implemented >in <disks> collection Semantically, a snapshot is a point in time of a VM. It not only associated any more only with the VM's disks; it includes the VM's meta-data as well. For this reason, maybe the parameter collapse_snapshots should not be in <disks> collection (although, technically, the collapse will be done on disks)

> > >> better suggestions? >> >> Thanks, >> Gilad > > >-- > >Michael Pasternak >RedHat, ENG-Virtualization R&D

On 05/16/2012 03:26 PM, Gilad Chaplik wrote: > > > Thanks, > Gilad. > > ----- Original Message ----- >> From: "Ori Liel&quot;&lt;oliel(a)redhat.com&gt; >> To: "Michael Pasternak&quot;&lt;mpastern(a)redhat.com&gt; >> Cc: &quot;engine-devel&quot;&lt;engine-devel(a)ovirt.org&gt;, "Itamar Heim&quot;&lt;iheim(a)redhat.com&gt;, "Doron Fediuck&quot;&lt;dfediuck(a)redhat.com&gt;, >> "Gilad Chaplik&quot;&lt;gchaplik(a)redhat.com&gt; >> Sent: Wednesday, May 16, 2012 2:49:17 PM >> Subject: Re: restapi: New params for import VM/Template >> >>> On 05/16/2012 01:16 PM, Gilad Chaplik wrote: >>>> Hi All, >>>> >>>> I am adding the ability to import a VM or a Template to a >>>> storage-domain, >>>> when this VM or Template already exists in the destination storage >>>> domain. >>>> Until now, Backend failed this. Now I want to enable the user to >>>> specify >>>> that he wishes this VM/Template will be created again by a >>>> different name, >>>> i.e - cloned. >>>> >>>> [feature page: >>>> http://www.ovirt.org/wiki/Features/ImportMoreThanOnce] >>>> >>>> I plan to achieve this using a new parameter, but I want to reach >>>> an agreement >>>> about this parameter's name. I thought simply to call it "clone". >>>> >>>> Another thing that I'll do in the patch-set is add the >>>> currently-missing ability >>>> to specify whether the snapshots of the VM, which is being >>>> imported, will >>>> be collapsed into a single snapshot (we have this ability in GUI). >>>> I am also >>>> deliberating about the name of this parameter. I thought about >>>> "collapse_snapshots" (same as in GUI). >>>> >>>> Does anyone think "clone" and "collapse_snapshots" are >>>> inappropriate and has >>> >>> /clone/ already in-use (used to clone vm from template), > > clone here has a different context, clone VM vs. clone disks. having two clone elements in vm will be confusing. -1 > >>> >>> <vm> >>> <disks> >>> <clone>true|false</clone> >>> ..., >>> >>> you can simply say if imported vm has<name> element, this is >>> import+clone, otherwise import, >> >> If in the future we will want to enable overriding a VM's params on >> import, this will be confusing >> (because a user might want to import a VM and change it's name - but >> not clone it if it already exists). > > +1, cloning a vm and changing the vm's metadata (i.e vm's name) should not be inter-dependent. how exactly this is contradict changing metadata on the fly?, exactly on opposite - it works perfectly well for your use-case: BE logic: -------- if (local<storage>) has vm named as on remote<storage> (export.domain)

{ if PARAMS<name> != remote<name> (export.domain) { copy + rename } else { error } } else { if PARAMS<name> != remote<name> (export.domain) { copy + rename } else { copy } } > >> >>> as about collapse_snapshots, i don't mind, but this should be done >>> in the way<clone> is implemented >>> in<disks> collection >> >> Semantically, a snapshot is a point in time of a VM. It not only >> associated any more only with the VM's >> disks; it includes the VM's meta-data as well. For this reason, maybe >> the parameter collapse_snapshots >> should not be in<disks> collection (although, technically, the >> collapse will be done on disks) > > +1, I think the collapse_snapshots should be in the vm context (snapshots is under vm). i meant<snapshots>, see my other email on this. > > Other than that, currently, if you want to clone a vm, it must be 'collapsed snapshots', so > the flow to clone a vm (with your suggestion) will be: > > <action> > <vm> > <name>new_vm</..> > <disks> > <collapse_snapshot>true</..> > </..> > </..> > <clone>true</..> > </..> > > where collapse_snapshot should be superior to clone, this structure is a bit confusing. > >> >>> >>> >>>> better suggestions? >>>> >>>> Thanks, >>>> Gilad >>> >>> >>> -- >>> >>> Michael Pasternak >>> RedHat, ENG-Virtualization R&D >>

From: "Michael Pasternak" <mpastern(a)redhat.com&gt; To: "Itamar Heim" <iheim(a)redhat.com&gt;, "Omer Frenkel" <ofrenkel(a)redhat.com&gt; Cc: "Gilad Chaplik" <gchaplik(a)redhat.com&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt;, "Doron Fediuck" <dfediuck(a)redhat.com&gt;, "Ori Liel" <oliel(a)redhat.com&gt; Sent: Wednesday, May 16, 2012 4:17:57 PM Subject: Re: restapi: New params for import VM/Template On 05/16/2012 04:05 PM, Itamar Heim wrote: > On 05/16/2012 03:49 PM, Michael Pasternak wrote: >> On 05/16/2012 03:26 PM, Gilad Chaplik wrote: >>> >>> >>> Thanks, >>> Gilad. >>> >>> ----- Original Message ----- >>>> From: "Ori Liel&quot;&lt;oliel(a)redhat.com&gt; >>>> To: "Michael Pasternak&quot;&lt;mpastern(a)redhat.com&gt; >>>> Cc: &quot;engine-devel&quot;&lt;engine-devel(a)ovirt.org&gt;, "Itamar >>>> Heim&quot;&lt;iheim(a)redhat.com&gt;, "Doron Fediuck&quot;&lt;dfediuck(a)redhat.com&gt;, >>>> "Gilad Chaplik&quot;&lt;gchaplik(a)redhat.com&gt; >>>> Sent: Wednesday, May 16, 2012 2:49:17 PM >>>> Subject: Re: restapi: New params for import VM/Template >>>> >>>>> On 05/16/2012 01:16 PM, Gilad Chaplik wrote: >>>>>> Hi All, >>>>>> >>>>>> I am adding the ability to import a VM or a Template to a >>>>>> storage-domain, >>>>>> when this VM or Template already exists in the destination >>>>>> storage >>>>>> domain. >>>>>> Until now, Backend failed this. Now I want to enable the user >>>>>> to >>>>>> specify >>>>>> that he wishes this VM/Template will be created again by a >>>>>> different name, >>>>>> i.e - cloned. >>>>>> >>>>>> [feature page: >>>>>> http://www.ovirt.org/wiki/Features/ImportMoreThanOnce] >>>>>> >>>>>> I plan to achieve this using a new parameter, but I want to >>>>>> reach >>>>>> an agreement >>>>>> about this parameter's name. I thought simply to call it >>>>>> "clone". >>>>>> >>>>>> Another thing that I'll do in the patch-set is add the >>>>>> currently-missing ability >>>>>> to specify whether the snapshots of the VM, which is being >>>>>> imported, will >>>>>> be collapsed into a single snapshot (we have this ability in >>>>>> GUI). >>>>>> I am also >>>>>> deliberating about the name of this parameter. I thought about >>>>>> "collapse_snapshots" (same as in GUI). >>>>>> >>>>>> Does anyone think "clone" and "collapse_snapshots" are >>>>>> inappropriate and has >>>>> >>>>> /clone/ already in-use (used to clone vm from template), >>> >>> clone here has a different context, clone VM vs. clone disks. >> >> having two clone elements in vm will be confusing. >> >> -1 >> >>> >>>>> >>>>> <vm> >>>>> <disks> >>>>> <clone>true|false</clone> >>>>> ..., >>>>> >>>>> you can simply say if imported vm has<name> element, this is >>>>> import+clone, otherwise import, >>>> >>>> If in the future we will want to enable overriding a VM's params >>>> on >>>> import, this will be confusing >>>> (because a user might want to import a VM and change it's name - >>>> but >>>> not clone it if it already exists). >>> >>> +1, cloning a vm and changing the vm's metadata (i.e vm's name) >>> should not be inter-dependent. >> >> how exactly this is contradict changing metadata on the fly?, >> exactly on opposite - it works perfectly well for your use-case: >> >> BE logic: >> -------- >> >> if (local<storage>) has vm named as on remote<storage> >> (export.domain) > > please note "vm exists" is based on vm uuid, not on vm name i think it based on name, Omer?

> >> { >> if PARAMS<name> != remote<name> (export.domain) >> { >> copy + rename >> } else { >> error >> } >> } else { >> if PARAMS<name> != remote<name> (export.domain) >> { >> copy + rename >> } else { >> copy >> } >> } >> >>> >>>> >>>>> as about collapse_snapshots, i don't mind, but this should be >>>>> done >>>>> in the way<clone> is implemented >>>>> in<disks> collection >>>> >>>> Semantically, a snapshot is a point in time of a VM. It not only >>>> associated any more only with the VM's >>>> disks; it includes the VM's meta-data as well. For this reason, >>>> maybe >>>> the parameter collapse_snapshots >>>> should not be in<disks> collection (although, technically, the >>>> collapse will be done on disks) >>> >>> +1, I think the collapse_snapshots should be in the vm context >>> (snapshots is under vm). >> >> i meant<snapshots>, see my other email on this. >> >>> >>> Other than that, currently, if you want to clone a vm, it must be >>> 'collapsed snapshots', so >>> the flow to clone a vm (with your suggestion) will be: >>> >>> <action> >>> <vm> >>> <name>new_vm</..> >>> <disks> >>> <collapse_snapshot>true</..> >>> </..> >>> </..> >>> <clone>true</..> >>> </..> >>> >>> where collapse_snapshot should be superior to clone, this >>> structure is a bit confusing. >>> >>>> >>>>> >>>>> >>>>>> better suggestions? >>>>>> >>>>>> Thanks, >>>>>> Gilad >>>>> >>>>> >>>>> -- >>>>> >>>>> Michael Pasternak >>>>> RedHat, ENG-Virtualization R&D >>>> >> >> > -- Michael Pasternak RedHat, ENG-Virtualization R&D

>> >> please note "vm exists" is based on vm uuid, not on vm name > > i think it based on name, Omer? two different things: 1. vm name is unique. 2. import vm cannot import an existing vm based on it's uuid (which is what this feature is about). i.e., if i create a vm X, export it, rename X to Y, i will still fail importing X without 'cloning' it (the cloning process is about changing uuid's of vm, disks, nics)

On 05/16/2012 05:08 PM, Michael Pasternak wrote: > On 05/16/2012 04:38 PM, Itamar Heim wrote: >>>> >>>> please note "vm exists" is based on vm uuid, not on vm name >>> >>> i think it based on name, Omer? >> >> two different things: >> 1. vm name is unique. >> 2. import vm cannot import an existing vm based on it's uuid (which is what this feature is about). >> >> i.e., if i create a vm X, export it, rename X to Y, i will still fail importing X without 'cloning' it (the cloning process is about changing uuid's of vm, disks, nics) > > why import not changing ids by definition? this way only collision that might happen > is a vm.name ..., i.e 'cannot import vm.x cause vm with same name already exist' ... > 1. because we didn't have this behavior till now. 2. because for templates you may want to preserve the uuid to move over VMs/chains using it. 3. because import keeping uuid's allows to handle snapshot chains and re-use of template which does not require changing the actual images, still pointing to the low level actual file/chains (can also be fixed by separating internal uuid's and disks/snapshots uuids, but much more work)

From: "Michael Pasternak" <mpastern(a)redhat.com&gt; To: "Itamar Heim" <iheim(a)redhat.com&gt; Cc: "Omer Frenkel" <ofrenkel(a)redhat.com&gt;, "Gilad Chaplik" <gchaplik(a)redhat.com&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt;, "Doron Fediuck" <dfediuck(a)redhat.com&gt;, "Ori Liel" <oliel(a)redhat.com&gt; Sent: Wednesday, May 16, 2012 6:06:15 PM Subject: Re: restapi: New params for import VM/Template On 05/16/2012 05:05 PM, Itamar Heim wrote: > On 05/16/2012 05:08 PM, Michael Pasternak wrote: >> On 05/16/2012 04:38 PM, Itamar Heim wrote: >>>>> >>>>> please note "vm exists" is based on vm uuid, not on vm name >>>> >>>> i think it based on name, Omer? >>> >>> two different things: >>> 1. vm name is unique. >>> 2. import vm cannot import an existing vm based on it's uuid >>> (which is what this feature is about). >>> >>> i.e., if i create a vm X, export it, rename X to Y, i will still >>> fail importing X without 'cloning' it (the cloning process is >>> about changing uuid's of vm, disks, nics) >> >> why import not changing ids by definition? this way only collision >> that might happen >> is a vm.name ..., i.e 'cannot import vm.x cause vm with same name >> already exist' ... >> > > 1. because we didn't have this behavior till now. > 2. because for templates you may want to preserve the uuid to move > over VMs/chains using it. > 3. because import keeping uuid's allows to handle snapshot chains > and re-use of template which does not require changing the actual > images, still pointing to the low level > actual file/chains (can also be fixed by separating internal uuid's > and disks/snapshots uuids, but much more work) ok, then implicit re-id can happen when importing already existent entity and only complaint will be entity name (which has to stay unique and user-defined). this way you does not force user to supply /X=true/ arg as it's obvious in this scenario.

-- Michael Pasternak RedHat, ENG-Virtualization R&D

From: "Livnat Peer" <lpeer(a)redhat.com&gt; To: "Gilad Chaplik" <gchaplik(a)redhat.com&gt; Cc: "Michael Pasternak" <mpastern(a)redhat.com&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt;, "Itamar Heim" <iheim(a)redhat.com&gt; Sent: Thursday, May 17, 2012 9:43:45 AM Subject: Re: [Engine-devel] restapi: New params for import VM/Template > > Going FW it would be nice to override parameters in import > vm/template I agree with you and that's why - As a user I don't think there should be a difference in the API between: - Importing a VM and changing it's name - Importing a VM for the second time The reason you want to add artificial difference between the two scenarios above is because currently there are implementations limitations (changing the image ID while importing is not supported yet) I think that we should solve the limitations in the implementation instead of making our API cumbersome (implicit clone by name and adding some clone entity are both bad IMO). Livnat

> > Going FW it would be nice to override parameters in import vm/template I agree with you and that's why - As a user I don't think there should be a difference in the API between: - Importing a VM and changing it's name - Importing a VM for the second time The reason you want to add artificial difference between the two scenarios above is because currently there are implementations limitations (changing the image ID while importing is not supported yet) I think that we should solve the limitations in the implementation instead of making our API cumbersome (implicit clone by name and adding some clone entity are both bad IMO).

Livnat