On Fri, 2020-03-13 at 17:59 +0100, Michal Privoznik wrote:
[...]
> - if (privileged) {
> + if (root) {
> + g_autofree char * hash = NULL;
> +
> + /* When two embed drivers start two domains with the same @name and @id
> + * we would generate a non-unique name. Include parts of hashed @root
> + * which guarantees uniqueness. The first 8 characters of SHA256 ought
> + * to be enough for anybody. */
> + if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, root, &hash) < 0)
> + return NULL;
> +
> + virBufferAsprintf(&buf, "%s-embed-%.8s-", drivername, hash);
When libvirt is non-privileged we use $username-$drivername, so there
would be a precedent for something like $hash-$drivername-embed; that
said, having $drivername first makes more sense to me, so if anything
I suggest we change the existing one to $drivername-$username.
> + } else if (privileged) {
> virBufferAsprintf(&buf, "%s-", drivername);
> } else {
> + g_autofree char *username = NULL;
> if (!(username = virGetUserName(geteuid()))) {
> virBufferFreeAndReset(&buf);
> return NULL;
> }
> virBufferAsprintf(&buf, "%s-%s-", username, drivername);
> - VIR_FREE(username);
This hunk is unrelated, so please drop it. It can be a standalone
trivial patch instead.
With that done,
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
Thank you so much for taking care of this!
--
Andrea Bolognani / Red Hat / Virtualization
