[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 1/2] libxl: postpone backend name resolution
On 04/15/2013 10:55 AM, Roger Pau Monné wrote: On 12/04/13 17:22, Daniel De Graaf wrote:This adds a backend_domname field in libxl devices that contain a backend_domid field, allowing either a domid or a domain name to be specified in the configuration structures. The domain name is resolved into a domain ID in the _setdefault function when adding the device. This change allows the backend of the block devices to be specified (which previously required passing the libxl_ctx down into the block device parser), and will simplify specification of backend domains in other users of libxl. Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx> --- This patch does not include the changes to tools/libxl/libxlu_disk_l.c and tools/libxl/libxlu_disk_l.h because the diffs contain unrelated changes due to different generator versions. Changes since v3: - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h - Create libxl_domain_qualifier_to_domid function in libxl_util.h docs/misc/xl-disk-configuration.txt | 12 ++++++ tools/libxl/libxl.c | 17 +++++++-- tools/libxl/libxl.h | 12 ++++++ tools/libxl/libxl_types.idl | 5 +++ tools/libxl/libxl_utils.c | 19 ++++++++++ tools/libxl/libxl_utils.h | 1 + tools/libxl/libxlu_disk_l.l | 1 + tools/libxl/xl_cmdimpl.c | 75 ++++++++----------------------------- 8 files changed, 78 insertions(+), 64 deletions(-) diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt index 86c16be..5bd456d 100644 --- a/docs/misc/xl-disk-configuration.txt +++ b/docs/misc/xl-disk-configuration.txt @@ -139,6 +139,18 @@ cdrom Convenience alias for "devtype=cdrom". +backend=<domain-name> +--------------------- + +Description: Designates a backend domain for the device +Supported values: Valid domain names +Mandatory: No + +Specifies the backend domain which this device should attach to. This +defaults to domain 0. Specifying another domain requires setting up a +driver domain which is outside the scope of this document. + + backendtype=<backend-type> -------------------------- diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 572c2c6..b994fea 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device) return nextid; } +static int libxl__resolve_domid(libxl__gc *gc, const char *name, + uint32_t *domid) +{ + if (!name) + return 0;This is just my opinion, but I find this function ambiguous, it will return 0 (success), if either name is NULL, or name != NULL and it can be resolved to a domid, which means that even when returning 0 the value in *domid might be uninitialized (if name == NULL). I would rather do something like if (!name || !domid) return ERROR_INVAL; And make sure callers only call this function if there's a real need to resolve a domid. This is an internal function which is called only when there is a real need to resolve a domid. The value of domid is never uninitialized when calling this function; it is always the value of backend_domid, and retains that value when backend_domname is not specified. Also, do we check somewhere that both backend_domid and backend_domname are not specified at the same time? Since backend_domid is an integer that defaults to 0 (which is a valid ID), it's impossible to tell if the 0 was specified or if it was simply the default. This is resolved by having backend_domname take precedence over backend_domid when it is present. -- Daniel De Graaf National Security Agency _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |