[Engine-devel] new engine watchdog version

Tue Mar 12 14:03:39 UTC 2013

----- Original Message -----
> From: "Omer Frenkel" <ofrenkel at redhat.com>
> To: "Laszlo Hornyak" <lhornyak at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>
> Sent: Tuesday, March 12, 2013 2:41:38 PM
> Subject: Re: [Engine-devel] new engine watchdog version
> 
> 
> 
> ----- Original Message -----
> > From: "Laszlo Hornyak" <lhornyak at redhat.com>
> > To: "Omer Frenkel" <ofrenkel at redhat.com>
> > Cc: "engine-devel" <engine-devel at ovirt.org>
> > Sent: Monday, March 11, 2013 6:46:06 PM
> > Subject: Re: [Engine-devel] new engine watchdog version
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Omer Frenkel" <ofrenkel at redhat.com>
> > > To: "Laszlo Hornyak" <lhornyak at redhat.com>
> > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > Sent: Monday, March 11, 2013 1:25:39 PM
> > > Subject: Re: [Engine-devel] new engine watchdog version
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Laszlo Hornyak" <lhornyak at redhat.com>
> > > > To: "Omer Frenkel" <ofrenkel at redhat.com>
> > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > Sent: Monday, March 11, 2013 12:15:39 PM
> > > > Subject: Re: [Engine-devel] new engine watchdog version
> > > > 
> > > > 
> > > > 
> > > > ----- Original Message -----
> > > > > From: "Omer Frenkel" <ofrenkel at redhat.com>
> > > > > To: "Laszlo Hornyak" <lhornyak at redhat.com>
> > > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > > Sent: Monday, March 11, 2013 11:12:48 AM
> > > > > Subject: Re: [Engine-devel] new engine watchdog version
> > > > > 
> > > > > 
> > > > > 
> > > > > ----- Original Message -----
> > > > > > From: "Laszlo Hornyak" <lhornyak at redhat.com>
> > > > > > To: "Omer Frenkel" <ofrenkel at redhat.com>
> > > > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > > > Sent: Monday, March 11, 2013 9:59:53 AM
> > > > > > Subject: Re: [Engine-devel] new engine watchdog version
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > ----- Original Message -----
> > > > > > > From: "Omer Frenkel" <ofrenkel at redhat.com>
> > > > > > > To: "Laszlo Hornyak" <lhornyak at redhat.com>
> > > > > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > > > > Sent: Sunday, March 10, 2013 8:36:46 AM
> > > > > > > Subject: Re: [Engine-devel] new engine watchdog version
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ----- Original Message -----
> > > > > > > > From: "Laszlo Hornyak" <lhornyak at redhat.com>
> > > > > > > > To: "engine-devel" <engine-devel at ovirt.org>
> > > > > > > > Sent: Friday, March 8, 2013 7:18:59 PM
> > > > > > > > Subject: [Engine-devel] new engine watchdog version
> > > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > I uploaded a new version of the watchdog patch. This
> > > > > > > > patch
> > > > > > > > is
> > > > > > > > still
> > > > > > > > a
> > > > > > > > work in progress, it adds audit log alerts to the
> > > > > > > > functionality.
> > > > > > > > http://gerrit.ovirt.org/12419/
> > > > > > > > 
> > > > > > > > Feature page:
> > > > > > > > http://www.ovirt.org/Features/Watchdog_engine_support
> > > > > > > > 
> > > > > > > > Laszlo
> > > > > > > > _______________________________________________
> > > > > > > > Engine-devel mailing list
> > > > > > > > Engine-devel at ovirt.org
> > > > > > > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > > > > > > 
> > > > > > > 
> > > > > > > Hi,
> > > > > > > i looked at the patch and there is something i don't
> > > > > > > understand,
> > > > > > > i see you are treating the watchdog as a vm device, which
> > > > > > > is
> > > > > > > great,
> > > > > > > so why do we need to save the device details in vm_static
> > > > > > > table
> > > > > > > in
> > > > > > > addition to the vm_devices?
> > > > > > > i think its even not used at all (only setting the device
> > > > > > > in
> > > > > > > command
> > > > > > > which could be parameters, no need to persist)
> > > > > > > 
> > > > > > 
> > > > > > Hi Omer,
> > > > > > 
> > > > > > Thanks, I hoped someone will come up with that question :)
> > > > > > The
> > > > > > answer
> > > > > > is that I followed the established design patterns in the
> > > > > > backend.
> > > > > > See smartcard and memory balloon, probably others. The
> > > > > > motivation
> > > > > > for this pattern could be that in case of these devices,
> > > > > > you
> > > > > > must
> > > > > > have the settings in the VM data, not separately in the
> > > > > > devices.
> > > > > > Also when vdsbroker builds the devices list, it just asks
> > > > > > the
> > > > > > device
> > > > > > list. The redundancy is already there, we can make it
> > > > > > differently
> > > > > > in
> > > > > > this case but that will present the readers with a puzzle:
> > > > > > why
> > > > > > this
> > > > > > pattern in feature X, why that pattern in feature Y...
> > > > > > So I would recommend to leave it like this for now and
> > > > > > schedule
> > > > > > a
> > > > > > cleanup on device handling. Devices deserve a cleanup.
> > > > > > 
> > > > > > Thx,
> > > > > > Laszlo
> > > > > > 
> > > > > 
> > > > > i agree there is a mess that requires clean-up,
> > > > > but i don't think its a good thing to keep piling up the
> > > > > mess,
> > > > > i don't like it that smartcard is there, but some other
> > > > > devices
> > > > > are
> > > > > ok (balloon and payload)
> > > > > so we already have 2 'patterns', lets go with the right one..
> > > > > and answering also @Doron's question - yes the device data
> > > > > should
> > > > > be
> > > > > kept with the device
> > > > > 
> > > > 
> > > > Ok, I may have missed the other pattern, could you explain
> > > > which
> > > > one
> > > > do you mean?
> > > > Balloon does not very different from smartcard, it is there in
> > > > VM.
> > > > 
> > > 
> > > the difference is that balloon is not in vm_static table at all
> > > (the
> > > only place in the db for it is in vm_devices)
> > > and smartcard has 'is_smartcard_enabled' field in vm_static in
> > > addition to vm_devices (which is not needed..)
> > 
> > Ok, so what you want is that
> > - the engine should query the devices each time the VM record is
> > set
> > (from DAO's or Action)
> > XOR
> > - the client code (rest-api and frontend) should query the devices
> > to
> > figure out if the watchdog is there
> > 
> 
> i prefer this approach, as we do with other sub-collections of vms
> (disks,networks..)

You get these sub-collections with another http request. E.g. /api/vms/<GUID>/disks and then /api/vms/<GUID>/notworks
The difference is that watchdog is not a sub-collection, it is in the VM structure. So I guess I would have to add some extra query to the mapping code of the rest-api.
Michael? Is this ok for you?

> but if we don't expose devices from the engine, so we need some other
> way of doing it
> (client specific query for "is XXX device enabled?" or engine set it
> in the VM record as you suggested.
> 
> > > 
> > > the way i think we (currently) need to work with devices is:
> > > add a parameter for it in the parameters, and use it in
> > > add/update
> > > (/run-once?) (as done for balloon)
> > 
> > run once for watchdog? why?
> > 
> 
> for watchdog probably not, i meant in general with devices

ok, that's in the SEP-field for now

> 
> > > i don't know what is the use of the field balloonEnabled in VM, i
> > > don't see any use of it..
> > 
> > It is a write-only property.
> > 
> > > 
> > > going forward we need to think if we want to expose devices to
> > > frontend,
> > > so then we can drop the encapsulation and just use list of
> > > devices
> > > in
> > > VmBase or something like that
> > > 
> > 
> > 
> 