Re: [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Jason Andryuk writes ("[PATCH v5 06/21] libxl: write qemu arguments into 
separate xenstore keys"):
> +static int libxl__write_stub_linux_dmargs(libxl__gc *gc,
> +                                    int dm_domid, int guest_domid,
> +                                    char **args)
> +{
...
> +    vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/vm", 
> guest_domid));
> +    path = GCSPRINTF("%s/image/dmargs", vm_path);
> +
> +retry_transaction:
> +    t = xs_transaction_start(ctx->xsh);
> +    xs_write(ctx->xsh, t, path, "", 0);
> +    xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));

This wants to be libxl__xs_mknod I think ?

> +    i = 1;
> +    for (i=1; args[i] != NULL; i++)
> +        xs_write(ctx->xsh, t, GCSPRINTF("%s/%03d", path, i), args[i], 
> strlen(args[i]));

Can you do this with libxl__xs_transaction_* please, and a loop rather
than a goto ?

Why not use libxl__xs_write_checked ?

> +    xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), 
> roperm, ARRAY_SIZE(roperm));

This line seems out of place.  At least, it is not mentioned in the
commit message.  If it's needed, can you please split it out - and, of
course, then, provide an explanation :-).

> +    if (!xs_transaction_end(ctx->xsh, t, 0))
> +        if (errno == EAGAIN)
> +            goto retry_transaction;
> +    return 0;

Thanks,
Ian.