Re: [ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName
On Wed, Sep 03, 2014 at 08:50:12AM +0200, Michal Skrivanek wrote: On Sep 2, 2014, at 21:03 , Nir Soffer nsof...@redhat.com wrote: - Original Message - From: Michal Skrivanek mskri...@redhat.com To: Liron Aravot lara...@redhat.com, Dan Kenigsberg dan...@redhat.com, Federico Simoncelli fsimo...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com Cc: us...@ovirt.org, devel@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 lara...@redhat.com wrote: - Original Message - From: Federico Simoncelli fsimo...@redhat.com To: devel@ovirt.org Cc: Liron Aravot lara...@redhat.com, us...@ovirt.org, smizr...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@redhat.com, Dan Kenigsberg dan...@redhat.com Sent: Tuesday, September 2, 2014 12:50:28 PM Subject: Re: feature review - ReportGuestDisksLogicalDeviceName - Original Message - From: Dan Kenigsberg dan...@redhat.com To: Liron Aravot lara...@redhat.com Cc: us...@ovirt.org, devel@ovirt.org, smizr...@redhat.com, fsimo...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@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(). ___ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
Re: [ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName
On Sep 2, 2014, at 21:03 , Nir Soffer nsof...@redhat.com wrote: - Original Message - From: Michal Skrivanek mskri...@redhat.com To: Liron Aravot lara...@redhat.com, Dan Kenigsberg dan...@redhat.com, Federico Simoncelli fsimo...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com Cc: us...@ovirt.org, devel@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 lara...@redhat.com wrote: - Original Message - From: Federico Simoncelli fsimo...@redhat.com To: devel@ovirt.org Cc: Liron Aravot lara...@redhat.com, us...@ovirt.org, smizr...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@redhat.com, Dan Kenigsberg dan...@redhat.com Sent: Tuesday, September 2, 2014 12:50:28 PM Subject: Re: feature review - ReportGuestDisksLogicalDeviceName - Original Message - From: Dan Kenigsberg dan...@redhat.com To: Liron Aravot lara...@redhat.com Cc: us...@ovirt.org, devel@ovirt.org, smizr...@redhat.com, fsimo...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@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 ___ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
Re: [ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName
On Sep 2, 2014, at 13:11 , Liron Aravot lara...@redhat.com wrote: - Original Message - From: Federico Simoncelli fsimo...@redhat.com To: devel@ovirt.org Cc: Liron Aravot lara...@redhat.com, us...@ovirt.org, smizr...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@redhat.com, Dan Kenigsberg dan...@redhat.com Sent: Tuesday, September 2, 2014 12:50:28 PM Subject: Re: feature review - ReportGuestDisksLogicalDeviceName - Original Message - From: Dan Kenigsberg dan...@redhat.com To: Liron Aravot lara...@redhat.com Cc: us...@ovirt.org, devel@ovirt.org, smizr...@redhat.com, fsimo...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel ___ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
Re: [ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName
- Original Message - From: Michal Skrivanek mskri...@redhat.com To: Liron Aravot lara...@redhat.com, Dan Kenigsberg dan...@redhat.com, Federico Simoncelli fsimo...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com Cc: us...@ovirt.org, devel@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 lara...@redhat.com wrote: - Original Message - From: Federico Simoncelli fsimo...@redhat.com To: devel@ovirt.org Cc: Liron Aravot lara...@redhat.com, us...@ovirt.org, smizr...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@redhat.com, Dan Kenigsberg dan...@redhat.com Sent: Tuesday, September 2, 2014 12:50:28 PM Subject: Re: feature review - ReportGuestDisksLogicalDeviceName - Original Message - From: Dan Kenigsberg dan...@redhat.com To: Liron Aravot lara...@redhat.com Cc: us...@ovirt.org, devel@ovirt.org, smizr...@redhat.com, fsimo...@redhat.com, Michal Skrivanek mskri...@redhat.com, Vinzenz Feenstra vfeen...@redhat.com, Allon Mureinik amure...@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 ___ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
Re: [ovirt-devel] 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. Would you describe the motivation for using this verb? What are the pros and cons? Which other ideas where considered? Regards, Dan. ___ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
[ovirt-devel] feature review - ReportGuestDisksLogicalDeviceName
Feel free to review the the following feature. http://www.ovirt.org/Features/ReportGuestDisksLogicalDeviceName ___ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
