Re: [libvirt] [libvirt-glib] Add gvir_domain_get_saved()
On Fri, Feb 17, 2012 at 1:01 AM, Daniel P. Berrange <berrange(a)redhat.com> wrote:
On Thu, Feb 16, 2012 at 10:16:11PM +0100, Christophe Fergeau wrote:
> On Thu, Feb 16, 2012 at 09:51:34AM +0100, Christophe Fergeau wrote:
> > On Thu, Feb 16, 2012 at 02:26:02AM +0200, Zeeshan Ali (Khattak) wrote:
> > > + */
> > > +gboolean gvir_domain_get_saved(GVirDomain *dom)
> >
> > The naming needs to be more explicit, libvirt will suspend the domain after
> > a call to virDomainSave or virDomainManagedSave, the current name only
> > checks for the latter state. I'd go for
> > gvir_domain_has_managed_save_image();
>
> I see this patch has been pushed to master with this part unchanged and no
> discussion whatsoever on the list, what happened there?
Oh, I ACK'd his new patch, without noticing your message in this
thread.
I tend to agree with your suggestion to rename to
gvir_domain_has_managed_save_image()
I disagree. I think we'd want a property for this too and that means
we need to keep the 'get' prefix and the string after that should
correspond the property name (We already discussed at length why this
is needed and we are already doing it for other boolean getters so
lets not have the discussion about this need, again). I think
gvir_domain_get_saved() might not be descriptive enough but it doesn't
cause any confusion either so if its purpose is not clear from the
signature, developer will read the (a few words of) docs for it and
know what it does.
Another reason I don't like the suggested name is that its giving out
details that I dont want app developer to care/know about: image. What
user needs to care is whether or not the domain in question has a
saved state or not that it could be restored to, that is it. He
doesn't even need to know what is 'managed' cause we are only
providing APIs for 'managed' saving.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124