Luiz Capitulino <lcapitul...@redhat.com> writes: > Today, hmp_cont() checks for QERR_DEVICE_ENCRYPTED in order to know > if qmp_cont() failed due to an encrypted device. If it did, > hmp_cont() accesses QERR_DEVICE_ENCRYPTED's data member 'device' to > precisely know for which device an encryption key must be set. > > However, all errors data members are going to be dropped by a later > commit, so hmp_cont() can't do this anymore. > > This commit changes hmp_cont() to loop through all block devices > and proactively set an encryption key for any encrypted device > without a valid one. > > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com> > --- > hmp.c | 43 +++++++++++++++++++++++++------------------ > 1 file changed, 25 insertions(+), 18 deletions(-) > > diff --git a/hmp.c b/hmp.c > index 6b72a64..a906f8a 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -610,34 +610,41 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict) > > static void hmp_cont_cb(void *opaque, int err) > { > - Monitor *mon = opaque; > - > if (!err) { > - hmp_cont(mon, NULL); > + qmp_cont(NULL); > } > } > > -void hmp_cont(Monitor *mon, const QDict *qdict) > +static bool block_dev_is_encrypted(const BlockInfo *bdev) > { > - Error *errp = NULL; > - > - qmp_cont(&errp); > - if (error_is_set(&errp)) { > - if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) { > - const char *device; > + return (bdev->inserted && bdev->inserted->encrypted); > +} > > - /* The device is encrypted. Ask the user for the password > - and retry */ > +static bool block_dev_key_is_set(const BlockInfo *bdev) > +{ > + return (bdev->inserted && bdev->inserted->valid_encryption_key); > +}
New static helpers block_dev_is_encrypted(), block_dev_key_is_set(). They work on BlockInfo. Call them blockinfo_{is_encrypted,key_is_set}()? > > - device = error_get_field(errp, "device"); > - assert(device != NULL); > +void hmp_cont(Monitor *mon, const QDict *qdict) > +{ > + BlockInfoList *bdev_list, *bdev; > + Error *errp = NULL; > > - monitor_read_block_device_key(mon, device, hmp_cont_cb, mon); > - error_free(errp); > - return; > + bdev_list = qmp_query_block(NULL); > + for (bdev = bdev_list; bdev; bdev = bdev->next) { > + if (block_dev_is_encrypted(bdev->value) && > + !block_dev_key_is_set(bdev->value)) { > + monitor_read_block_device_key(mon, bdev->value->device, > + hmp_cont_cb, NULL); > + goto out; Any particular reason for creating BlockInfos just to find out whether we lack the key? Why not bdrv_key_required()? > } > - hmp_handle_error(mon, &errp); > } > + > + qmp_cont(&errp); > + hmp_handle_error(mon, &errp); > + > +out: > + qapi_free_BlockInfoList(bdev_list); > } > > void hmp_system_wakeup(Monitor *mon, const QDict *qdict) Diff makes this change look worse than it is. Odd: M-x ediff gets it right. Anyway, here's how I think it works: Unchanged qmp_cont(): search the bdrv_states for the first encrypted one without a key. If found, set err argument to QERR_DEVICE_ENCRYPTED. Other errors unrelated to encrypted devices are also possible. hmp_cont() before: try qmp_cont(). If we get QERR_DEVICE_ENCRYPTED, extract the device from the error object, and prompt for its key, with a callback that retries hmp_cont() if the key was provided. After: search the bdrv_states for an encrypted one without a key. If there is none, qmp_cont(), no special error handling. If there is one, prompt for its key, with a callback that runs qmp_cont() if the key was provided. Have you tested this with multiple devices lacking their key?