Re: [Xen-devel] [PATCH v3] tools: set Dom0 UUID if requested

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Nov 20, 2018 at 06:09:41PM +0000, Wei Liu wrote:
> diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
> index 09bc0027f9..a2f31fe9bf 100644
> --- a/tools/helpers/xen-init-dom0.c
> +++ b/tools/helpers/xen-init-dom0.c
[..]
>      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");

It might be usefull to print errno as well. I mean what perror would
print.

> +        rc = 1;
> +        goto out;
>      }
>  
>      /* Sanity check: this program can only be run once. */
> @@ -31,7 +50,23 @@ int main(int argc, char **argv)
>          goto out;
>      }
>  
> -    rc = gen_stub_json_config(0, NULL);
> +    libxl_uuid_clear(&uuid);
> +
> +    /* If UUID is supplied, parse it. */
> +    if (argc == 2 && libxl_uuid_from_string(&uuid, argv[1])) {
> +        fprintf(stderr, "failed to parse UUID %s\n", argv[1]);
> +        rc = 1;
> +        goto out;
> +    }
> +
> +    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);

Same here, I think it would be more usefull to use perror or strerror
instead of printing the errno value.

> diff --git a/tools/hotplug/Linux/init.d/xencommons.in 
> b/tools/hotplug/Linux/init.d/xencommons.in
> index ec42b05587..a33058ed44 100644
> --- a/tools/hotplug/Linux/init.d/xencommons.in
> +++ b/tools/hotplug/Linux/init.d/xencommons.in
> @@ -63,7 +63,7 @@ do_start () {
>       @XEN_SCRIPT_DIR@/launch-xenstore || exit 1
>  
>       echo Setting domain 0 name, domid and JSON config...
> -     ${LIBEXEC_BIN}/xen-init-dom0
> +     ${LIBEXEC_BIN}/xen-init-dom0 ${XEN_DOM0_UUID}
>  
>       echo Starting xenconsoled...
>       test -z "$XENCONSOLED_TRACE" || XENCONSOLED_ARGS=" 
> --log=$XENCONSOLED_TRACE"
> diff --git a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in 
> b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in
> index 3befadcea3..beed3126c6 100644
> --- a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in
> +++ b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in
> @@ -7,8 +7,9 @@ ConditionPathExists=/proc/xen/capabilities
>  [Service]
>  Type=oneshot
>  RemainAfterExit=true
> +EnvironmentFile=@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
>  ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
> -ExecStart=@LIBEXEC_BIN@/xen-init-dom0
> +ExecStart=@LIBEXEC_BIN@/xen-init-dom0 $XEN_DOM0_UUID
>  
>  [Install]
>  WantedBy=multi-user.target

Don't you need to modify FreeBSD/rc.d/xencommons.in and
NetBSD/rc.d/xencommons.in as well?

-- 
Anthony PERARD

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