[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 07/16] save: wire up trivial save/restore flags implementations



On Tue, Jul 19, 2011 at 10:20:30PM -0600, Eric Blake wrote:
> For all hypervisors that support save and restore, the new API
> now performs the same functions as the old.
> 
> VBox is excluded from this list, because its existing domainsave
> is broken (there is no corresponding domainrestore, and there
> is no control over the filename used in the save).  A later
> patch should change vbox to use its implementation for
> managedsave, and teach start to use managedsave results.
> 
> * src/libxl/libxl_driver.c (libxlDomainSave): Move guts...
> (libxlDomainSaveFlags): ...to new function.
> (libxlDomainRestore): Move guts...
> (libxlDomainRestoreFlags): ...to new function.
> * src/test/test_driver.c (testDomainSave, testDomainSaveFlags)
> (testDomainRestore, testDomainRestoreFlags): Likewise.
> * src/xen/xen_driver.c (xenUnifiedDomainSave)
> (xenUnifiedDomainSaveFlags, xenUnifiedDomainRestore)
> (xenUnifiedDomainRestoreFlags): Likewise.
> * src/qemu/qemu_driver.c (qemudDomainSave, qemudDomainRestore):
> Rename and move guts.
> (qemuDomainSave, qemuDomainSaveFlags, qemuDomainRestore)
> (qemuDomainRestoreFlags): ...here.
> (qemudDomainSaveFlag): Rename...
> (qemuDomainSaveInternal): ...to this, and update callers.
> ---
> 
> v2: merge 3 and 13 of v1, drop vbox support
> 
>  src/libxl/libxl_driver.c |   34 ++++++++++++++++++++++++++++-
>  src/qemu/qemu_driver.c   |   52 ++++++++++++++++++++++++++++++++++++++--------
>  src/test/test_driver.c   |   42 +++++++++++++++++++++++++++++++++---
>  src/xen/xen_driver.c     |   34 ++++++++++++++++++++++++++++-
>  4 files changed, 145 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 381d90b..e84fa36 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1906,12 +1906,20 @@ cleanup:
>  }
> 
>  static int
> -libxlDomainSave(virDomainPtr dom, const char *to)
> +libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
> +                     unsigned int flags)
>  {
>      libxlDriverPrivatePtr driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      int ret = -1;
> 
> +    virCheckFlags(0, -1);
> +    if (dxml) {
> +        libxlError(VIR_ERR_INVALID_ARG, "%s",
> +                   _("xml modification unsupported"));
> +        return -1;
> +    }

I don't particularly like the idea of using VIR_ERR_INVALID_ARG here
since that's used to indicate illegal use of an API. This usage is
legal, but unsupported.

VIR_ERR_CONFIG_UNSUPPORTED is not quite right and VIR_ERR_NO_SUPPORT
is used to indicate complete lack of the driver API impl.

So perhaps we should have an VIR_ERR_ARGUMENT_UNSUPPORTED ?


ACK to the patch with a different error code used.

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]