[libvirt] [PATCH 01/15] Introduce virDomainSuspendFlags
Daniel P. Berrange
berrange at redhat.com
Tue Feb 4 10:14:44 UTC 2014
On Tue, Feb 04, 2014 at 08:37:28AM +0100, Michal Privoznik wrote:
> On 04.02.2014 08:05, Martin Kletzander wrote:
> >On Mon, Feb 03, 2014 at 10:12:58AM -0700, Eric Blake wrote:
> >>On 02/03/2014 09:16 AM, Michal Privoznik wrote:
> >>>So far, we have just bare virDomainSuspend() API that suspends a domain.
> >>>However, in the future there might occur a case, in which we may want
> >>>to modify suspend behavior slightly. In that case, @flags are useful.
> >>>
> >>>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> >>>---
> >>> include/libvirt/libvirt.h.in | 2 ++
> >>> src/driver.h | 5 +++++
> >>> src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++----
> >>> src/libvirt_public.syms | 5 +++++
> >>> src/remote/remote_driver.c | 1 +
> >>> src/remote/remote_protocol.x | 13 +++++++++++-
> >>> src/remote_protocol-structs | 5 +++++
> >>> 7 files changed, 75 insertions(+), 5 deletions(-)
> >>
> >>Back when we added virDomainShutdownFlags in 0.9.10, I asked if we
> >>should also add *Flags variants for remaining functions without them; at
> >>the time, we decided against it, but I'm not quite sure why.
> >>
> >>I'm perfectly fine with adding this for the sake of making future
> >>additions easier, even if we don't have a use for the flags now - it's
> >>easier to support a flag than it is to rebase to pick up a new function
> >>for any situation where the .so contains a flags function, but it may be
> >>worth getting a second opinion before pushing, if you don't have a plan
> >>to use flags right away.
> >>
> >
> >I like this approach as there are many issues that can be easily
> >solved in case there is a 'Flags' version of some API. That's why we
> >advocate usage of a flags parameter in new APIs even when it is not
> >yet used.
> >
> >Although I was wondering whether it would be too much overkill to use
> >'Params' instead of 'Flags' as Jiri did with migrations as that has
> >way more power. And that's for both new APIs and this change proposed
> >by Michal.
> >
>
> I think migration API is different to these ones. I mean, with
> migration you want to pass tons of arguments (from spoofing domain
> XML, through changing domain name on the dst or graphic listen
> address, to limiting migration bandwidth). However, with suspend or
> resume it's different. We haven't been missing even flags till now,
> nor Typed Params.
>
> For instance, one usage scenario (and this answers Dan's question):
> At the resume, when domain's CPUs were not running for a certain
> period of time (and guest basically doesn't know nothing about it),
> users might want to resync the guest time. There's currently one
> approach being discussed on the qemu-devel: The qemu-ga has this
> guest-set-time which requires a time to be passed (currently). With
> the approach, the time argument becomes optional, so that whenever
> it is not passed with the command, the qemu-ga reads current time
> from guest's RTC.
> So in libvirt we could then just:
>
> virDomainResume(dom, VIR_DOMAIN_RESUME_SYNC_TIME);
>
> or something. And for virDomainSupsend I don't have any particular
> example, but since suspend and resume are a pair, I've changed both
> of them.
So there's interesting, and I'm not sure I entirely agree about
adding flags here. It seems to me that if the QEMU agent has a
"set-time" capability we'd be better off having an explicit API
virDomainSetTime(...). The action of resume + set time cannot
be atomic so I don't see any point in overloading the "set time"
functionality into the resume API call. Just let apps call the
set time method if they so desire.
> tl;dr - TypedParams are bit overkill IMO.
Agreed, TypedParams are also pretty nasty to work with as an
application developer, so we should only use them where absolutely
required.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list