Re: [Xen-devel] [PATCH v4 --for 4.6 COLOPre 05/25] libxl/remus: introduce libxl__remus_setup

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote:
> Refactoring Remus setup by introducing libxl__remus_setup API.
> All Remus setup work are done in this function.
> 
> Also remove the libxl__ prefix for static functions.

There is a subtle behavioural change here, which is that if anything
which is now done in _setup fails then the result is a call to
dss->callback( ..,..,ERROR_FAIL) rather than _start returning
AO_CREATE_FAIL(ERROR_FAIL).

I think this is probably a reasonable and correct change, but I think it
is worth mentioning in the commit log.

That said, I also wonder if the actual check for netbuffer_enabled (the
only such failure in practice) ought to be moved up such that it stays
in _start along with the other similar checks, i.e. _start would do:

    if (libxl_defbool_val(info->netbuf) && !libxl__netbuffer_enabled(gc)) {
            LOG(ERROR, "Remus: No support for network buffering");
            rc = ERROR_FAIL;
            goto out;
        }

while _setup would do:

    if (libxl_defbool_val(info->netbuf)) {
        // MAYBE : assert(libxl__netbuffer_enabled(gc))
        rds->device_kind_flags |= (1 << LIBXL__DEVICE_KIND_VIF);
    }

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel