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

Re: [Xen-devel] [PATCH RESEND v3 1/2] libxl: postpone backend name resolution



On 04/12/2013 10:05 AM, Ian Campbell wrote:
On Thu, 2013-03-14 at 14:23 +0000, 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.

please can we get a #define LIBXL_HAVE_DEVICE_BACKEND_NAME (or something
similar) in libxl.h so applications can tell that it is safe to use
this.

OK; I will use LIBXL_HAVE_DEVICE_BACKEND_DOMNAME to match the new field.


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.

  docs/misc/xl-disk-configuration.txt | 12 +++++++++
  tools/libxl/libxl.c                 | 49 ++++++++++++++++++++++++++++++++++++-
  tools/libxl/libxl_types.idl         |  5 ++++
  tools/libxl/libxlu_disk_l.l         |  1 +
  tools/libxl/xl_cmdimpl.c            | 36 ++++++---------------------
  5 files changed, 73 insertions(+), 30 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 73e0dc3..e442afc 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1730,12 +1730,35 @@ static int libxl__device_nextid(libxl__gc *gc, uint32_t 
domid, char *device)
      return nextid;
  }

+/* backend domain name-to-domid conversion utility */
+static int libxl__resolve_domain(libxl__gc *gc, const char *name)
+{
+    int i, rv;
+    uint32_t domid;
+    for (i=0; name[i]; i++) {
+        if (!isdigit(name[i])) {
+            rv = libxl_name_to_domid(libxl__gc_owner(gc), name, &domid);
+            if (rv)
+                return rv;
+            return domid;
+        }
+    }

What are the constraints on the names of domains? I suppose it is
toolstack specific but the xl docs jsut say it has to be unique.

I'd suggest a simple rule like they cannot start with a digit, that
would make the loop in the above unnecessary. In any case the loop is a
bit odd since AFAICT it will treat "123boo" as "boo", I think it should
either treat the entire string as a name or a number.

Actually, this will treat "123boo" properly, as name (not name+i) is passed
to libxl_name_to_domid.

I think you should just move domain_qualifier_to_domid  from xl to libxl
and use that...

That would also work, although it would need a better name:
libxl_qualifier_to_domid since libxl_name_to_domid is already used?

+    return atoi(name);
+}
+
  
/******************************************************************************/
  int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
  {
+   int rc;
     if(libxl_uuid_is_nil(&vtpm->uuid)) {
        libxl_uuid_generate(&vtpm->uuid);
     }
+   if (vtpm->backend_domname) {
+       rc = libxl__resolve_domain(gc, vtpm->backend_domname);
+       if (rc < 0)
+           return rc;
+       vtpm->backend_domid = rc;
+   }

How about
        rc = libxl__resole_domain(gc, vtpm->backend_domname, 
&vtpm->backend_domid);
        if (rc < 0)
                return rc;
?

Then the if (...) which is repeated could be in the function and it'd be
easier to keep the logic the same for all of them.

Ian.


This would conflict a bit with making libxl__resolve_domain public, although I
guess the function could note that it does nothing if passed a NULL string
which is all that is needed here.

--
Daniel De Graaf
National Security Agency

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.