On 12/7/23 16:08, Richard W.M. Jones wrote: Reviewed-by: Laszlo Ersek <lersek(a)redhat.com&gt;

On 12/7/23 16:08, Richard W.M. Jones wrote: Should we use C<NAME> and C<UUID>? The removed part was C<ID>, not just ID. Reviewed-by: Laszlo Ersek <lersek(a)redhat.com&gt;

virt-v2v (and other virt tools using --key) forces you to supply keys on the command line per-device, eg. --key UUID:SELECTOR where UUID is the UUID of the encrypted device. In practice this requires users to pre-inspect guests to find out what device UUIDs they are using (or have some out-of-band method), match keys to UUIDs, and then pass the result to virt-v2v. Obviously this is a pain in the neck for users, as predicted when the feature was initially added. Although it is more efficient for virt-v2v -- because attempting to decrypt a device with an invalid key is slow -- there is a significant number of VMs (probably the vast majority) that only have a single encrypted device, so this isn't an issue in most cases. This change allows --key all:SELECTOR to be used, to try the key against any encrypted device encountered. --- options/key-option.pod | 9 ++++++++- options/keys.c | 1 + options/options.h | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/options/key-option.pod b/options/key-option.pod index ee515fa..1470d86 100644 --- a/options/key-option.pod +++ b/options/key-option.pod @@ -9,8 +9,11 @@ the inspection. =item B<--key> UUIDB<:key:>KEY_STRING +=item B<--key> B<all:key:>KEY_STRING + C<NAME> is the libguestfs device name (eg. C</dev/sda1>). C<UUID> is -the device UUID. +the device UUID. C<all> means try the key against any encrypted +device. Use the specified C<KEY_STRING> as passphrase. @@ -18,12 +21,16 @@ Use the specified C<KEY_STRING> as passphrase. =item B<--key> UUIDB<:file:>FILENAME +=item B<--key> B<all:file:>FILENAME + Read the passphrase from F<FILENAME>. =item B<--key> NAMEB<:clevis> =item B<--key> UUIDB<:clevis> +=item B<--key> B<all:clevis> + Attempt passphrase-less unlocking for the device with Clevis, over the network. Please refer to L<guestfs(3)/ENCRYPTED DISKS> for more information on network-bound disk encryption (NBDE). diff --git a/options/keys.c b/options/keys.c index 3a49273..87acba5 100644 --- a/options/keys.c +++ b/options/keys.c @@ -156,6 +156,7 @@ get_keys (struct key_store *ks, const char *device, const char *uuid, bool key_id_matches_this_device; key_id_matches_this_device = + STREQ (key->id, "all") || /* special string "all" matches any device */ STREQ (key->id, device) || (uuid && STREQ (key->id, uuid)); if (!key_id_matches_this_device) continue; diff --git a/options/options.h b/options/options.h index 94e8b9e..dcb15c2 100644 --- a/options/options.h +++ b/options/options.h @@ -109,6 +109,8 @@ struct key_store_key { * device name, or the UUID. * * There may be multiple matching devices in the list. + * + * This may be the special string "all" which matches any device. */ char *id; -- 2.43.0

On 12/7/23 16:08, Richard W.M. Jones wrote: With a large number of devices and a matching number of "all" keys, the quadratic slowdown could become serious (on average N/2 keys tried for each of N devices). But, that depends on the VM to convert, and how the user chooses to pass in the keys. Not a counter-argument, just something to be aware of, perhaps. "--key all:SELECTOR" is not precise, considering the documentation; the option format is "--key SELECTOR", so "all:" is a part of (i.e., the prefix of) SELECTOR. Instead, This change enables "all:*" selectors to be used with with --key could be more precise. Or, well, not. :) Either way, this only concerns the commit message, not the code or the docs (and the series is upstream anyway). The patch looks good to me: Reviewed-by: Laszlo Ersek <lersek(a)redhat.com&gt; Thanks! Laszlo