[Qemu-devel] [PATCH v8 0/4] Improve convert and dd commands

Daniel P. Berrange posted 4 patches 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170509094837.22852-1-berrange@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
qemu-img-cmds.hx |   4 +-
qemu-img.c       | 145 +++++++++++++++++++++++++++++++++++++++++++------------
qemu-img.texi    |  12 ++++-
3 files changed, 126 insertions(+), 35 deletions(-)
[Qemu-devel] [PATCH v8 0/4] Improve convert and dd commands
Posted by Daniel P. Berrange 6 years, 11 months ago
Update to

  v1: https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05699.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00728.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg04391.html
  v4: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02153.html
  v5: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg04109.html
  v6: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00215.html

This series is in response to Max pointing out that you cannot
use 'convert' for an encrypted target image.

The 'convert' and 'dd' commands need to first create the image
and then open it. The bdrv_create() method takes a set of options
for creating the image, which let us provide a key-secret for the
encryption key. When the commands then open the new image, they
don't provide any options, so the image is unable to be opened
due to lack of encryption key. It is also not possible to use
the --image-opts argument to provide structured options in the
target image name - it must be a plain filename to satisfy the
bdrv_create() API contract.

This series addresses these problems to some extent

 - Adds a new --target-image-opts flag which is used to say
   that the target filename is using structured options.
   It is *only* permitted to use this when -n is also set.
   ie the target image must be pre-created so convert/dd
   don't need to run bdrv_create().

 - When --target-image-opts is not used, add special case
   code that identifies options passed to bdrv_create()
   named "*key-secret" and adds them to the options used
   to open the new image

In future it is desirable to make --target-image-opts work even when -n is
*not* given. This requires considerable work to create a new bdrv_create()
API impl.

The first patch fixes a bug in the 'dd' command while the second adds support
for the missing '--object' arg to 'dd', allowing it to reference secrets when
opening files.  The last two patches implement the new features described above
for the 'convert' command.

NB v8 is based against git master once more, since the img_convert changes
previously in block-next have now merged.

Changed in v8:

 - Readd accidentally dropped check for compression (Max)
 - Fix indentation of variable declaration (Max)
 - Fix goto jump target (Max)

Changed in v7:

 - Drop the (accidentally included) revert patch (Eric)

Changed in v6:

 - Fix misc typos (Fam)
 - Resolve messy conflicts wrt max/block-next (Max)

Changed in v5:

 - Fix return value (Max)
 - Misc doc changes (Max)
 - Use error_abort (Max)

Changed in v4:

 - Refactor img_open_new_file in terms of img_open_file (Kevin)

Changed in v3:

 - Drop all patches affecting the 'dd' command except for the clear bug fix
   and the --object support. They can be re-considered once dd is rewritten
   to run ontop of convert.
 - Use consistent return/goto style in dd command (Max)
 - Fix error reporting when using compressed image and skip-create (Max)
 - Unconditionally create QDict when open files (Max)

Changed in v2:

 - Replace dd -n flag with support for conv=nocreat,notrunc
 - Misc typos (Eric, Fam)


Daniel P. Berrange (4):
  qemu-img: add support for --object with 'dd' command
  qemu-img: fix --image-opts usage with dd command
  qemu-img: introduce --target-image-opts for 'convert' command
  qemu-img: copy *key-secret opts when opening newly created files

 qemu-img-cmds.hx |   4 +-
 qemu-img.c       | 145 +++++++++++++++++++++++++++++++++++++++++++------------
 qemu-img.texi    |  12 ++++-
 3 files changed, 126 insertions(+), 35 deletions(-)

-- 
2.9.3


Re: [Qemu-devel] [PATCH v8 0/4] Improve convert and dd commands
Posted by Max Reitz 6 years, 11 months ago
On 2017-05-09 11:48, Daniel P. Berrange wrote:
> Update to
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05699.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00728.html
>   v3: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg04391.html
>   v4: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02153.html
>   v5: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg04109.html
>   v6: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00215.html
> 
> This series is in response to Max pointing out that you cannot
> use 'convert' for an encrypted target image.
> 
> The 'convert' and 'dd' commands need to first create the image
> and then open it. The bdrv_create() method takes a set of options
> for creating the image, which let us provide a key-secret for the
> encryption key. When the commands then open the new image, they
> don't provide any options, so the image is unable to be opened
> due to lack of encryption key. It is also not possible to use
> the --image-opts argument to provide structured options in the
> target image name - it must be a plain filename to satisfy the
> bdrv_create() API contract.
> 
> This series addresses these problems to some extent
> 
>  - Adds a new --target-image-opts flag which is used to say
>    that the target filename is using structured options.
>    It is *only* permitted to use this when -n is also set.
>    ie the target image must be pre-created so convert/dd
>    don't need to run bdrv_create().
> 
>  - When --target-image-opts is not used, add special case
>    code that identifies options passed to bdrv_create()
>    named "*key-secret" and adds them to the options used
>    to open the new image
> 
> In future it is desirable to make --target-image-opts work even when -n is
> *not* given. This requires considerable work to create a new bdrv_create()
> API impl.
> 
> The first patch fixes a bug in the 'dd' command while the second adds support
> for the missing '--object' arg to 'dd', allowing it to reference secrets when
> opening files.  The last two patches implement the new features described above
> for the 'convert' command.
> 
> NB v8 is based against git master once more, since the img_convert changes
> previously in block-next have now merged.

Changes from the previous version look good, but unfortunately here's
the "but": The image locking series has brought even more changes to
qemu-img. :-(

I tried resolving them, but the following backport-diff didn't look like
I should proceed:

001/4:[----] [-C] 'qemu-img: add support for --object with 'dd' command'
002/4:[0004] [FC] 'qemu-img: fix --image-opts usage with dd command'
003/4:[0015] [FC] 'qemu-img: introduce --target-image-opts for 'convert'
command'
004/4:[0024] [FC] 'qemu-img: copy *key-secret opts when opening newly
created files'

The fun is increased by the fact that the locking series has
(inadvertently) removed the -B documentation from convert, so there is
another conflict looming in the future...

(Or you just inadvertently add it back. Then we'd have resolved the
issue altogether...)

Max

Re: [Qemu-devel] [PATCH v8 0/4] Improve convert and dd commands
Posted by Fam Zheng 6 years, 11 months ago
On Fri, 05/12 19:41, Max Reitz wrote:
> The fun is increased by the fact that the locking series has
> (inadvertently) removed the -B documentation from convert, so there is
> another conflict looming in the future...

Sorry about the mistake there..

I've posted a patch for that:

[Qemu-devel] [PATCH] qemu-img: Fix documentation of convert

Fam