Re: [Qemu-devel] [PATCH 5/7] vdi: Support .bdrv_co_create

2018-03-12 Thread Max Reitz
On 2018-03-09 22:46, Kevin Wolf wrote:
> This adds the .bdrv_co_create driver callback to vdi, which
> enables image creation over QMP.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  qapi/block-core.json |  21 ++-
>  block/vdi.c  | 169 
> ++-
>  2 files changed, 148 insertions(+), 42 deletions(-)

As I said on IRC, I'd prefer cluster-size not to be exposed over QAPI,
seeing it is not supported by default (and you can't enable it through
configure, you can only enable it by modifying vdi.c), and that it is
explicitly untested.

Max



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 5/7] vdi: Support .bdrv_co_create

2018-03-09 Thread Kevin Wolf
This adds the .bdrv_co_create driver callback to vdi, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  21 ++-
 block/vdi.c  | 169 ++-
 2 files changed, 148 insertions(+), 42 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1e2edbc063..2eba0eef7e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3680,6 +3680,25 @@
 'size': 'size' } }
 
 ##
+# @BlockdevCreateOptionsVdi:
+#
+# Driver specific image creation options for vdi.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @cluster-size Cluster size in bytes (default: 1 MB)
+# @static   Whether to create a static (preallocated) image
+#   (default: false)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsVdi',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*cluster-size':'size',
+'*static':  'bool' } }
+
+##
 # @BlockdevCreateNotSupported:
 #
 # This is used for all drivers that don't support creating images.
@@ -3733,7 +3752,7 @@
   'sheepdog':   'BlockdevCreateOptionsSheepdog',
   'ssh':'BlockdevCreateOptionsSsh',
   'throttle':   'BlockdevCreateNotSupported',
-  'vdi':'BlockdevCreateNotSupported',
+  'vdi':'BlockdevCreateOptionsVdi',
   'vhdx':   'BlockdevCreateNotSupported',
   'vmdk':   'BlockdevCreateNotSupported',
   'vpc':'BlockdevCreateNotSupported',
diff --git a/block/vdi.c b/block/vdi.c
index 2b5ddd0666..c60ddc58c0 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -60,6 +60,9 @@
 #include "qemu/coroutine.h"
 #include "qemu/cutils.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 
 /* Code configuration options. */
 
@@ -182,6 +185,8 @@ typedef struct {
 Error *migration_blocker;
 } BDRVVdiState;
 
+static QemuOptsList vdi_create_opts;
+
 static void vdi_header_to_cpu(VdiHeader *header)
 {
 le32_to_cpus(&header->signature);
@@ -716,67 +721,72 @@ nonallocating_write:
 return ret;
 }
 
-static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts 
*opts,
-   Error **errp)
+static int coroutine_fn vdi_co_create(BlockdevCreateOptions *opts,
+  Error **errp)
 {
+BlockdevCreateOptionsVdi *vdi_opts;
+BlockBackend *blk = NULL;
+BlockDriverState *bs = NULL;
+
 int ret = 0;
-uint64_t bytes = 0;
 uint32_t blocks;
-size_t block_size = DEFAULT_CLUSTER_SIZE;
 uint32_t image_type = VDI_TYPE_DYNAMIC;
 VdiHeader header;
 size_t i;
 size_t bmap_size;
 int64_t offset = 0;
-Error *local_err = NULL;
-BlockBackend *blk = NULL;
 uint32_t *bmap = NULL;
 
 logout("\n");
 
-/* Read out options. */
-bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
- BDRV_SECTOR_SIZE);
-#if defined(CONFIG_VDI_BLOCK_SIZE)
-/* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
-block_size = qemu_opt_get_size_del(opts,
-   BLOCK_OPT_CLUSTER_SIZE,
-   DEFAULT_CLUSTER_SIZE);
-#endif
-#if defined(CONFIG_VDI_STATIC_IMAGE)
-if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) {
-image_type = VDI_TYPE_STATIC;
+assert(opts->driver == BLOCKDEV_DRIVER_VDI);
+vdi_opts = &opts->u.vdi;
+
+/* Validate options and set default values */
+if (!vdi_opts->has_cluster_size) {
+vdi_opts->cluster_size = DEFAULT_CLUSTER_SIZE;
 }
-#endif
 
-if (bytes > VDI_DISK_SIZE_MAX) {
-ret = -ENOTSUP;
+if (vdi_opts->size > VDI_DISK_SIZE_MAX) {
 error_setg(errp, "Unsupported VDI image size (size is 0x%" PRIx64
   ", max supported is 0x%" PRIx64 ")",
-  bytes, VDI_DISK_SIZE_MAX);
-goto exit;
+  vdi_opts->size, VDI_DISK_SIZE_MAX);
+return -ENOTSUP;
 }
 
-ret = bdrv_create_file(filename, opts, &local_err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-goto exit;
+#if !defined(CONFIG_VDI_BLOCK_SIZE)
+if (vdi_opts->has_cluster_size) {
+error_setg(errp, "Non-default cluster size not supported");
+return -ENOTSUP;
+}
+#endif
+#if !defined(CONFIG_VDI_STATIC_IMAGE)
+if (vdi_opts->has_static) {
+error_setg(errp, "Static images not supported");
+return -ENOTSUP;
+}
+#else
+if (vdi_opts->q_static) {
+image_type = VDI_TYPE_STATIC;
+}
+#endif
+
+/* Create BlockBackend to write to the image */
+bs = bdrv_open_blockdev_ref(vdi_opts->file, errp);
+if (bs == NULL) {
+