| Message ID | 1399030002-27222-15-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/02/2014 05:26 AM, Markus Armbruster wrote: > Using error_is_set(errp) that way can sweep programming errors under > the carpet when we get called incorrectly with an error set. > > encrypted_bdrv_it() does it, because there's no way to make > bdrv_iterate() break its loop. Actually safe, because qmp_cont() > clears the error before the loop. Clean it up anyway: replace > bdrv_iterate() by bdrv_next(), break the loop on error. > > Replace both occurences, for consistency. s/occurences/occurrences/ > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> Fixing up the commit message typo is trivial and doesn't alter the positive review.
Eric Blake <eblake@redhat.com> writes: > On 05/02/2014 05:26 AM, Markus Armbruster wrote: >> Using error_is_set(errp) that way can sweep programming errors under >> the carpet when we get called incorrectly with an error set. >> >> encrypted_bdrv_it() does it, because there's no way to make >> bdrv_iterate() break its loop. Actually safe, because qmp_cont() >> clears the error before the loop. Clean it up anyway: replace >> bdrv_iterate() by bdrv_next(), break the loop on error. >> >> Replace both occurences, for consistency. > > s/occurences/occurrences/ > >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> > > Fixing up the commit message typo is trivial and doesn't alter the > positive review. Luiz, could you fix it up on commit, when you add Eric's R-by?
On Wed, 07 May 2014 08:08:30 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Eric Blake <eblake@redhat.com> writes: > > > On 05/02/2014 05:26 AM, Markus Armbruster wrote: > >> Using error_is_set(errp) that way can sweep programming errors under > >> the carpet when we get called incorrectly with an error set. > >> > >> encrypted_bdrv_it() does it, because there's no way to make > >> bdrv_iterate() break its loop. Actually safe, because qmp_cont() > >> clears the error before the loop. Clean it up anyway: replace > >> bdrv_iterate() by bdrv_next(), break the loop on error. > >> > >> Replace both occurences, for consistency. > > > > s/occurences/occurrences/ > > > >> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> Reviewed-by: Eric Blake <eblake@redhat.com> > > > > Fixing up the commit message typo is trivial and doesn't alter the > > positive review. > > Luiz, could you fix it up on commit, when you add Eric's R-by? Fixed. But I didn't have to add his R-bys, you did it already.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Wed, 07 May 2014 08:08:30 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Eric Blake <eblake@redhat.com> writes: >> >> > On 05/02/2014 05:26 AM, Markus Armbruster wrote: >> >> Using error_is_set(errp) that way can sweep programming errors under >> >> the carpet when we get called incorrectly with an error set. >> >> >> >> encrypted_bdrv_it() does it, because there's no way to make >> >> bdrv_iterate() break its loop. Actually safe, because qmp_cont() >> >> clears the error before the loop. Clean it up anyway: replace >> >> bdrv_iterate() by bdrv_next(), break the loop on error. >> >> >> >> Replace both occurences, for consistency. >> > >> > s/occurences/occurrences/ >> > >> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> > >> > Fixing up the commit message typo is trivial and doesn't alter the >> > positive review. >> >> Luiz, could you fix it up on commit, when you add Eric's R-by? > > Fixed. But I didn't have to add his R-bys, you did it already. -EFRIEDBRAIN. Thanks!
diff --git a/qmp.c b/qmp.c index ee8357f..df10c7f 100644 --- a/qmp.c +++ b/qmp.c @@ -146,24 +146,9 @@ SpiceInfo *qmp_query_spice(Error **errp) }; #endif -static void iostatus_bdrv_it(void *opaque, BlockDriverState *bs) -{ - bdrv_iostatus_reset(bs); -} - -static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs) -{ - Error **errp = opaque; - - if (!error_is_set(errp) && bdrv_key_required(bs)) { - error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs), - bdrv_get_encrypted_filename(bs)); - } -} - void qmp_cont(Error **errp) { - Error *local_err = NULL; + BlockDriverState *bs; if (runstate_needs_reset()) { error_setg(errp, "Resetting the Virtual Machine is required"); @@ -172,11 +157,16 @@ void qmp_cont(Error **errp) return; } - bdrv_iterate(iostatus_bdrv_it, NULL); - bdrv_iterate(encrypted_bdrv_it, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; + for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + bdrv_iostatus_reset(bs); + } + for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + if (bdrv_key_required(bs)) { + error_set(errp, QERR_DEVICE_ENCRYPTED, + bdrv_get_device_name(bs), + bdrv_get_encrypted_filename(bs)); + return; + } } if (runstate_check(RUN_STATE_INMIGRATE)) {