[PATCH 0/3] Provide mechanism to set UUID for Dom0

[PATCH 0/3] Provide mechanism to set UUID for Dom0

[Wei Liu]

Wei Liu (3):
  tools: update examples/README
  tools/helpers: make gen_stub_json_config accept an UUID argument
  xen-init-dom0: set Dom0 UUID if requested

 tools/examples/Makefile              |  1 +
 tools/examples/README                | 26 ++++++--------
 tools/examples/dom0-uuid             |  0
 tools/helpers/Makefile               |  3 +-
 tools/helpers/init-dom-json.c        |  5 ++-
 tools/helpers/init-dom-json.h        |  3 +-
 tools/helpers/init-xenstore-domain.c |  2 +-
 tools/helpers/xen-init-dom0.c        | 67 ++++++++++++++++++++++++++++++++++--
 8 files changed, 84 insertions(+), 23 deletions(-)
 create mode 100644 tools/examples/dom0-uuid

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/3] tools: update examples/README

[Wei Liu]

This file gets installed to the host system.

This patch cleans it up: 1. remove things that don't exist anymore; 2.
change xm to xl; 3. fix xen-devel list address; 4. add things that are
missing.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/examples/README | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/tools/examples/README b/tools/examples/README
index 13380a48eb..80a4652b06 100644
--- a/tools/examples/README
+++ b/tools/examples/README
@@ -7,13 +7,13 @@ incorporate code from them into your own scripts.
 
 If you write a useful script and would like to share it, please do
 send it (preferably with a little summary to go in this file) to
-<xen-devel@lists.sourceforge.net> so we can add it to this directory.
+<xen-devel@lists.xenproject.org> so we can add it to this directory.
 
 block               - called by xen-backend.agent to bind/unbind dev  
 block-common.sh     - sourced by block, block-*
 block-enbd          - binds/unbinds network block devices
 block-nbd           - binds/unbinds network block devices
-cpupool             - example configuration script for 'xm cpupool-create'
+cpupool             - example configuration script for 'xl cpupool-create'
 external-device-migrate - called by xend for migrating external devices
 locking.sh          - locking functions to prevent concurrent access to
                       critical sections inside script files
@@ -22,20 +22,12 @@ vif-bridge          - virtual network start/stop script in bridged mode
 vif-common.sh       - sourced by vif-bridge 
 vif-nat             - xen virtual network start/stop script in NAT mode 
 vif-route           - xen virtual network start/stop script in routed mode
-xen-backend.agent   - calls block, vif-* scripts to add, remove, hotplug
-                      devices  
 xen-hotplug-common.sh - sourced by vif-common.sh
 xen-network-common.sh - sourced by vif-common.sh
 xen-script-common.sh  - sourced by xen-hotplug-common.sh
-xmexample1          - example configuration script for 'xm create'
-xmexample2          - a more complex configuration script for 'xm create'
-xmexample3          - an advanced configuration script for 'xm create' 
-                      that utilizes the vmid
-xmexample.nbd       - configuration script that uses NBD filesystems
-xmexample.hvm       - a configuration script for creating a hvm domain with
-                      'xm create'
-xmexample.hvm-stubdom - a configuration script for creating a hvm domain with
-                        'xm create' that utilizes a stubdomain for device model
-xmexample.pv-grub   - a configuration script for creating a domain with 'xm create'
-                      which boots PV-GRUB.
-xmexample.vti       - a configuration script for creating a domain on vti
+oxenstored.conf     - configuration file for oxenstored
+xl.conf             - configuration file for xl
+xlexample.hvm       - a configuration script for creating a hvm domain with
+                      'xl create'
+xlexample.pvlinux   - a configuration script for creating a pv domain with
+                      'xl create'
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/3] tools/helpers: make gen_stub_json_config accept an UUID argument

[Wei Liu]

If that's set, the stub is going to contain that UUID.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/helpers/init-dom-json.c        | 5 ++++-
 tools/helpers/init-dom-json.h        | 3 ++-
 tools/helpers/init-xenstore-domain.c | 2 +-
 tools/helpers/xen-init-dom0.c        | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/helpers/init-dom-json.c b/tools/helpers/init-dom-json.c
index 704e7cb4f0..9514b3ceb6 100644
--- a/tools/helpers/init-dom-json.c
+++ b/tools/helpers/init-dom-json.c
@@ -7,7 +7,7 @@
 #include <xenctrl.h>
 #include <libxl.h>
 
-int gen_stub_json_config(uint32_t domid)
+int gen_stub_json_config(uint32_t domid, libxl_uuid *uuid)
 {
     int rc = 1;
     xentoollog_logger_stdiostream *logger;
@@ -40,6 +40,9 @@ int gen_stub_json_config(uint32_t domid)
     libxl_domain_build_info_init_type(&dom_config.b_info,
                                       dom_config.c_info.type);
 
+    if (uuid && !libxl_uuid_is_nil(uuid))
+        libxl_uuid_copy(ctx, &dom_config.c_info.uuid, uuid);
+
     json = libxl_domain_config_to_json(ctx, &dom_config);
     /* libxl-json format requires the string ends with '\0'. Code
      * snippet taken from libxl.
diff --git a/tools/helpers/init-dom-json.h b/tools/helpers/init-dom-json.h
index 58c85df157..5be88f6638 100644
--- a/tools/helpers/init-dom-json.h
+++ b/tools/helpers/init-dom-json.h
@@ -1,10 +1,11 @@
 #ifndef __INIT_DOM_JSON_H
 #define __INIT_DOM_JSON_H
 
+#include <libxl.h>
 /*
  * Generate a stub JSON config for a domain with the given domid.
  */
-int gen_stub_json_config(uint32_t domid);
+int gen_stub_json_config(uint32_t domid, libxl_uuid *uuid);
 
 #endif
 /*
diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 3236d14416..adb8408b63 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -382,7 +382,7 @@ int main(int argc, char** argv)
     if ( rv )
         return 1;
 
-    rv = gen_stub_json_config(domid);
+    rv = gen_stub_json_config(domid, NULL);
     if ( rv )
         return 3;
 
diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
index 9ab8468bdb..09bc0027f9 100644
--- a/tools/helpers/xen-init-dom0.c
+++ b/tools/helpers/xen-init-dom0.c
@@ -31,7 +31,7 @@ int main(int argc, char **argv)
         goto out;
     }
 
-    rc = gen_stub_json_config(0);
+    rc = gen_stub_json_config(0, NULL);
     if (rc)
         goto out;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Wei Liu]

Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
it for Dom0.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/examples/Makefile       |  1 +
 tools/examples/README         |  2 ++
 tools/examples/dom0-uuid      |  0
 tools/helpers/Makefile        |  3 +-
 tools/helpers/xen-init-dom0.c | 67 +++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 69 insertions(+), 4 deletions(-)
 create mode 100644 tools/examples/dom0-uuid

diff --git a/tools/examples/Makefile b/tools/examples/Makefile
index f86ed3a271..f8492462db 100644
--- a/tools/examples/Makefile
+++ b/tools/examples/Makefile
@@ -9,6 +9,7 @@ XEN_CONFIGS += xlexample.hvm
 XEN_CONFIGS += xlexample.pvlinux
 XEN_CONFIGS += xl.conf
 XEN_CONFIGS += cpupool
+XEN_CONFIGS += dom0-uuid
 
 XEN_CONFIGS += $(XEN_CONFIGS-y)
 
diff --git a/tools/examples/README b/tools/examples/README
index 80a4652b06..8f940a55c1 100644
--- a/tools/examples/README
+++ b/tools/examples/README
@@ -14,6 +14,8 @@ block-common.sh     - sourced by block, block-*
 block-enbd          - binds/unbinds network block devices
 block-nbd           - binds/unbinds network block devices
 cpupool             - example configuration script for 'xl cpupool-create'
+dom0-uuid           - stores the UUID in canonical form for Dom0, will be
+                      read by xen-init-dom0
 external-device-migrate - called by xend for migrating external devices
 locking.sh          - locking functions to prevent concurrent access to
                       critical sections inside script files
diff --git a/tools/examples/dom0-uuid b/tools/examples/dom0-uuid
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tools/helpers/Makefile b/tools/helpers/Makefile
index 4f3bbe6a7d..f759528322 100644
--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -14,6 +14,7 @@ XEN_INIT_DOM0_OBJS = xen-init-dom0.o init-dom-json.o
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxenstore)
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxenlight)
+$(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxenctrl)
 
 INIT_XENSTORE_DOMAIN_OBJS = init-xenstore-domain.o init-dom-json.o
 $(INIT_XENSTORE_DOMAIN_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
@@ -26,7 +27,7 @@ $(INIT_XENSTORE_DOMAIN_OBJS): CFLAGS += $(CFLAGS_libxenlight)
 all: $(PROGS)
 
 xen-init-dom0: $(XEN_INIT_DOM0_OBJS)
-	$(CC) $(LDFLAGS) -o $@ $(XEN_INIT_DOM0_OBJS) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
+	$(CC) $(LDFLAGS) -o $@ $(XEN_INIT_DOM0_OBJS) $(LDLIBS_libxenctrl) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
 
 $(INIT_XENSTORE_DOMAIN_OBJS): _paths.h
 
diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
index 09bc0027f9..2b06610c7c 100644
--- a/tools/helpers/xen-init-dom0.c
+++ b/tools/helpers/xen-init-dom0.c
@@ -2,24 +2,75 @@
 #include <stdint.h>
 #include <string.h>
 #include <stdio.h>
+#include <fcntl.h>
 
+#include <xenctrl.h>
 #include <xenstore.h>
+#include <libxl.h>
 
 #include "init-dom-json.h"
+#include "_paths.h"
 
 #define DOMNAME_PATH   "/local/domain/0/name"
 #define DOMID_PATH     "/local/domain/0/domid"
 
+#define DOM0_UUID_PATH XEN_CONFIG_DIR "/dom0-uuid"
+
+static void get_dom0_uuid(libxl_uuid *uuid)
+{
+    int fd = -1;
+    ssize_t r;
+    char uuid_buf[LIBXL_UUID_FMTLEN+1];
+
+    libxl_uuid_clear(uuid);
+
+    fd = open(DOM0_UUID_PATH, O_RDONLY);
+    if (fd < 0) {
+        fprintf(stderr, "failed to open %s\n", DOM0_UUID_PATH);
+        goto out;
+    }
+
+    r = read(fd, uuid_buf, LIBXL_UUID_FMTLEN);
+    if (r == -1) {
+        fprintf(stderr, "failed to read %s, errno %d\n", DOM0_UUID_PATH, errno);
+        goto out;
+    }
+
+    if (r != LIBXL_UUID_FMTLEN) {
+        fprintf(stderr, "file too short\n");
+        goto out;
+    }
+
+    uuid_buf[LIBXL_UUID_FMTLEN] = 0;
+
+    if (libxl_uuid_from_string(uuid, uuid_buf)) {
+        fprintf(stderr, "failed to parse UUID\n");
+        libxl_uuid_clear(uuid);
+    }
+
+out:
+    if (fd >= 0) close(fd);
+}
+
 int main(int argc, char **argv)
 {
     int rc;
-    struct xs_handle *xsh;
+    struct xs_handle *xsh = NULL;
+    xc_interface *xch = NULL;
     char *domname_string = NULL, *domid_string = NULL;
+    libxl_uuid uuid;
 
     xsh = xs_open(0);
     if (!xsh) {
         fprintf(stderr, "cannot open xenstore connection\n");
-        exit(1);
+        rc = 1;
+        goto out;
+    }
+
+    xch = xc_interface_open(NULL, NULL, 0);
+    if (!xch) {
+        fprintf(stderr, "xc_interface_open() failed\n");
+        rc = 1;
     }
 
     /* Sanity check: this program can only be run once. */
@@ -31,7 +82,16 @@ int main(int argc, char **argv)
         goto out;
     }
 
-    rc = gen_stub_json_config(0, NULL);
+    get_dom0_uuid(&uuid);
+
+    if (!libxl_uuid_is_nil(&uuid) &&
+        xc_domain_sethandle(xch, 0, libxl_uuid_bytearray(&uuid))) {
+        fprintf(stderr, "failed to set Dom0 UUID\n");
+        rc = 1;
+        goto out;
+    }
+
+    rc = gen_stub_json_config(0, &uuid);
     if (rc)
         goto out;
 
@@ -55,6 +115,7 @@ out:
     free(domid_string);
     free(domname_string);
     xs_close(xsh);
+    xc_interface_close(xch);
     return rc;
 }
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Jason Andryuk]

On Wed, Nov 14, 2018 at 1:20 PM Wei Liu <wei.liu2@citrix.com> wrote:
>
> Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
> it for Dom0.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

<snip>
> diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
> index 09bc0027f9..2b06610c7c 100644
> --- a/tools/helpers/xen-init-dom0.c
> +++ b/tools/helpers/xen-init-dom0.c
> @@ -2,24 +2,75 @@
>  #include <stdint.h>
>  #include <string.h>
>  #include <stdio.h>
> +#include <fcntl.h>
>
> +#include <xenctrl.h>
>  #include <xenstore.h>
> +#include <libxl.h>
>
>  #include "init-dom-json.h"
> +#include "_paths.h"
>
>  #define DOMNAME_PATH   "/local/domain/0/name"
>  #define DOMID_PATH     "/local/domain/0/domid"
>
> +#define DOM0_UUID_PATH XEN_CONFIG_DIR "/dom0-uuid"
> +
> +static void get_dom0_uuid(libxl_uuid *uuid)
> +{
> +    int fd = -1;
> +    ssize_t r;
> +    char uuid_buf[LIBXL_UUID_FMTLEN+1];
> +
> +    libxl_uuid_clear(uuid);
> +
> +    fd = open(DOM0_UUID_PATH, O_RDONLY);
> +    if (fd < 0) {
> +        fprintf(stderr, "failed to open %s\n", DOM0_UUID_PATH);
> +        goto out;
> +    }
> +
> +    r = read(fd, uuid_buf, LIBXL_UUID_FMTLEN);
> +    if (r == -1) {
> +        fprintf(stderr, "failed to read %s, errno %d\n", DOM0_UUID_PATH, errno);
> +        goto out;
> +    }
> +
> +    if (r != LIBXL_UUID_FMTLEN) {
> +        fprintf(stderr, "file too short\n");
> +        goto out;
> +    }
> +
> +    uuid_buf[LIBXL_UUID_FMTLEN] = 0;
> +
> +    if (libxl_uuid_from_string(uuid, uuid_buf)) {
> +        fprintf(stderr, "failed to parse UUID\n");
> +        libxl_uuid_clear(uuid);
> +    }
> +
> +out:
> +    if (fd >= 0) close(fd);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      int rc;
> -    struct xs_handle *xsh;
> +    struct xs_handle *xsh = NULL;
> +    xc_interface *xch = NULL;
>      char *domname_string = NULL, *domid_string = NULL;
> +    libxl_uuid uuid;
>
>      xsh = xs_open(0);
>      if (!xsh) {
>          fprintf(stderr, "cannot open xenstore connection\n");
> -        exit(1);
> +        rc = 1;
> +        goto out;
> +    }
> +
> +    xch = xc_interface_open(NULL, NULL, 0);
> +    if (!xch) {
> +        fprintf(stderr, "xc_interface_open() failed\n");
> +        rc = 1;

Do you want a goto out here?

Regards,
Jason

>      }
>
>      /* Sanity check: this program can only be run once. */
> @@ -31,7 +82,16 @@ int main(int argc, char **argv)
>          goto out;
>      }
>
> -    rc = gen_stub_json_config(0, NULL);
> +    get_dom0_uuid(&uuid);
> +
> +    if (!libxl_uuid_is_nil(&uuid) &&
> +        xc_domain_sethandle(xch, 0, libxl_uuid_bytearray(&uuid))) {
> +        fprintf(stderr, "failed to set Dom0 UUID\n");
> +        rc = 1;
> +        goto out;
> +    }
> +
> +    rc = gen_stub_json_config(0, &uuid);
>      if (rc)
>          goto out;
>
> @@ -55,6 +115,7 @@ out:
>      free(domid_string);
>      free(domname_string);
>      xs_close(xsh);
> +    xc_interface_close(xch);
>      return rc;
>  }
>
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Wei Liu]

On Wed, Nov 14, 2018 at 03:16:49PM -0500, Jason Andryuk wrote:
[...]
> >  int main(int argc, char **argv)
> >  {
> >      int rc;
> > -    struct xs_handle *xsh;
> > +    struct xs_handle *xsh = NULL;
> > +    xc_interface *xch = NULL;
> >      char *domname_string = NULL, *domid_string = NULL;
> > +    libxl_uuid uuid;
> >
> >      xsh = xs_open(0);
> >      if (!xsh) {
> >          fprintf(stderr, "cannot open xenstore connection\n");
> > -        exit(1);
> > +        rc = 1;
> > +        goto out;
> > +    }
> > +
> > +    xch = xc_interface_open(NULL, NULL, 0);
> > +    if (!xch) {
> > +        fprintf(stderr, "xc_interface_open() failed\n");
> > +        rc = 1;
> 
> Do you want a goto out here?

Yes I do. Good catch!

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Edwin Török]

On 14/11/2018 18:17, Wei Liu wrote:
> Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
> it for Dom0.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

[snip]
In general this looks good, however I am not familiar with libxl
conventions, so just some generic comments below.

> +static void get_dom0_uuid(libxl_uuid *uuid)
> +{
> +    int fd = -1;
> +    ssize_t r;
> +    char uuid_buf[LIBXL_UUID_FMTLEN+1];
> +
> +    libxl_uuid_clear(uuid);
> +
> +    fd = open(DOM0_UUID_PATH, O_RDONLY);
> +    if (fd < 0) {
> +        fprintf(stderr, "failed to open %s\n", DOM0_UUID_PATH);
> +        goto out;
> +    }
> +
> +    r = read(fd, uuid_buf, LIBXL_UUID_FMTLEN);

Could this be a short read? I'm not familiar with libxl conventions, but
would there be a utility function that repeats the read if it is short,
or would fread be better?

> +    if (r == -1) {
> +        fprintf(stderr, "failed to read %s, errno %d\n", DOM0_UUID_PATH, errno);
> +        goto out;
> +    }
> +
> +    if (r != LIBXL_UUID_FMTLEN) {
> +        fprintf(stderr, "file too short\n");

Would be nice to print which file is too short.


> +        goto out;
> +    }
> +
> +    uuid_buf[LIBXL_UUID_FMTLEN] = 0;
> +
> +    if (libxl_uuid_from_string(uuid, uuid_buf)) {
> +        fprintf(stderr, "failed to parse UUID\n");

As above, would be nice to print which file this error is from.

> +        libxl_uuid_clear(uuid);
> +    }
> +
> +out:
> +    if (fd >= 0) close(fd);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      int rc;
> -    struct xs_handle *xsh;
> +    struct xs_handle *xsh = NULL;
> +    xc_interface *xch = NULL;
>      char *domname_string = NULL, *domid_string = NULL;
> +    libxl_uuid uuid;
>  
>      xsh = xs_open(0);
>      if (!xsh) {
>          fprintf(stderr, "cannot open xenstore connection\n");
> -        exit(1);
> +        rc = 1;
> +        goto out;
> +    }
> +
> +    xch = xc_interface_open(NULL, NULL, 0);
> +    if (!xch) {
> +        fprintf(stderr, "xc_interface_open() failed\n");
> +        rc = 1;
>      }
>  
>      /* Sanity check: this program can only be run once. */
> @@ -31,7 +82,16 @@ int main(int argc, char **argv)
>          goto out;
>      }
>  
> -    rc = gen_stub_json_config(0, NULL);
> +    get_dom0_uuid(&uuid);
> +
> +    if (!libxl_uuid_is_nil(&uuid) &&
> +        xc_domain_sethandle(xch, 0, libxl_uuid_bytearray(&uuid))) {
> +        fprintf(stderr, "failed to set Dom0 UUID\n");

Can xc_domain_sethandle tell us why it failed?

> +        rc = 1;
> +        goto out;
> +    }
> +
> +    rc = gen_stub_json_config(0, &uuid);
>      if (rc)
>          goto out;
>  
> @@ -55,6 +115,7 @@ out:
>      free(domid_string);
>      free(domname_string);
>      xs_close(xsh);
> +    xc_interface_close(xch);

I assume this function doesn't mind getting called with NULL, right?

Best regards,
--Edwin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Wei Liu]

On Thu, Nov 15, 2018 at 10:45:52AM +0000, Edwin Török wrote:
> On 14/11/2018 18:17, Wei Liu wrote:
> > Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
> > it for Dom0.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> [snip]
> In general this looks good, however I am not familiar with libxl
> conventions, so just some generic comments below.
> 
> > +static void get_dom0_uuid(libxl_uuid *uuid)
> > +{
> > +    int fd = -1;
> > +    ssize_t r;
> > +    char uuid_buf[LIBXL_UUID_FMTLEN+1];
> > +
> > +    libxl_uuid_clear(uuid);
> > +
> > +    fd = open(DOM0_UUID_PATH, O_RDONLY);
> > +    if (fd < 0) {
> > +        fprintf(stderr, "failed to open %s\n", DOM0_UUID_PATH);
> > +        goto out;
> > +    }
> > +
> > +    r = read(fd, uuid_buf, LIBXL_UUID_FMTLEN);
> 
> Could this be a short read? I'm not familiar with libxl conventions, but
> would there be a utility function that repeats the read if it is short,
> or would fread be better?

I can use libxl_read_exactly instead. That saves me from writing some
code to handle every corner case.

> 
> > +    if (r == -1) {
> > +        fprintf(stderr, "failed to read %s, errno %d\n", DOM0_UUID_PATH, errno);
> > +        goto out;
> > +    }
> > +
> > +    if (r != LIBXL_UUID_FMTLEN) {
> > +        fprintf(stderr, "file too short\n");
> 
> Would be nice to print which file is too short.
> 

OK.

> 
> > +        goto out;
> > +    }
> > +
> > +    uuid_buf[LIBXL_UUID_FMTLEN] = 0;
> > +
> > +    if (libxl_uuid_from_string(uuid, uuid_buf)) {
> > +        fprintf(stderr, "failed to parse UUID\n");
> 
> As above, would be nice to print which file this error is from.
> 

OK.

> > +        libxl_uuid_clear(uuid);
> > +    }
> > +
> > +out:
> > +    if (fd >= 0) close(fd);
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >      int rc;
> > -    struct xs_handle *xsh;
> > +    struct xs_handle *xsh = NULL;
> > +    xc_interface *xch = NULL;
> >      char *domname_string = NULL, *domid_string = NULL;
> > +    libxl_uuid uuid;
> >  
> >      xsh = xs_open(0);
> >      if (!xsh) {
> >          fprintf(stderr, "cannot open xenstore connection\n");
> > -        exit(1);
> > +        rc = 1;
> > +        goto out;
> > +    }
> > +
> > +    xch = xc_interface_open(NULL, NULL, 0);
> > +    if (!xch) {
> > +        fprintf(stderr, "xc_interface_open() failed\n");
> > +        rc = 1;
> >      }
> >  
> >      /* Sanity check: this program can only be run once. */
> > @@ -31,7 +82,16 @@ int main(int argc, char **argv)
> >          goto out;
> >      }
> >  
> > -    rc = gen_stub_json_config(0, NULL);
> > +    get_dom0_uuid(&uuid);
> > +
> > +    if (!libxl_uuid_is_nil(&uuid) &&
> > +        xc_domain_sethandle(xch, 0, libxl_uuid_bytearray(&uuid))) {
> > +        fprintf(stderr, "failed to set Dom0 UUID\n");
> 
> Can xc_domain_sethandle tell us why it failed?
> 

We can print errno here.

> > +        rc = 1;
> > +        goto out;
> > +    }
> > +
> > +    rc = gen_stub_json_config(0, &uuid);
> >      if (rc)
> >          goto out;
> >  
> > @@ -55,6 +115,7 @@ out:
> >      free(domid_string);
> >      free(domname_string);
> >      xs_close(xsh);
> > +    xc_interface_close(xch);
> 
> I assume this function doesn't mind getting called with NULL, right?

No, it doesn't mind.

Wei.

> 
> Best regards,
> --Edwin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Wei Liu]

On Thu, Nov 15, 2018 at 11:20:37AM +0000, Wei Liu wrote:
> On Thu, Nov 15, 2018 at 10:45:52AM +0000, Edwin Török wrote:
> > On 14/11/2018 18:17, Wei Liu wrote:
> > > Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
> > > it for Dom0.
> > > 
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > [snip]
> > In general this looks good, however I am not familiar with libxl
> > conventions, so just some generic comments below.
> > 
> > > +static void get_dom0_uuid(libxl_uuid *uuid)
> > > +{
> > > +    int fd = -1;
> > > +    ssize_t r;
> > > +    char uuid_buf[LIBXL_UUID_FMTLEN+1];
> > > +
> > > +    libxl_uuid_clear(uuid);
> > > +
> > > +    fd = open(DOM0_UUID_PATH, O_RDONLY);
> > > +    if (fd < 0) {
> > > +        fprintf(stderr, "failed to open %s\n", DOM0_UUID_PATH);
> > > +        goto out;
> > > +    }
> > > +
> > > +    r = read(fd, uuid_buf, LIBXL_UUID_FMTLEN);
> > 
> > Could this be a short read? I'm not familiar with libxl conventions, but
> > would there be a utility function that repeats the read if it is short,
> > or would fread be better?
> 
> I can use libxl_read_exactly instead. That saves me from writing some
> code to handle every corner case.
> 

On second thought, this requires code to allocating and destroying libxl
ctx. I will write a loop here to handle short-read instead.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Andrew Cooper]

On 15/11/2018 13:35, Wei Liu wrote:
> On Thu, Nov 15, 2018 at 11:20:37AM +0000, Wei Liu wrote:
>> On Thu, Nov 15, 2018 at 10:45:52AM +0000, Edwin Török wrote:
>>> On 14/11/2018 18:17, Wei Liu wrote:
>>>> Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
>>>> it for Dom0.
>>>>
>>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> [snip]
>>> In general this looks good, however I am not familiar with libxl
>>> conventions, so just some generic comments below.
>>>
>>>> +static void get_dom0_uuid(libxl_uuid *uuid)
>>>> +{
>>>> +    int fd = -1;
>>>> +    ssize_t r;
>>>> +    char uuid_buf[LIBXL_UUID_FMTLEN+1];
>>>> +
>>>> +    libxl_uuid_clear(uuid);
>>>> +
>>>> +    fd = open(DOM0_UUID_PATH, O_RDONLY);
>>>> +    if (fd < 0) {
>>>> +        fprintf(stderr, "failed to open %s\n", DOM0_UUID_PATH);
>>>> +        goto out;
>>>> +    }
>>>> +
>>>> +    r = read(fd, uuid_buf, LIBXL_UUID_FMTLEN);
>>> Could this be a short read? I'm not familiar with libxl conventions, but
>>> would there be a utility function that repeats the read if it is short,
>>> or would fread be better?
>> I can use libxl_read_exactly instead. That saves me from writing some
>> code to handle every corner case.
>>
> On second thought, this requires code to allocating and destroying libxl
> ctx. I will write a loop here to handle short-read instead.

fopen()/fread() will get you the correct behaviour for far less code.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/3] tools: update examples/README

[Andrew Cooper]

On 14/11/2018 18:17, Wei Liu wrote:
> This file gets installed to the host system.
>
> This patch cleans it up: 1. remove things that don't exist anymore; 2.
> change xm to xl; 3. fix xen-devel list address; 4. add things that are
> missing.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Is this file actually worth keeping?  Considering how obsolete its
information is, I severely doubt anyone uses it.

If we do want to keep it, can you strip trailing whitespace in this
patch as well please?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 3/3] xen-init-dom0: set Dom0 UUID if requested

[Wei Liu]

Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
it for Dom0.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2:
1. add missing "goto out"
2. print file names more
3. also print errno in xc_interface_open error message
4. take care of short-read
---
 tools/examples/Makefile       |  1 +
 tools/examples/README         |  2 ++
 tools/examples/dom0-uuid      |  0
 tools/helpers/Makefile        |  3 +-
 tools/helpers/xen-init-dom0.c | 65 +++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 67 insertions(+), 4 deletions(-)
 create mode 100644 tools/examples/dom0-uuid

diff --git a/tools/examples/Makefile b/tools/examples/Makefile
index f86ed3a271..f8492462db 100644
--- a/tools/examples/Makefile
+++ b/tools/examples/Makefile
@@ -9,6 +9,7 @@ XEN_CONFIGS += xlexample.hvm
 XEN_CONFIGS += xlexample.pvlinux
 XEN_CONFIGS += xl.conf
 XEN_CONFIGS += cpupool
+XEN_CONFIGS += dom0-uuid
 
 XEN_CONFIGS += $(XEN_CONFIGS-y)
 
diff --git a/tools/examples/README b/tools/examples/README
index 80a4652b06..8f940a55c1 100644
--- a/tools/examples/README
+++ b/tools/examples/README
@@ -14,6 +14,8 @@ block-common.sh     - sourced by block, block-*
 block-enbd          - binds/unbinds network block devices
 block-nbd           - binds/unbinds network block devices
 cpupool             - example configuration script for 'xl cpupool-create'
+dom0-uuid           - stores the UUID in canonical form for Dom0, will be
+                      read by xen-init-dom0
 external-device-migrate - called by xend for migrating external devices
 locking.sh          - locking functions to prevent concurrent access to
                       critical sections inside script files
diff --git a/tools/examples/dom0-uuid b/tools/examples/dom0-uuid
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tools/helpers/Makefile b/tools/helpers/Makefile
index 4f3bbe6a7d..f759528322 100644
--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -14,6 +14,7 @@ XEN_INIT_DOM0_OBJS = xen-init-dom0.o init-dom-json.o
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxenstore)
 $(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxenlight)
+$(XEN_INIT_DOM0_OBJS): CFLAGS += $(CFLAGS_libxenctrl)
 
 INIT_XENSTORE_DOMAIN_OBJS = init-xenstore-domain.o init-dom-json.o
 $(INIT_XENSTORE_DOMAIN_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
@@ -26,7 +27,7 @@ $(INIT_XENSTORE_DOMAIN_OBJS): CFLAGS += $(CFLAGS_libxenlight)
 all: $(PROGS)
 
 xen-init-dom0: $(XEN_INIT_DOM0_OBJS)
-	$(CC) $(LDFLAGS) -o $@ $(XEN_INIT_DOM0_OBJS) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
+	$(CC) $(LDFLAGS) -o $@ $(XEN_INIT_DOM0_OBJS) $(LDLIBS_libxenctrl) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
 
 $(INIT_XENSTORE_DOMAIN_OBJS): _paths.h
 
diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
index 09bc0027f9..e826da57b4 100644
--- a/tools/helpers/xen-init-dom0.c
+++ b/tools/helpers/xen-init-dom0.c
@@ -3,23 +3,72 @@
 #include <string.h>
 #include <stdio.h>
 
+#include <xenctrl.h>
 #include <xenstore.h>
+#include <libxl.h>
 
 #include "init-dom-json.h"
+#include "_paths.h"
 
 #define DOMNAME_PATH   "/local/domain/0/name"
 #define DOMID_PATH     "/local/domain/0/domid"
 
+#define DOM0_UUID_PATH XEN_CONFIG_DIR "/dom0-uuid"
+
+static void get_dom0_uuid(libxl_uuid *uuid)
+{
+    FILE *f = NULL;
+    size_t r;
+    char uuid_buf[LIBXL_UUID_FMTLEN+1];
+    bool ok = false;
+
+    f = fopen(DOM0_UUID_PATH, "r");
+    if (!f) {
+        fprintf(stderr, "failed to open %s, errno %d\n",
+                DOM0_UUID_PATH, errno);
+        goto out;
+    }
+
+    r = fread(uuid_buf, 1, LIBXL_UUID_FMTLEN, f);
+    if (r != LIBXL_UUID_FMTLEN) {
+        fprintf(stderr, "failed to read %s, read %zu, errno %d\n",
+                DOM0_UUID_PATH, r, ferror(f));
+        goto out;
+    }
+
+    uuid_buf[LIBXL_UUID_FMTLEN] = 0;
+
+    if (libxl_uuid_from_string(uuid, uuid_buf)) {
+        fprintf(stderr, "failed to parse UUID in %s\n", DOM0_UUID_PATH);
+        goto out;
+    }
+
+    ok = true;
+out:
+    if (f) fclose(f);
+    if (!ok) libxl_uuid_clear(uuid);
+}
+
 int main(int argc, char **argv)
 {
     int rc;
-    struct xs_handle *xsh;
+    struct xs_handle *xsh = NULL;
+    xc_interface *xch = NULL;
     char *domname_string = NULL, *domid_string = NULL;
+    libxl_uuid uuid;
 
     xsh = xs_open(0);
     if (!xsh) {
         fprintf(stderr, "cannot open xenstore connection\n");
-        exit(1);
+        rc = 1;
+        goto out;
+    }
+
+    xch = xc_interface_open(NULL, NULL, 0);
+    if (!xch) {
+        fprintf(stderr, "xc_interface_open() failed\n");
+        rc = 1;
+        goto out;
     }
 
     /* Sanity check: this program can only be run once. */
@@ -31,7 +80,16 @@ int main(int argc, char **argv)
         goto out;
     }
 
-    rc = gen_stub_json_config(0, NULL);
+    get_dom0_uuid(&uuid);
+
+    if (!libxl_uuid_is_nil(&uuid) &&
+        xc_domain_sethandle(xch, 0, libxl_uuid_bytearray(&uuid))) {
+        fprintf(stderr, "failed to set Dom0 UUID, errno %d\n", errno);
+        rc = 1;
+        goto out;
+    }
+
+    rc = gen_stub_json_config(0, &uuid);
     if (rc)
         goto out;
 
@@ -55,6 +113,7 @@ out:
     free(domid_string);
     free(domname_string);
     xs_close(xsh);
+    xc_interface_close(xch);
     return rc;
 }
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/3] tools: update examples/README

[Wei Liu]

On Thu, Nov 15, 2018 at 02:28:15PM +0000, Andrew Cooper wrote:
> On 14/11/2018 18:17, Wei Liu wrote:
> > This file gets installed to the host system.
> >
> > This patch cleans it up: 1. remove things that don't exist anymore; 2.
> > change xm to xl; 3. fix xen-devel list address; 4. add things that are
> > missing.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Is this file actually worth keeping?  Considering how obsolete its
> information is, I severely doubt anyone uses it.

Somebody will probably want to know what is what.

> 
> If we do want to keep it, can you strip trailing whitespace in this
> patch as well please?

Sure.

Wei.

> 
> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/3] tools/helpers: make gen_stub_json_config accept an UUID argument

[Andrew Cooper]

On 14/11/2018 18:17, Wei Liu wrote:
> If that's set, the stub is going to contain that UUID.
>
> No functional change.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/helpers/init-dom-json.c        | 5 ++++-
>  tools/helpers/init-dom-json.h        | 3 ++-
>  tools/helpers/init-xenstore-domain.c | 2 +-
>  tools/helpers/xen-init-dom0.c        | 2 +-
>  4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/tools/helpers/init-dom-json.c b/tools/helpers/init-dom-json.c
> index 704e7cb4f0..9514b3ceb6 100644
> --- a/tools/helpers/init-dom-json.c
> +++ b/tools/helpers/init-dom-json.c
> @@ -7,7 +7,7 @@
>  #include <xenctrl.h>
>  #include <libxl.h>
>  
> -int gen_stub_json_config(uint32_t domid)
> +int gen_stub_json_config(uint32_t domid, libxl_uuid *uuid)
>  {
>      int rc = 1;
>      xentoollog_logger_stdiostream *logger;
> @@ -40,6 +40,9 @@ int gen_stub_json_config(uint32_t domid)
>      libxl_domain_build_info_init_type(&dom_config.b_info,
>                                        dom_config.c_info.type);
>  
> +    if (uuid && !libxl_uuid_is_nil(uuid))
> +        libxl_uuid_copy(ctx, &dom_config.c_info.uuid, uuid);

Is the nil check necessary, given that nil is the default value for
dom_config.c_info.uuid ?

Either way, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/3] xen-init-dom0: set Dom0 UUID if requested

[Edwin Török]

On 15/11/2018 14:30, Wei Liu wrote:
> Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
> it for Dom0.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2:
> 1. add missing "goto out"
> 2. print file names more
> 3. also print errno in xc_interface_open error message
> 4. take care of short-read
> ---
>  tools/examples/Makefile       |  1 +
>  tools/examples/README         |  2 ++
>  tools/examples/dom0-uuid      |  0
>  tools/helpers/Makefile        |  3 +-
>  tools/helpers/xen-init-dom0.c | 65 +++++++++++++++++++++++++++++++++++++++++--
>  5 files changed, 67 insertions(+), 4 deletions(-)
>  create mode 100644 tools/examples/dom0-uuid


I can't spot anything wrong here.
Acked-by: Edwin Török <edvin.torok@citrix.com>

Best regards,
--Edwin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/3] tools/helpers: make gen_stub_json_config accept an UUID argument

[Ian Jackson]

Wei Liu writes ("[PATCH 2/3] tools/helpers: make gen_stub_json_config accept an UUID argument"):
> If that's set, the stub is going to contain that UUID.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/3] tools: update examples/README

[Ian Jackson]

Wei Liu writes ("[PATCH 1/3] tools: update examples/README"):
> This file gets installed to the host system.
> 
> This patch cleans it up: 1. remove things that don't exist anymore; 2.
> change xm to xl; 3. fix xen-devel list address; 4. add things that are
> missing.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested

[Ian Jackson]

Wei Liu writes ("[PATCH 3/3] xen-init-dom0: set Dom0 UUID if requested"):
> Read from XEN_CONFIG_DIR/dom0-uuid. If it contains a valid UUID, set
> it for Dom0.

I approve of the basic principle of this change.  Thanks.

However, I am not particularly keen on the details of the config
representation.

> +dom0-uuid           - stores the UUID in canonical form for Dom0, will be
> +                      read by xen-init-dom0

The parsing approach taken means this file
  - may not contain leading comments
  - may contain ignore trailing stuff
  - is not read completely

Wouldn't it be better to put this in the global xl configuration ?

Ie, call xlu_cfg_init, xl_cfg_readdata, and xlu_cfg_get_string ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel