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