[ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName

[Nir Soffer]

----- Original Message -----
> From: "Michal Skrivanek" <mskrivan at redhat.com>
> To: "Liron Aravot" <laravot at redhat.com>, "Dan Kenigsberg" <danken at redhat.com>, "Federico Simoncelli"
> <fsimonce at redhat.com>, "Vinzenz Feenstra" <vfeenstr at redhat.com>
> Cc: users at ovirt.org, devel at ovirt.org
> Sent: Tuesday, September 2, 2014 3:29:57 PM
> Subject: Re: [ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName
> 
> 
> On Sep 2, 2014, at 13:11 , Liron Aravot <laravot at redhat.com> wrote:
> 
> > 
> > 
> > ----- Original Message -----
> >> From: "Federico Simoncelli" <fsimonce at redhat.com>
> >> To: devel at ovirt.org
> >> Cc: "Liron Aravot" <laravot at redhat.com>, users at ovirt.org,
> >> smizrahi at redhat.com, "Michal Skrivanek"
> >> <mskrivan at redhat.com>, "Vinzenz Feenstra" <vfeenstr at redhat.com>, "Allon
> >> Mureinik" <amureini at redhat.com>, "Dan
> >> Kenigsberg" <danken at redhat.com>
> >> Sent: Tuesday, September 2, 2014 12:50:28 PM
> >> Subject: Re: feature review - ReportGuestDisksLogicalDeviceName
> >> 
> >> ----- Original Message -----
> >>> From: "Dan Kenigsberg" <danken at redhat.com>
> >>> To: "Liron Aravot" <laravot at redhat.com>
> >>> Cc: users at ovirt.org, devel at ovirt.org, smizrahi at redhat.com,
> >>> fsimonce at redhat.com, "Michal Skrivanek"
> >>> <mskrivan at redhat.com>, "Vinzenz Feenstra" <vfeenstr at redhat.com>, "Allon
> >>> Mureinik" <amureini at redhat.com>
> >>> Sent: Monday, September 1, 2014 11:23:45 PM
> >>> Subject: Re: feature review - ReportGuestDisksLogicalDeviceName
> >>> 
> >>> On Sun, Aug 31, 2014 at 07:20:04AM -0400, Liron Aravot wrote:
> >>>> Feel free to review the the following feature.
> >>>> 
> >>>> http://www.ovirt.org/Features/ReportGuestDisksLogicalDeviceName
> >>> 
> >>> Thanks for posting this feature page. Two things worry me about this
> >>> feature. The first is timing. It is not reasonable to suggest an API
> >>> change, and expect it to get to ovirt-3.5.0. We are two late anyway.
> >>> 
> >>> The other one is the suggested API. You suggest placing volatile and
> >>> optional infomation in getVMList. It won't be the first time that we
> >>> have it (guestIPs, guestFQDN, clientIP, and displayIP are there) but
> >>> it's foreign to the notion of "conf" reported by getVMList() - the set
> >>> of parameters needed to recreate the VM.
> > 
> > The fact is that today we return guest information in list(Full=true), We
> > decide on it's notion
> > and it seems like we already made our minds when guest info was added there
> > :) . I don't see any harm in returning the disk mapping there
> > and if we'll want to extract the guest info out, we can extract all of it
> > in later version (4?) without need for BC. Having
> > the information spread between different verbs is no better imo.
> >> 
> >> At first sight this seems something belonging to getVmStats (which
> >> is reporting already other guest agent information).
> >> 
> > 
> > Fede, I've mentioned in the wiki, getVmStats is called by the engine every
> > few seconds and therefore that info
> > wasn't added there but to list() which is called only when the hash is
> > changed. If everyone is in for that simple
> > solution i'm fine with that, but Michal/Vincenz preferred it that way.
> 
> yes, that was the main reason me and Vinzenz suggested to use list(). 15s is
> a reasonable compromise, IMHO.
> And since it's also reported by guest agent in a similar manner (and actually
> via the same vdsm<->ga API call) as other guest information I think it
> should sit alongside guestIPs, FQDN, etc…
> 
> Maybe not the best place, but I would leave that for a bigger discussion
> if/when we want to refactor reporting of the guest agent information

Given that we are late, adding disk mapping where other guest info is
in a backward compatible way looks reasonable.

Did you consider adding a new verb for getting guest information? This 
verb can be called once after starting/recovering a vm, and then only when
guest info hash changes (like the xml hash).

Nir