Libguestfs - Sep 2018 - Re: [PATCH 2/2] Introduce a --key option in tools that accept keys

This would have been a bit easier to review if the keystore
changes had been broken out from the tools changes.

On Wed, Sep 19, 2018 at 12:37:01PM +0200, Pino Toscano
wrote:
> @@ -599,13 +621,21 @@ let is_btrfs_subvolume g fs >      if g#last_errno
() = Guestfs.Errno.errno_EINVAL then false
>      else raise exn
>  
> -let inspect_decrypt g > +let inspect_decrypt g ks > +  (* Turn the
keys in the key_store into a simpler struct, so it is possible
> +   * to read it using the C API.
> +   *)
> +  let keys_as_list = Hashtbl.fold (
> +    fun k v acc ->
> +      (k, v) :: acc
> +  ) ks.keys [] in
>    (* Note we pass original 'g' even though it is not used by the
>     * callee.  This is so that 'g' is kept as a root on the stack,
and
>     * so cannot be garbage collected while we are in the c_inspect_decrypt
>     * function.
>     *)
>    c_inspect_decrypt g#ocaml_handle (Guestfs.c_pointer g#ocaml_handle)
> +    keys_as_list
An array would be even easier, but I guess you've written
the list code now :-)

- - -

I think Eric's / qemu's shared key stuff sounds very complex, and I
wonder who uses it.  But in any case what you've proposed is
extensible enough that we would be able to add other methods to pass
the key in future.

It all looks good to me, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Thursday, 20 September 2018 12:15:12 CEST Richard W.M. Jones
wrote:
> This would have been a bit easier to review if the keystore
> changes had been broken out from the tools changes.
I actually thought (even too much) about various ways of splitting it;
since I wanted to not become a new Buridan's ass [1], then I lumped it
all in a single patch.

Splitting is not an issue, so if you suggest a preferred layout I can
work on it.

[1] https://en.wikipedia.org/wiki/Buridan%27s_ass
> I think Eric's / qemu's shared key stuff sounds very complex, and I
> wonder who uses it.  But in any case what you've proposed is
> extensible enough that we would be able to add other methods to pass
> the key in future.
As I suggested in my reply to Eric's email, we can always create a
"fd"
selector, if needed.

-- 
Pino Toscano

On Thu, Sep 20, 2018 at 05:26:26PM +0200, Pino Toscano
wrote:
> On Thursday, 20 September 2018 12:15:12 CEST Richard W.M. Jones wrote:
> > This would have been a bit easier to review if the keystore
> > changes had been broken out from the tools changes.
> 
> I actually thought (even too much) about various ways of splitting it;
> since I wanted to not become a new Buridan's ass [1], then I lumped it
> all in a single patch.
> 
> Splitting is not an issue, so if you suggest a preferred layout I can
> work on it.
Don't worry about it - it would have been easier, but I was able
to review it anyway.

I'd actually prefer if you pushed it as soon as possible so that
I can do a new upstream release today.

Also if you could look at the few patches I posted yesterday.

Rich.
> [1] https://en.wikipedia.org/wiki/Buridan%27s_ass
> 
> > I think Eric's / qemu's shared key stuff sounds very complex,
and I
> > wonder who uses it.  But in any case what you've proposed is
> > extensible enough that we would be able to add other methods to pass
> > the key in future.
> 
> As I suggested in my reply to Eric's email, we can always create a
"fd"
> selector, if needed.
> 
> -- 
> Pino Toscano

> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v