----- Original Message -----
From: "Liron Aravot" <laravot(a)redhat.com>
To: "Michael Kublin" <mkublin(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Monday, November 19, 2012 5:16:27 PM
Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
----- Original Message -----
> From: "Michael Kublin" <mkublin(a)redhat.com>
> To: "Liron Aravot" <laravot(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Monday, November 19, 2012 4:48:19 PM
> Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
>
>
>
> ----- Original Message -----
> > From: "Liron Aravot" <laravot(a)redhat.com>
> > To: "Michael Kublin" <mkublin(a)redhat.com>
> > Cc: engine-devel(a)ovirt.org
> > Sent: Monday, November 19, 2012 3:58:55 PM
> > Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
> >
> >
> >
> > ----- Original Message -----
> > > From: "Michael Kublin" <mkublin(a)redhat.com>
> > > To: "Liron Aravot" <laravot(a)redhat.com>
> > > Cc: engine-devel(a)ovirt.org
> > > Sent: Monday, November 19, 2012 3:44:41 PM
> > > Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
> > >
> > >
> > >
> > > ----- Original Message -----
> > > > From: "Liron Aravot" <laravot(a)redhat.com>
> > > > To: "Michael Kublin" <mkublin(a)redhat.com>
> > > > Cc: engine-devel(a)ovirt.org
> > > > Sent: Monday, November 19, 2012 3:34:11 PM
> > > > Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
> > > >
> > > >
> > > >
> > > > ----- Original Message -----
> > > > > From: "Michael Kublin" <mkublin(a)redhat.com>
> > > > > To: "Liron Aravot" <laravot(a)redhat.com>
> > > > > Cc: engine-devel(a)ovirt.org
> > > > > Sent: Monday, November 19, 2012 3:28:06 PM
> > > > > Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
> > > > >
> > > > >
> > > > >
> > > > > ----- Original Message -----
> > > > > > From: "Liron Aravot" <laravot(a)redhat.com>
> > > > > > To: "Michael Kublin" <mkublin(a)redhat.com>
> > > > > > Cc: engine-devel(a)ovirt.org
> > > > > > Sent: Monday, November 19, 2012 3:17:04 PM
> > > > > > Subject: Re: [Engine-devel] [Engine-Devel] OvfDataUpdater
> > > > > >
> > > > > >
> > > > > >
> > > > > > ----- Original Message -----
> > > > > > > From: "Michael Kublin"
<mkublin(a)redhat.com>
> > > > > > > To: "Liron Aravot"
<laravot(a)redhat.com>
> > > > > > > Cc: engine-devel(a)ovirt.org
> > > > > > > Sent: Monday, November 19, 2012 3:03:23 PM
> > > > > > > Subject: Re: [Engine-devel] [Engine-Devel]
> > > > > > > OvfDataUpdater
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > ----- Original Message -----
> > > > > > > > From: "Liron Aravot"
<laravot(a)redhat.com>
> > > > > > > > To: "Mike Kolesnik"
<mkolesni(a)redhat.com>
> > > > > > > > Cc: engine-devel(a)ovirt.org, "Michael
Kublin"
> > > > > > > > <mkublin(a)redhat.com>
> > > > > > > > Sent: Monday, November 19, 2012 2:52:56 PM
> > > > > > > > Subject: Re: [Engine-devel] [Engine-Devel]
> > > > > > > > OvfDataUpdater
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > ----- Original Message -----
> > > > > > > > > From: "Mike Kolesnik"
<mkolesni(a)redhat.com>
> > > > > > > > > To: "Liron Aravot"
<laravot(a)redhat.com>
> > > > > > > > > Cc: engine-devel(a)ovirt.org, "Michael
Kublin"
> > > > > > > > > <mkublin(a)redhat.com>
> > > > > > > > > Sent: Monday, November 19, 2012 2:44:56 PM
> > > > > > > > > Subject: Re: [Engine-devel] [Engine-Devel]
> > > > > > > > > OvfDataUpdater
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > ----- Original Message -----
> > > > > > > > > > > From: "Liron Aravot"
<laravot(a)redhat.com>
> > > > > > > > > > > To: engine-devel(a)ovirt.org
> > > > > > > > > > > Sent: Monday, November 19, 2012
2:03:57 PM
> > > > > > > > > > > Subject: [Engine-devel]
[Engine-Devel]
> > > > > > > > > > > OvfDataUpdater
> > > > > > > > > > >
> > > > > > > > > > > starting a new discussion thread.
> > > > > > > > > > >
> > > > > > > > > > > ----- Original Message -----
> > > > > > > > > > > > From: "Mike
Kolesnik" <mkolesni(a)redhat.com>
> > > > > > > > > > > > To: "Liron Aravot"
<laravot(a)redhat.com>
> > > > > > > > > > > > Sent: Monday, November 19,
2012 12:42:10 PM
> > > > > > > > > > > > Subject: Re: [Engine-devel]
OvfAutoUpdater
> > > > > > > > > > > >
> > > > > > > > > > > > I think 'version' is
a more standard term for
> > > > > > > > > > > > the
> > > > > > > > > > > > column
> > > > > > > > > > > > names.
> > > > > > > > > > > >
> > > > > > > > > > > > Also in: 4. If succesfull -
for each vm
> > > > > > > > > > > > update
> > > > > > > > > > > > the
> > > > > > > > > > > > ovf_generation
> > > > > > > > > > > > to
> > > > > > > > > > > > equal the db_generation.
> > > > > > > > > > > > I think you mean that the
update should be to
> > > > > > > > > > > > the
> > > > > > > > > > > > entity
> > > > > > > > > > > > version
> > > > > > > > > > > > you
> > > > > > > > > > > > initially selected.
> > > > > > > > > > > >
> > > > > > > > > > 1. I think should be version = version
+1;
> > > > > > > > the version will be the db_generation that was
loaded
> > > > > > > > when
> > > > > > > > we
> > > > > > > > loaded
> > > > > > > > the vm/template.
> > > > > > > > for example - if the vm/template was updated
twice
> > > > > > > > between
> > > > > > > > OvfUpdater
> > > > > > > > runs, we will need to increment twice - so
> > > > > > > > incrementing
> > > > > > > > by
> > > > > > > > one
> > > > > > > > will
> > > > > > > > cause another unneeded ovf update for that vm.
> > > > > > > I am talking about ovfVersion or how you called it.
> > > > > > me too,
> > > > > > if we update the vm three times between OvfAutoUpdater
> > > > > > runs
> > > > > > ,
> > > > > > we
> > > > > > will
> > > > > > have the following values in the loaded vm for example:
> > > > > > db_generation 8
> > > > > > ovf_generation 5
> > > > > >
> > > > > > when we perform the ovf update , ovf_generation should be
> > > > > > set
> > > > > > to
> > > > > > 8
> > > > > > and not to 6, as version '8' is the version that we
wrote
> > > > > > the
> > > > > > ovf
> > > > > > metadata of.
> > > > > These should be written in wiki, with remarks that values
> > > > > which
> > > > > were
> > > > > retrieved
> > > > > from DB together.
> > > > > > > > > > 2. Now the quartz is automatic
process,
> > > > > > > > > > updateVmInSpm
> > > > > > > > > > it
> > > > > > > > > > is
> > > > > > > > > > spm
> > > > > > > > > > operation -it means it can trigger a
reconstruct
> > > > > > > > > > and spm election.
> > > > > > > > 2. updateVmInSpm command would be changed so it
won't
> > > > > > > > trigger
> > > > > > > > failover, will add that to the wiki.
> > > > > > > > > > 3. How your solution is handling a case
of
> > > > > > > > > > hotPlug/hotUnplug/remove/add of vm
disks.
> > > > > > > > > > vm_static
> > > > > > > > > > is
> > > > > > > > > > not
> > > > > > > > > > usually
> > > > > > > > > > updated.
> > > > > > > > Whenever will be a command that locks a
vm/template,
> > > > > > > > you
> > > > > > > > will
> > > > > > > > have
> > > > > > > > an
> > > > > > > > option during the unlock to specify if the
version
> > > > > > > > need
> > > > > > > > to
> > > > > > > > be
> > > > > > > > incremented, you'll be able to increment it
also
> > > > > > > > yourself.
> > > > > > > > The HotUnPlug command does lock the vm and
during
> > > > > > > > endSuccesfully
> > > > > > > > attempt to update the vm in spm, so it will
increment
> > > > > > > > the
> > > > > > > > db_version/generation instead.
> > > > > > > > >
> > > > > > > addDisk, removeDisk, hotPlug/unPlug not locking a vm.
> > > > > > > Also, as far as I know LockVm/UnLockVm it is
operation
> > > > > > > on
> > > > > > > vm_dynamic
> > > > > > > and not
> > > > > > > vm_static.
> > > > > >
> > > > > > hotplug acquires memory lock on the vm and has no async
> > > > > > tasks
> > > > > > if
> > > > > > I
> > > > > > recall correctly.
> > > > > Yes, correct and what? These what I said, vm_static is not
> > > > > updated.
> > > > you will just call the dao method for incrementing the
> > > > db_generation,
> > > > you don't have to perform vm_static update.
> > > > > > adddisk acquires shared lock on the vm which is
> > > > > > problematic
> > > > > > with
> > > > > > today flow as well - as you have a race during
> > > > > > updateVmInSpm,
> > > > > > it
> > > > > > should be fixed regardless of the OvfAutoUpdater.
> > > > > It is not answer on my question. How ovf will be updated?
> > > > > How
> > > > > you
> > > > > understand
> > > > > that ovf of vm should be updated?
> > > >
> > > > the ovf will be updated when you increment the db_generation
> > > > value
> > > > instead of calling updateVmInSpm().
> > > > you will be able to do that from the dao directly when you
> > > > want,
> > > > or
> > > > when you unlock a vm/template.
> > > > when you will update the db_generation, the vm/template would
> > > > be
> > > > picked up by OvfUpdater as it's db_generation is different
> > > > than
> > > > its
> > > > ovf_generation.
> > > 1. Update of ovfs is not related to locks and it should not be,
> > > I
> > > don't understand why
> > > you link them.
> > > 2. Races - your automatic process and couple of add disk
> > > commands.
Sorry, part of my response was cutted by mistake, adding it
As of today - there is a race.
After adding ovfautoupdater , you won't update the ovf for vm which is locked or has a
locked disk (written in the wiki).
ovfautoupdater won't introduce nor keep that race as we should have
db exclusive row lock when we perform this update so even if we had context switch and we
later returned to the update process, we will run the update again as we don't send
the db_generation value from the engine, but only incrementing in the db. correct
me if i'm wrong.
> > > You will miss update
> > > of ovf.
> > > 3. Also when you will add increase of generation it is not
> > > clear
> > > from
> > > your design
> >
> > 1+2 The OvfAutoUpdater is not related and doesn't acquire any
> > locks.
> >
> > as of today, if you call updateVmInSpm without performing the
> > operations with lock on the vm you'll have a race - for example,
> > in
> Liron, updateVmInSpm is called at endSuccessfully, you can not
> build
> on locks,
> especially on in memory locks.
> > addDisk (shared calls, two updateVmInSpm might be executed in the
> > same time and you can't know with with ovf you'll end).
> >
> > therefore, when updating the ovf_generation you should have a
> > lock
> > somewhere otherwise you might miss updates because of races.
> > if today updatevminspm is called without locks and have races, it
> > needs to be fixed unrelated to it.
> Liron, you should fix these. These is a new development, it should
> fix
> all known bugs and problems and not introduce a new ones
answered below,
As of today - there is a race.
ovfautoupdater won't introduce nor keep that race as we should have
automatic db exclusive row lock when we perform this update, correct
me if i'm wrong.
>
> > 3. you will increase the generation whenever you made
> > updateVmInSpm.
> > > > > > >
> > > > > > > > > Also consider snapshots & vNICs which
affect the VM
> > > > > > > > > state.
> > > > > > > > Whatever affect the vm state and triggers
> > > > > > > > updateVmInSpm
> > > > > > > > today
> > > > > > > > will
> > > > > > > > continue to, it can be added to flows which
don't do
> > > > > > > > that
> > > > > > > > today.
> > > > > > > > >
> > > > > > > > > > 4. Reconstruct/Recovery - updateVmInSpm
should be
> > > > > > > > > > called
> > > > > > > > > > on
> > > > > > > > > > all
> > > > > > > > > > vms,
> > > > > > > > > > no matter if they were updated.
> > > > > > > >
> > > > > > > > Ofcourse, it's being taken care of.
> > > > > > > How?
> > > > > > we don't have ovf's of the vms when we reconstruct
as we
> > > > > > don't
> > > > > > have
> > > > > > master domain - so we should set the ovf_generation of
> > > > > > the
> > > > > > vms
> > > > > > to
> > > > > > be
> > > > > > 0 - which will cause ovfautoupdater to copy the vm
> > > > > > metadata.
> > > > > I did not understand that statement and how it is related.
> > > > OvfAutoUpdater will get those vms for update as db_generation
> > > > is
> > > > different than ovf_generation.
> > > > > > a performance improvement may be introduced later on in
> > > > > > case
> > > > > > of
> > > > > > wrong
> > > > > > master version, but that's not related to the
> > > > > > ovfautoupdater.
> > > > > I don't understand connection to performance.
> > > > > Now you are replacing a calls to VmCommand.updateVmInSpm()
> > > > > one
> > > > > of
> > > > > them it
> > > > > is at reconstruct flow, and you should handle it also and
> > > > > add
> > > > > it
> > > > > to
> > > > > wiki.
> > > > answered below, OvfAutoUpdater will get those vms for update
> > > > as
> > > At reconstruct all vms are updated. So it is not related to
> > > generation.
> > > How you handle these case?
> >
> > when doing reconstruct, the ovf_generation of all vms is set to 0
> > as
> > we don't have them on the master domain.
> > ovfupdater will get this vms for update as ovf_generation <
> > db_generation, will copy the vm ovf and set db_genertion to
> > ovf_generation.
> >
> > > > db_generation is different than ovf_generation.
> > > > > > > > > >
> > > > > > > > > > > > Regards,
> > > > > > > > > > > > Mike
> > > > > > > > > > > >
> > > > > > > > > > > > ----- Original Message -----
> > > > > > > > > > > > >
http://wiki.ovirt.org/wiki/Feature/OvfAutoUpdater
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi all, i'll be glad
if you could review
> > > > > > > > > > > > > the
> > > > > > > > > > > > > wiki
> > > > > > > > > > > > > page
> > > > > > > > > > > > > of
> > > > > > > > > > > > > OvfAutoUpdater, if you
have any suggestions
> > > > > > > > > > > > > or
> > > > > > > > > > > > > ideas
> > > > > > > > > > > > > please
> > > > > > > > > > > > > let
> > > > > > > > > > > > > me
> > > > > > > > > > > > > know.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
http://wiki.ovirt.org/wiki/Feature/OvfAutoUpdater
> > > > > > > > > > > > >
> > > > > > > > > > > > > short preview from the
wiki:
> > > > > > > > > > > > > vm/template
configurations (including disks
> > > > > > > > > > > > > info)
> > > > > > > > > > > > > are
> > > > > > > > > > > > > stored
> > > > > > > > > > > > > on
> > > > > > > > > > > > > the
> > > > > > > > > > > > > master storage domain
for backup purposes,
> > > > > > > > > > > > > import/export
> > > > > > > > > > > > > and
> > > > > > > > > > > > > also
> > > > > > > > > > > > > to
> > > > > > > > > > > > > provide the abillity to
run VMs without
> > > > > > > > > > > > > having
> > > > > > > > > > > > > a
> > > > > > > > > > > > > running
> > > > > > > > > > > > > engine/db.
> > > > > > > > > > > > > Currently ovf update is
done synchronously
> > > > > > > > > > > > > when
> > > > > > > > > > > > > performing
> > > > > > > > > > > > > various
> > > > > > > > > > > > > operations on
vms/templates - update, save,
> > > > > > > > > > > > > adding/removing
> > > > > > > > > > > > > a
> > > > > > > > > > > > > disk,
> > > > > > > > > > > > > etc. What's more,
currently updating the
> > > > > > > > > > > > > ovf
> > > > > > > > > > > > > (updateVM
> > > > > > > > > > > > > vdsm
> > > > > > > > > > > > > call)
> > > > > > > > > > > > > is
> > > > > > > > > > > > > usually done within a
transcation.
> > > > > > > > > > > > >
> > > > > > > > > > > > > The idea behined
OvfAutoUpdater is to
> > > > > > > > > > > > > perform
> > > > > > > > > > > > > batch
> > > > > > > > > > > > > ovf
> > > > > > > > > > > > > update
> > > > > > > > > > > > > operations that
aggregate all outstanding
> > > > > > > > > > > > > updates
> > > > > > > > > > > > > per
> > > > > > > > > > > > > data
> > > > > > > > > > > > > center.
> > > > > > > > > > > > > These updates will be
done in specifed time
> > > > > > > > > > > > > intervals
> > > > > > > > > > > > > which
> > > > > > > > > > > > > will
> > > > > > > > > > > > > reduce XML-RPC calls and
will enable the
> > > > > > > > > > > > > removal
> > > > > > > > > > > > > of
> > > > > > > > > > > > > this
> > > > > > > > > > > > > syncronous
> > > > > > > > > > > > > vdsm call from all over
the code.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Liron Aravot.
> > > > > > > > > > > > >
_______________________________________________
> > > > > > > > > > > > > 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