| Message ID | 1431446962-9860-5-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: > The qemu-io tool does not check if the image is encrypted so > historically would silently corrupt the sectors by writing > plain text data into them instead of cipher text. The earlier > commit turns this mistake into a fatal abort, so check for > encryption and prompt for key when required. Doesn't that mean that 'git bisect' gives a crashing qemu-io for 3 patches? Should this be rearranged so that 1/5 comes after this to avoid triggering the abort? > > This enables us to add unit tests to ensure we don't break > the ability of qemu-img to convert existing encrypted qcow2 > files into a non-encrypted format. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > qemu-io.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > Reviewed-by: Eric Blake <eblake@redhat.com>
On Tue, May 12, 2015 at 12:32:53PM -0600, Eric Blake wrote: > On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: > > The qemu-io tool does not check if the image is encrypted so > > historically would silently corrupt the sectors by writing > > plain text data into them instead of cipher text. The earlier > > commit turns this mistake into a fatal abort, so check for > > encryption and prompt for key when required. > > Doesn't that mean that 'git bisect' gives a crashing qemu-io for 3 > patches? Should this be rearranged so that 1/5 comes after this to > avoid triggering the abort? I'm ambivalent on that - previously qemu-io was data corrupting for this scenario, so crashing isn't really that much worse :-) It is easy enough to reorder these though if that's desired. The latter patches have no build time dep on the 1st patch, so its trivial for Kevin to re-order when applying if he thinks it is worth it. Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Tue, May 12, 2015 at 12:32:53PM -0600, Eric Blake wrote: >> On 05/12/2015 10:09 AM, Daniel P. Berrange wrote: >> > The qemu-io tool does not check if the image is encrypted so >> > historically would silently corrupt the sectors by writing >> > plain text data into them instead of cipher text. The earlier >> > commit turns this mistake into a fatal abort, so check for >> > encryption and prompt for key when required. >> >> Doesn't that mean that 'git bisect' gives a crashing qemu-io for 3 >> patches? Should this be rearranged so that 1/5 comes after this to >> avoid triggering the abort? > > I'm ambivalent on that - previously qemu-io was data corrupting > for this scenario, so crashing isn't really that much worse :-) If it crashes before it can corrupt anything, I'd sell it as an improvement ;) [...]
diff --git a/qemu-io.c b/qemu-io.c index 8e41080..34ae933 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -52,6 +52,7 @@ static const cmdinfo_t close_cmd = { static int openfile(char *name, int flags, QDict *opts) { Error *local_err = NULL; + BlockDriverState *bs; if (qemuio_blk) { fprintf(stderr, "file open already, try 'help close'\n"); @@ -68,7 +69,27 @@ static int openfile(char *name, int flags, QDict *opts) return 1; } + bs = blk_bs(qemuio_blk); + if (bdrv_is_encrypted(bs)) { + char password[256]; + printf("Disk image '%s' is encrypted.\n", name); + if (qemu_read_password(password, sizeof(password)) < 0) { + error_report("No password given"); + goto error; + } + if (bdrv_set_key(bs, password) < 0) { + error_report("invalid password"); + goto error; + } + } + + return 0; + + error: + blk_unref(qemuio_blk); + qemuio_blk = NULL; + return 1; } static void open_help(void)
The qemu-io tool does not check if the image is encrypted so historically would silently corrupt the sectors by writing plain text data into them instead of cipher text. The earlier commit turns this mistake into a fatal abort, so check for encryption and prompt for key when required. This enables us to add unit tests to ensure we don't break the ability of qemu-img to convert existing encrypted qcow2 files into a non-encrypted format. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemu-io.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)