qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 2/2] block: move the bdrv_dev_change_media_cb


From: Pavel Hrdina
Subject: Re: [Qemu-devel] [PATCH v2 2/2] block: move the bdrv_dev_change_media_cb()
Date: Mon, 17 Jun 2013 15:38:23 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6

On 17.6.2013 15:32, Luiz Capitulino wrote:
On Mon, 17 Jun 2013 15:25:24 +0200
Pavel Hrdina <address@hidden> wrote:

On 17.6.2013 15:22, Luiz Capitulino wrote:
On Mon, 17 Jun 2013 14:33:10 +0200
Stefan Hajnoczi <address@hidden> 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.


In that case, how do you think, that we should handle the situation
that user is trying to open device that isn't require the password, but
user will provide the password?

I don't think that we should fail and abort that operation.

Pavel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]