Re: [Qemu-devel] [PATCH V8 08/14] Introduce file lock for the block layer

[Stefan Berger]

On 09/07/2011 10:35 AM, Michael S. Tsirkin wrote:
> On Wed, Sep 07, 2011 at 10:25:08AM -0400, Stefan Berger wrote:
> > On 09/07/2011 10:10 AM, Michael S. Tsirkin wrote:
> > > On Wed, Sep 07, 2011 at 09:56:52AM -0400, Stefan Berger wrote:
> > > > On 09/07/2011 09:16 AM, Michael S. Tsirkin wrote:
> > > > > On Wed, Sep 07, 2011 at 09:06:05AM -0400, Stefan Berger wrote:
> > > > > > > > First: There are two ways to encrypt the data.
> > > > > > > >
> > > > > > > > One comes with the QCoW2 type of image and it comes for free. Set
> > > > > > > > the encryption flag when creating the QCoW2 file and one has to
> > > > > > > > provide a key to access the QCoW2. I found this mode problematic for
> > > > > > > > users since it required me to go through the monitor every time I
> > > > > > > > started the VM. Besides that the key is provided so late that all
> > > > > > > > devices are already initialized and if the wrong key was provided
> > > > > > > > the only thing the TPM can do is to go into shutdown mode since
> > > > > > > > there is state on the QCoW2 but it cannot be decrypted. This also
> > > > > > > > became problematic when doing migrations with libvirt for example
> > > > > > > > and one was to have a wrong key/password installed on the target
> > > > > > > > side -- graceful termination of the migration is impossible.
> > > > > OK let's go back to this for a moment. Add a load
> > > > > callback, access file there. On failure, return
> > > > > an error. migration fails gracefully, and
> > > > > management can retry, or migrate to another node,
> > > > > or whatever.
> > > > >
> > > > > What's the problem exactly?
> > > > The switch-over from source to destination already happened when the
> > > > key is finally passed and you just won't be able to access the QCoW2
> > > > in case the key was wrong.
> > > This is exactly what happens with any kind of othe rmigration errror.
> > > So fail migration, and source can get restarted if necessary.
> > I guess I wanted to improve on this and catch user errors.
> > If we let migration fail then all you can do is try to terminate the
> > VM on the destination and cold-start on the source.
> No, normally if migration fails VM is not started on destination,
> and it can just continue on source.
When I had tried this in conjunction with encrypted QCoW2 the 
switch-over already had happened and the source had died. So a wrong key 
on the destination was fatal.
> > > > Similar problems occur when you start a
> > > > VM with an encrypted QCoW2 image. The monitor will prompt you for
> > > > the password and then you start the VM and if the password was wrong
> > > > the OS just won't be able to access the image.
> > > >
> > > >     Stefan
> > > Why can't you verify the password?
> > I do verify the key/password in the TPM driver. If the driver cannot
> > make sense of the contents of the QCoW2 due to wrong key I simply
> > put the driver into failure mode. That's all I can do with encrypted
> > QCoW2.
> You can return error from init script which will make qemu exit.
I can return an error code when the front- and backend interfaces are 
initialized, but that happens really early and the encyrption key 
entered through the monitor is not available at this point.

I also don't get a notification about when the key was entered. In case 
of QCoW2 encryption (and migration) I delay initialization until very 
late, basically when the VM accesses the tpm tis hardware emulation 
layer again (needs to be done this way I think to support block 
migration where I cannot even access the block device early on at all). 
Only then I find out that the key was wrong. I guess any other handling 
would require blockdev.c's invocation of monitor_read_bdrv_key_start() 
to be 'somehow' extended and ... do what ? loop until the correct 
password was entered?

   Stefan
> > In case of a QCoW2 encrypted VM image it's different. There I guess
> > the intelligence of what is good and bad data is only inside the OS.
> >
> >     Stefan