On Fri, 5 Mar 2010 17:47:33 +0200 Shahar Havivi <shah...@redhat.com> wrote:
> On Fri, Mar 05, 2010 at 04:22:55PM +0100, Kevin Wolf wrote: > > Date: Fri, 05 Mar 2010 16:22:55 +0100 > > From: Kevin Wolf <kw...@redhat.com> > > To: Shahar Havivi <shah...@redhat.com> > > CC: qemu-devel@nongnu.org, Dor Laor <dl...@redhat.com>, > > Luiz Capitulino <lcapi...@redhat.com> > > Subject: Re: [PATCH] Wrong error message in block_passwd command > > > > Am 05.03.2010 16:12, schrieb Shahar Havivi: > > > Monitor command 'block_passwd' reports a wrong error message when > > > drive is not encrypted > > > > > > Signed-off-by: Shahar Havivi <shah...@redhat.com> > > > --- > > > block.c | 9 ++++++--- > > > monitor.c | 7 ++++++- > > > qerror.c | 4 ++++ > > > qerror.h | 3 +++ > > > 4 files changed, 19 insertions(+), 4 deletions(-) > > > > > > diff --git a/block.c b/block.c > > > index 31d1ba4..dd484fa 100644 > > > --- a/block.c > > > +++ b/block.c > > > @@ -1132,10 +1132,13 @@ int bdrv_set_key(BlockDriverState *bs, const char > > > *key) > > > if (ret < 0) > > > return ret; > > > if (!bs->encrypted) > > > - return 0; > > > + return -EINVAL; > > > + } > > > > I think this part is wrong actually. Sorry for not catching it when you > > sent me the patch first. > > > > The logic here is that it's not an error to set the password for a file > > which isn't encrypted itself, but its backing file is. In this case the > > key is only set for the backing file and we return success. > > > > The other parts look good to me. > > > > Kevin > You right Kevin, > Revert returning error when backing file in bdrv_set_key. Looks good to me too, but it's better to send it as new message, as maintainers can miss patches submitted in the middle of thread.