On Mon, 17 Jun 2013 15:25:24 +0200 Pavel Hrdina <phrd...@redhat.com> wrote:
> On 17.6.2013 15:22, Luiz Capitulino wrote: > > On Mon, 17 Jun 2013 14:33:10 +0200 > > Stefan Hajnoczi <stefa...@gmail.com> wrote: > > > >> On Mon, Jun 17, 2013 at 11:46:19AM +0200, Pavel Hrdina wrote: > >>> On 5.6.2013 15:23, Stefan Hajnoczi wrote: > >>>> On Wed, May 29, 2013 at 06:18:19PM +0200, Pavel Hrdina wrote: > >>>>> @@ -1071,14 +1072,18 @@ static void > >>>>> qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename, > >>>>> if (password) { > >>>>> if (bdrv_set_key(bs, password) < 0) { > >>>>> error_set(errp, QERR_INVALID_PASSWORD); > >>>>> + return; > >>>>> } > >>>>> } else { > >>>>> error_set(errp, QERR_DEVICE_ENCRYPTED, > >>>>> bdrv_get_device_name(bs), > >>>>> bdrv_get_encrypted_filename(bs)); > >>>>> + return; > >>>>> } > >>>>> } else if (password) { > >>>>> error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, > >>>>> bdrv_get_device_name(bs)); > >>>>> } > >>>>> + > >>>>> + bdrv_dev_change_media_cb(bs, true); > >>>>> } > >>>> > >>>> Calling bdrv_dev_change_media_cb() after raising > >>>> QERR_DEVICE_NOT_ENCRYPTED is intentional? It might warrant a comment. > >>>> > >>> > >>> Sorry for my late answer. > >>> > >>> It's just a warning, that you used a password for a block device that > >>> doesn't require it. The device is opened successfully and should be > >>> handled correctly (call the bdrv_dev_change_media_cb() ). > >> > >> Yep, IMO it's worth a comment that this isn't an "error" just a > >> "warning". > > > > Actually, you can't have such a warning in QMP. You either fail or you > > succeed. We should just do what the current code does. > > > > This is the same logic as the old one. The device is loaded but the > error is emitted. That's a bug if the operation succeeded.