7:20 a.m.
Feel free to review the the following feature.
http://www.ovirt.org/Features/ReportGuestDisksLogicalDeviceName
5:23 p.m.
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.
Would you describe the motivation for using this verb? What are the
pros and cons? Which other ideas where considered?
Regards,
Dan.
5:50 a.m.
----- 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.
At first sight this seems something belonging to getVmStats (which
is reporting already other guest agent information).
--
Federico
7:11 a.m.
----- 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.
Federico
8:29 a.m.
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
Thanks,
michal
> Federico
>
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
3: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
2:50 a.m.
On Sep 2, 2014, at 21:03 , Nir Soffer <nsoffer(a)redhat.com> wrote:
----- 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).
yes, was considered but turned down.
Main reason is an additional overhead and pollution of the API for a tiny little item
which is in the same group of guest agent reported items. It doesn't make sense to
introduce an inconsistency just this one item.
Refactoring of the frequency of what we report is indeed long overdue, but we
shouldn't start here…(the first and most offending is still the application list)
Thanks,
michal
Nir
12:18 p.m.
On Wed, Sep 03, 2014 at 08:50:12AM +0200, Michal Skrivanek wrote:
On Sep 2, 2014, at 21:03 , Nir Soffer <nsoffer(a)redhat.com> wrote:
> ----- 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).
yes, was considered but turned down.
Main reason is an additional overhead and pollution of the API for a tiny little item
which is in the same group of guest agent reported items. It doesn't make sense to
introduce an inconsistency just this one item.
Refactoring of the frequency of what we report is indeed long overdue, but we
shouldn't start here…(the first and most offending is still the application list)
Ok, lacking another alternative, let's dump the maps to list().
3755
days inactive
3768
days old
7 comments
5 participants
participants (5)
-
Dan Kenigsberg -
Federico Simoncelli -
Liron Aravot -
Michal Skrivanek -
Nir Soffer