Re: [ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName

----- Original Message ----- > From: "Michal Skrivanek" <mskrivan(a)redhat.com&gt; > To: "Liron Aravot" <laravot(a)redhat.com&gt;, "Dan Kenigsberg" <danken(a)redhat.com&gt;, "Federico Simoncelli" > <fsimonce(a)redhat.com&gt;, "Vinzenz Feenstra" <vfeenstr(a)redhat.com&gt; > Cc: users(a)ovirt.org, devel(a)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(a)redhat.com&gt; wrote: > >> >> >> ----- Original Message ----- >>> From: "Federico Simoncelli" <fsimonce(a)redhat.com&gt; >>> To: devel(a)ovirt.org >>> Cc: "Liron Aravot" <laravot(a)redhat.com&gt;, users(a)ovirt.org, >>> smizrahi(a)redhat.com, "Michal Skrivanek" >>> <mskrivan(a)redhat.com&gt;, "Vinzenz Feenstra" <vfeenstr(a)redhat.com&gt;, "Allon >>> Mureinik" <amureini(a)redhat.com&gt;, "Dan >>> Kenigsberg" <danken(a)redhat.com&gt; >>> Sent: Tuesday, September 2, 2014 12:50:28 PM >>> Subject: Re: feature review - ReportGuestDisksLogicalDeviceName >>> >>> ----- Original Message ----- >>>> From: "Dan Kenigsberg" <danken(a)redhat.com&gt; >>>> To: "Liron Aravot" <laravot(a)redhat.com&gt; >>>> Cc: users(a)ovirt.org, devel(a)ovirt.org, smizrahi(a)redhat.com, >>>> fsimonce(a)redhat.com, "Michal Skrivanek" >>>> <mskrivan(a)redhat.com&gt;, "Vinzenz Feenstra" <vfeenstr(a)redhat.com&gt;, "Allon >>>> Mureinik" <amureini(a)redhat.com&gt; >>>> 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