[Qemu-devel] [PATCH 0/3] Add -blockdev command line option

[Qemu-devel] [PATCH 0/3] Add -blockdev command line option

[Kevin Wolf]

This series adds an option that is directly mapped to the blockdev-add QMP
command. It works more or less like -drive, except that it doesn't create a
BlockBackend (creating just a BDS without a BB is impossible with -drive) and
doesn't support legacy options.

Depends on Dan's "[PATCH v13 0/6] QAPI/QOM work for non-scalar object
properties".

Kevin Wolf (3):
  block: Add '-blockdev' command line option
  doc: Document generic -blockdev options
  doc: Document driver-specific -blockdev options

 blockdev.c              |  12 ++++
 include/sysemu/sysemu.h |   1 +
 qemu-options.hx         | 172 ++++++++++++++++++++++++++++++++++++++++--------
 vl.c                    |  57 ++++++++++++++++
 4 files changed, 213 insertions(+), 29 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] block: Add '-blockdev' command line option

[Kevin Wolf]

This is an option that is directly mapped to the blockdev-add QMP
command. It works more or less like -drive, except that it doesn't
create a BlockBackend and doesn't support legacy options.

This patch adds minimal documentation, the next patches will improve it.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c              | 12 +++++++++++
 include/sysemu/sysemu.h |  1 +
 qemu-options.hx         | 12 +++++++++++
 vl.c                    | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 76b86ab..5931383 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4075,3 +4075,15 @@ QemuOptsList qemu_drive_opts = {
         { /* end of list */ }
     },
 };
+
+QemuOptsList qemu_blockdev_opts = {
+    .name = "blockdev",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_blockdev_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any params
+         * validation will happen later
+         */
+        { /* end of list */ }
+    },
+};
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7c760..1a2013a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -238,6 +238,7 @@ bool defaults_enabled(void);
 extern QemuOptsList qemu_legacy_drive_opts;
 extern QemuOptsList qemu_common_drive_opts;
 extern QemuOptsList qemu_drive_opts;
+extern QemuOptsList qemu_blockdev_opts;
 extern QemuOptsList qemu_chardev_opts;
 extern QemuOptsList qemu_device_opts;
 extern QemuOptsList qemu_netdev_opts;
diff --git a/qemu-options.hx b/qemu-options.hx
index 0b621bb..542c483 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -513,6 +513,18 @@ Use @var{file} as CD-ROM image (you cannot use @option{-hdc} and
 using @file{/dev/cdrom} as filename (@pxref{host_drives}).
 ETEXI
 
+DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
+    "-blockdev driver=driver[,node-name=n][,read-only=on|off]\n"
+    "       [,cache.direct=on|off][,cache.no-flush=on|off]\n"
+    "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
+    "       [,...driver specific...]\n", QEMU_ARCH_ALL)
+STEXI
+@item -blockdev @var{option}[,@var{option}[,@var{option}[,...]]]
+@findex -blockdev
+
+Define a new block driver node.
+ETEXI
+
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
diff --git a/vl.c b/vl.c
index fca0487..dbf7849 100644
--- a/vl.c
+++ b/vl.c
@@ -122,6 +122,9 @@ int main(int argc, char **argv)
 #include "sysemu/replay.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/iothread.h"
+#include "qapi-visit.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/dealloc-visitor.h"
 
 #define MAX_VIRTIO_CONSOLES 1
 #define MAX_SCLP_CONSOLES 1
@@ -1132,6 +1135,46 @@ static int cleanup_add_fd(void *opaque, QemuOpts *opts, Error **errp)
 #define MTD_OPTS ""
 #define SD_OPTS ""
 
+static int blockdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    BlockdevOptions *options;
+    Visitor *v = NULL;
+    Error *local_err = NULL;
+
+    QDict *opts_dict = qemu_opts_to_qdict(opts, NULL);
+    QObject *crumpled = qdict_crumple(opts_dict, true, &local_err);
+    if (local_err) {
+        goto fail;
+    }
+
+    v = qobject_string_input_visitor_new(crumpled);
+    visit_type_BlockdevOptions(v, NULL, &options, &local_err);
+    if (local_err) {
+        goto fail;
+    }
+    visit_complete(v, opts);
+
+    qmp_blockdev_add(options, &local_err);
+    if (local_err) {
+        goto fail;
+    }
+
+fail:
+    QDECREF(opts_dict);
+    qobject_decref(crumpled);
+
+    visit_free(v);
+
+    v = qapi_dealloc_visitor_new();
+    visit_type_BlockdevOptions(v, NULL, &options, NULL);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+    return !!local_err;
+}
+
 static int drive_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     BlockInterfaceType *block_default_type = opaque;
@@ -2990,6 +3033,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_QAPI);
 
     qemu_add_opts(&qemu_drive_opts);
+    qemu_add_opts(&qemu_blockdev_opts);
     qemu_add_drive_opts(&qemu_legacy_drive_opts);
     qemu_add_drive_opts(&qemu_common_drive_opts);
     qemu_add_drive_opts(&qemu_drive_opts);
@@ -3122,6 +3166,13 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_blockdev:
+                opts = qemu_opts_parse_noisily(qemu_find_opts("blockdev"),
+                                               optarg, false);
+                if (opts == NULL) {
+                    exit(1);
+                }
+                break;
             case QEMU_OPTION_set:
                 if (qemu_set_option(optarg) != 0)
                     exit(1);
@@ -4409,6 +4460,12 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* open the virtual block devices */
+    if (qemu_opts_foreach(qemu_find_opts("blockdev"), blockdev_init_func,
+                          NULL, &err)) {
+        error_report_err(err);
+        exit(1);
+    }
+
     if (snapshot || replay_mode != REPLAY_MODE_NONE) {
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
                           NULL, NULL);
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] doc: Document generic -blockdev options

[Kevin Wolf]

This adds documentation for the -blockdev options that apply to all
nodes independent of the block driver used.

All options that are shared by -blockdev and -drive are now explained in
the section for -blockdev. The documentation of -drive mentions that all
-blockdev options are accepted as well.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-options.hx | 98 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 69 insertions(+), 29 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 542c483..8766589 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -523,6 +523,43 @@ STEXI
 @findex -blockdev
 
 Define a new block driver node.
+
+@table @option
+@item Valid options for any block driver node:
+
+@table @code
+@item driver
+Specifies the block driver to use for the given node.
+@item node-name
+This defines the name of the block driver node by which it will be referenced
+later. The name must be unique, i.e. it must not match the name of a different
+block driver node, or (if you use @option{-drive} as well) the ID of a drive.
+@item read-only
+Open the node read-only. Guest write attempts will fail.
+@item cache.direct
+The host page cache can be avoided with @option{cache.direct=on}. This will
+attempt to do disk IO directly to the guest's memory. QEMU may still perform an
+internal copy of the data.
+@item cache.no-flush
+In case you don't care about data integrity over host failures, use
+@option{cache.no-flush=on}. This option tells QEMU that it never needs to write
+any data to the disk but can instead keep things in cache. If anything goes
+wrong, like your host losing power, the disk storage getting disconnected
+accidentally, etc. your image will most probably be rendered unusable.
+@item discard=@var{discard}
+@var{discard} is one of "ignore" (or "off") or "unmap" (or "on") and controls
+whether @dfn{discard} (also known as @dfn{trim} or @dfn{unmap}) requests are
+ignored or passed to the filesystem.  Some machine types may not support
+discard requests.
+@item detect-zeroes=@var{detect-zeroes}
+@var{detect-zeroes} is "off", "on" or "unmap" and enables the automatic
+conversion of plain zero writes by the OS to driver specific optimized
+zero write commands. You may even choose "unmap" if @var{discard} is set
+to "unmap" to allow a zero write to be converted to an UNMAP operation.
+@end table
+
+@end table
+
 ETEXI
 
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
@@ -544,7 +581,12 @@ STEXI
 @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
 @findex -drive
 
-Define a new drive. Valid options are:
+Define a new drive. This includes creating a block driver node (the backend) as
+well as a guest device, and is mostly a shortcut for defining the corresponding
+@option{-blockdev} and @option{-device} options.
+
+@option{-drive} accepts all options that are accepted by @option{-blockdev}. In
+addition, it knows the following options:
 
 @table @option
 @item file=@var{file}
@@ -571,11 +613,31 @@ These options have the same definition as they have in @option{-hdachs}.
 @var{snapshot} is "on" or "off" and controls snapshot mode for the given drive
 (see @option{-snapshot}).
 @item cache=@var{cache}
-@var{cache} is "none", "writeback", "unsafe", "directsync" or "writethrough" and controls how the host cache is used to access block data.
+@var{cache} is "none", "writeback", "unsafe", "directsync" or "writethrough"
+and controls how the host cache is used to access block data. This is a
+shortcut that sets the @option{cache.direct} and @option{cache.no-flush}
+options (as in @option{-blockdev}), and additionally @option{cache.writeback},
+which provides a default for the @option{write-cache} option of block guest
+devices (as in @option{-device}). The modes correspond to the following
+settings:
+
+@c Our texi2pod.pl script doesn't support @multitable, so fall back to using
+@c plain ASCII art (well, UTF-8 art really). This looks okay both in the manpage
+@c and the HTML output.
+@example
+@             │ cache.writeback   cache.direct   cache.no-flush
+─────────────┼─────────────────────────────────────────────────
+writeback    │ on                off            off
+none         │ on                on             off
+writethrough │ off               off            off
+directsync   │ off               on             off
+unsafe       │ on                off            on
+@end example
+
+The default mode is @option{cache=writeback}.
+
 @item aio=@var{aio}
 @var{aio} is "threads", or "native" and selects between pthread based disk I/O and native Linux AIO.
-@item discard=@var{discard}
-@var{discard} is one of "ignore" (or "off") or "unmap" (or "on") and controls whether @dfn{discard} (also known as @dfn{trim} or @dfn{unmap}) requests are ignored or passed to the filesystem.  Some machine types may not support discard requests.
 @item format=@var{format}
 Specify which disk @var{format} will be used rather than detecting
 the format.  Can be used to specify format=raw to avoid interpreting
@@ -590,46 +652,24 @@ Specify which @var{action} to take on write and read errors. Valid actions are:
 "report" (report the error to the guest), "enospc" (pause QEMU only if the
 host disk is full; report the error to the guest otherwise).
 The default setting is @option{werror=enospc} and @option{rerror=report}.
-@item readonly
-Open drive @option{file} as read-only. Guest write attempts will fail.
 @item copy-on-read=@var{copy-on-read}
 @var{copy-on-read} is "on" or "off" and enables whether to copy read backing
 file sectors into the image file.
-@item detect-zeroes=@var{detect-zeroes}
-@var{detect-zeroes} is "off", "on" or "unmap" and enables the automatic
-conversion of plain zero writes by the OS to driver specific optimized
-zero write commands. You may even choose "unmap" if @var{discard} is set
-to "unmap" to allow a zero write to be converted to an UNMAP operation.
 @end table
 
-By default, the @option{cache=writeback} mode is used. It will report data
+By default, the @option{cache.writeback=on} mode is used. It will report data
 writes as completed as soon as the data is present in the host page cache.
 This is safe as long as your guest OS makes sure to correctly flush disk caches
 where needed. If your guest OS does not handle volatile disk write caches
 correctly and your host crashes or loses power, then the guest may experience
 data corruption.
 
-For such guests, you should consider using @option{cache=writethrough}. This
+For such guests, you should consider using @option{cache.writeback=off}. This
 means that the host page cache will be used to read and write data, but write
 notification will be sent to the guest only after QEMU has made sure to flush
 each write to the disk. Be aware that this has a major impact on performance.
 
-The host page cache can be avoided entirely with @option{cache=none}.  This will
-attempt to do disk IO directly to the guest's memory.  QEMU may still perform
-an internal copy of the data. Note that this is considered a writeback mode and
-the guest OS must handle the disk write cache correctly in order to avoid data
-corruption on host crashes.
-
-The host page cache can be avoided while only sending write notifications to
-the guest when the data has been flushed to the disk using
-@option{cache=directsync}.
-
-In case you don't care about data integrity over host failures, use
-@option{cache=unsafe}. This option tells QEMU that it never needs to write any
-data to the disk but can instead keep things in cache. If anything goes wrong,
-like your host losing power, the disk storage getting disconnected accidentally,
-etc. your image will most probably be rendered unusable.   When using
-the @option{-snapshot} option, unsafe caching is always used.
+When using the @option{-snapshot} option, unsafe caching is always used.
 
 Copy-on-read avoids accessing the same backing file sectors repeatedly and is
 useful when the backing file is over a slow network.  By default copy-on-read
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] doc: Document driver-specific -blockdev options

[Kevin Wolf]

This documents the driver-specific options for the raw, qcow2 and file
block drivers for the man page. For everything else, we refer to the
QAPI documentation.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-options.hx | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 8766589..bbe0d52 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -558,6 +558,68 @@ zero write commands. You may even choose "unmap" if @var{discard} is set
 to "unmap" to allow a zero write to be converted to an UNMAP operation.
 @end table
 
+@item Driver-specific options for @code{file}
+@table @code
+@item filename
+The path to the image file in the local filesystem
+@item aio
+Specifies the AIO backend (threads/native, default: threads)
+@end table
+
+@item Driver-specific options for @code{raw}
+@table @code
+@item file
+Reference to or definition of the data source block device
+@end table
+
+@item Driver-specific options for @code{qcow2}
+@table @code
+@item file
+Reference to or definition of the data source block device
+
+@item backing
+Reference to or definition of the backing file block device (if missing, taken
+from the image file content). It is allowed to pass an empty string here in
+order to disable the default backing file.
+
+@item lazy-refcounts
+Whether to enable the lazy refcounts feature (on/off, default is taken from the
+image file)
+
+@item cache-size
+The maximum total size of the L2 table and refcount block caches in bytes
+
+@item l2-cache-size
+The maximum size of the L2 table cache in bytes
+
+@item refcount-cache-size
+The maximum size of the refcount block cache in bytes
+
+@item cache-clean-interval
+Clean unused entries in the L2 and refcount caches. The interval is in seconds.
+The default value is 0 and it disables this feature.
+
+@item pass-discard-request
+Whether discard requests to the qcow2 device should be forwarded to the data
+source (on/off)
+
+@item pass-discard-snapshot
+Whether discard requests for the data source should be issued when a snapshot
+operation (e.g. deleting a snapshot) frees clusters in the qcow2 file (on/off)
+
+@item pass-discard-other
+Whether discard requests for the data source should be issued on other
+occasions where a cluster gets freed (on/off)
+
+@item overlap-check
+Which overlap checks to perform for writes to the image
+(none/constant/cache/all, defaults to 'cached'). For details or finer
+granularity control refer to the QAPI documentation of @code{blockdev-add}.
+@end table
+
+@item Driver-specific options for other drivers
+Please refer to the QAPI documentation of the @code{blockdev-add} QMP command.
+
 @end table
 
 ETEXI
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/3] Add -blockdev command line option

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1474558931-8968-1-git-send-email-kwolf@redhat.com
Subject: [Qemu-devel] [PATCH 0/3] Add -blockdev command line option

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/147455590865.8519.11191009507297313736.stgit@brijesh-build-machine -> patchew/147455590865.8519.11191009507297313736.stgit@brijesh-build-machine
 * [new tag]         patchew/1474558931-8968-1-git-send-email-kwolf@redhat.com -> patchew/1474558931-8968-1-git-send-email-kwolf@redhat.com
Switched to a new branch 'test'
90e69cc doc: Document driver-specific -blockdev options
0b26966 doc: Document generic -blockdev options
abd4ca2 block: Add '-blockdev' command line option

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix    /tmp/qemu-test/src/tests/docker/install
BIOS directory    /tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory  /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory  /tmp/qemu-test/src/tests/docker/install/etc
local state directory   /tmp/qemu-test/src/tests/docker/install/var
Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path       /tmp/qemu-test/src
C compiler        cc
Host C compiler   cc
C++ compiler      
Objective-C compiler cc
ARFLAGS           rv
CFLAGS            -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
QEMU_CFLAGS       -I/usr/include/pixman-1    -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS           -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make              make
install           install
python            python -B
smbd              /usr/sbin/smbd
module support    no
host CPU          x86_64
host big endian   no
target list       x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled     no
sparse enabled    no
strip binaries    yes
profiler          no
static build      no
pixman            system
SDL support       yes (1.2.14)
GTK support       no 
GTK GL support    no
VTE support       no 
TLS priority      NORMAL
GNUTLS support    no
GNUTLS rnd        no
libgcrypt         no
libgcrypt kdf     no
nettle            no 
nettle kdf        no
libtasn1          no
curses support    no
virgl support     no
curl support      no
mingw32 support   no
Audio drivers     oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support    no
VNC support       yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support       no
brlapi support    no
bluez  support    no
Documentation     no
PIE               yes
vde support       no
netmap support    no
Linux AIO support no
ATTR/XATTR support yes
Install blobs     yes
KVM support       yes
RDMA support      no
TCG interpreter   no
fdt support       yes
preadv support    yes
fdatasync         yes
madvise           yes
posix_madvise     yes
uuid support      no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backends    log
spice support     no 
rbd support       no
xfsctl support    no
smartcard support no
libusb            no
usb net redir     no
OpenGL support    no
OpenGL dmabufs    no
libiscsi support  no
libnfs support    no
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine pool    yes
GlusterFS support no
Archipelago support no
gcov              gcov
gcov enabled      no
TPM support       yes
libssh2 support   no
TPM passthrough   yes
QOM debugging     yes
vhdx              no
lzo support       no
snappy support    no
bzip2 support     no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
  GEN   x86_64-softmmu/config-devices.mak.tmp
  GEN   aarch64-softmmu/config-devices.mak.tmp
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
  GEN   qapi-event.h
  GEN   x86_64-softmmu/config-devices.mak
  GEN   aarch64-softmmu/config-devices.mak
  GEN   qmp-introspect.h
  GEN   module_block.h
  GEN   tests/test-qapi-types.h
  GEN   tests/test-qapi-visit.h
  GEN   tests/test-qmp-commands.h
  GEN   tests/test-qapi-event.h
  GEN   tests/test-qmp-introspect.h
  GEN   config-all-devices.mak
  GEN   trace/generated-events.h
  GEN   trace/generated-tracers.h
  GEN   trace/generated-tcg-tracers.h
  GEN   trace/generated-helpers-wrappers.h
  GEN   trace/generated-helpers.h
  CC    tests/qemu-iotests/socket_scm_helper.o
  GEN   qga/qapi-generated/qga-qapi-types.h
  GEN   qga/qapi-generated/qga-qapi-visit.h
  GEN   qga/qapi-generated/qga-qmp-commands.h
  GEN   qga/qapi-generated/qga-qapi-types.c
  GEN   qga/qapi-generated/qga-qapi-visit.c
  GEN   qga/qapi-generated/qga-qmp-marshal.c
  GEN   qmp-introspect.c
  GEN   qapi-types.c
  GEN   qapi-visit.c
  GEN   qapi-event.c
  CC    qapi/qapi-visit-core.o
  CC    qapi/qapi-dealloc-visitor.o
  CC    qapi/qmp-input-visitor.o
  CC    qapi/qmp-output-visitor.o
  CC    qapi/qmp-registry.o
  CC    qapi/qmp-dispatch.o
  CC    qapi/string-input-visitor.o
  CC    qapi/string-output-visitor.o
  CC    qapi/opts-visitor.o
  CC    qapi/qapi-clone-visitor.o
  CC    qapi/qmp-event.o
  CC    qapi/qapi-util.o
  CC    qobject/qnull.o
  CC    qobject/qint.o
  CC    qobject/qstring.o
  CC    qobject/qdict.o
  CC    qobject/qlist.o
  CC    qobject/qfloat.o
  CC    qobject/qbool.o
  CC    qobject/qjson.o
  CC    qobject/qobject.o
  CC    qobject/json-lexer.o
  CC    qobject/json-streamer.o
  CC    qobject/json-parser.o
  GEN   trace/generated-events.c
  CC    trace/control.o
  CC    trace/qmp.o
  CC    util/osdep.o
  CC    util/cutils.o
  CC    util/unicode.o
  CC    util/qemu-timer-common.o
  CC    util/bufferiszero.o
  CC    util/compatfd.o
  CC    util/event_notifier-posix.o
  CC    util/mmap-alloc.o
  CC    util/oslib-posix.o
  CC    util/qemu-openpty.o
  CC    util/qemu-thread-posix.o
  CC    util/memfd.o
  CC    util/envlist.o
  CC    util/path.o
  CC    util/module.o
  CC    util/bitmap.o
  CC    util/bitops.o
  CC    util/hbitmap.o
  CC    util/fifo8.o
  CC    util/acl.o
  CC    util/error.o
  CC    util/qemu-error.o
  CC    util/id.o
  CC    util/iov.o
  CC    util/qemu-config.o
  CC    util/qemu-sockets.o
  CC    util/uri.o
  CC    util/notify.o
  CC    util/qemu-option.o
  CC    util/qemu-progress.o
  CC    util/hexdump.o
  CC    util/crc32c.o
  CC    util/throttle.o
  CC    util/getauxval.o
  CC    util/readline.o
  CC    util/rfifolock.o
  CC    util/rcu.o
  CC    util/qemu-coroutine.o
  CC    util/qemu-coroutine-lock.o
  CC    util/qemu-coroutine-io.o
  CC    util/qemu-coroutine-sleep.o
  CC    util/coroutine-ucontext.o
  CC    util/buffer.o
  CC    util/timed-average.o
  CC    util/base64.o
  CC    util/log.o
  CC    util/qdist.o
  CC    util/qht.o
  CC    util/range.o
  CC    crypto/pbkdf-stub.o
  CC    stubs/arch-query-cpu-def.o
  CC    stubs/arch-query-cpu-model-expansion.o
  CC    stubs/arch-query-cpu-model-comparison.o
  CC    stubs/arch-query-cpu-model-baseline.o
/tmp/qemu-test/src/util/qht.c: In function ‘qht_reset_size’:
/tmp/qemu-test/src/util/qht.c:413: warning: ‘new’ may be used uninitialized in this function
  CC    stubs/bdrv-next-monitor-owned.o
  CC    stubs/blk-commit-all.o
  CC    stubs/blockdev-close-all-bdrv-states.o
  CC    stubs/clock-warp.o
  CC    stubs/cpu-get-clock.o
  CC    stubs/cpu-get-icount.o
  CC    stubs/dump.o
  CC    stubs/fdset-add-fd.o
  CC    stubs/fdset-find-fd.o
  CC    stubs/fdset-get-fd.o
  CC    stubs/fdset-remove-fd.o
  CC    stubs/gdbstub.o
  CC    stubs/get-fd.o
  CC    stubs/get-next-serial.o
  CC    stubs/get-vm-name.o
  CC    stubs/iothread-lock.o
  CC    stubs/is-daemonized.o
  CC    stubs/machine-init-done.o
  CC    stubs/migr-blocker.o
  CC    stubs/mon-is-qmp.o
  CC    stubs/mon-printf.o
  CC    stubs/monitor-init.o
  CC    stubs/qtest.o
  CC    stubs/notify-event.o
  CC    stubs/replay.o
  CC    stubs/replay-user.o
  CC    stubs/reset.o
  CC    stubs/runstate-check.o
  CC    stubs/set-fd-handler.o
  CC    stubs/slirp.o
  CC    stubs/sysbus.o
  CC    stubs/trace-control.o
  CC    stubs/uuid.o
  CC    stubs/vm-stop.o
  CC    stubs/vmstate.o
  CC    stubs/cpus.o
  CC    stubs/kvm.o
  CC    stubs/qmp_pc_dimm_device_list.o
  CC    stubs/target-monitor-defs.o
  CC    stubs/target-get-monitor-def.o
  CC    stubs/vhost.o
  CC    stubs/iohandler.o
  CC    stubs/smbios_type_38.o
  CC    stubs/ipmi.o
  CC    stubs/pc_madt_cpu_entry.o
  CC    contrib/ivshmem-client/ivshmem-client.o
  CC    contrib/ivshmem-client/main.o
  CC    contrib/ivshmem-server/ivshmem-server.o
  CC    contrib/ivshmem-server/main.o
  CC    qemu-nbd.o
  CC    async.o
  CC    thread-pool.o
  CC    block.o
  CC    blockjob.o
  CC    main-loop.o
  CC    iohandler.o
  CC    qemu-timer.o
  CC    aio-posix.o
  CC    qemu-io-cmds.o
  CC    replication.o
  CC    block/raw_bsd.o
  CC    block/qcow.o
  CC    block/vdi.o
  CC    block/vmdk.o
  CC    block/cloop.o
  CC    block/bochs.o
  CC    block/vpc.o
  CC    block/vvfat.o
  CC    block/dmg.o
  CC    block/qcow2.o
  CC    block/qcow2-refcount.o
  CC    block/qcow2-cluster.o
  CC    block/qcow2-snapshot.o
  CC    block/qcow2-cache.o
  CC    block/qed.o
  CC    block/qed-gencb.o
  CC    block/qed-l2-cache.o
  CC    block/qed-table.o
  CC    block/qed-cluster.o
  CC    block/qed-check.o
  CC    block/quorum.o
  CC    block/parallels.o
  CC    block/blkdebug.o
  CC    block/blkverify.o
  CC    block/blkreplay.o
  CC    block/block-backend.o
  CC    block/snapshot.o
  CC    block/qapi.o
  CC    block/raw-posix.o
  CC    block/null.o
  CC    block/mirror.o
  CC    block/commit.o
  CC    block/io.o
  CC    block/throttle-groups.o
  CC    block/nbd.o
  CC    block/nbd-client.o
  CC    block/sheepdog.o
  CC    block/accounting.o
  CC    block/dirty-bitmap.o
  CC    block/write-threshold.o
  CC    block/backup.o
  CC    block/replication.o
  CC    block/crypto.o
  CC    nbd/server.o
  CC    nbd/client.o
  CC    nbd/common.o
  CC    crypto/init.o
  CC    crypto/hash.o
  CC    crypto/hash-glib.o
  CC    crypto/aes.o
  CC    crypto/desrfb.o
  CC    crypto/cipher.o
  CC    crypto/tlscreds.o
  CC    crypto/tlscredsanon.o
  CC    crypto/tlscredsx509.o
  CC    crypto/tlssession.o
  CC    crypto/secret.o
  CC    crypto/random-platform.o
  CC    crypto/pbkdf.o
  CC    crypto/ivgen-essiv.o
  CC    crypto/ivgen.o
  CC    crypto/ivgen-plain.o
  CC    crypto/ivgen-plain64.o
  CC    crypto/afsplit.o
  CC    crypto/xts.o
  CC    crypto/block-qcow.o
  CC    crypto/block.o
  CC    crypto/block-luks.o
  CC    io/channel.o
  CC    io/channel-buffer.o
  CC    io/channel-command.o
  CC    io/channel-file.o
  CC    io/channel-socket.o
  CC    io/channel-tls.o
  CC    io/channel-watch.o
  CC    io/channel-websock.o
  CC    io/channel-util.o
  CC    io/task.o
  CC    qom/object.o
  CC    qom/container.o
  CC    qom/qom-qobject.o
  CC    qom/object_interfaces.o
  GEN   qemu-img-cmds.h
  CC    qemu-io.o
  CC    qemu-bridge-helper.o
  CC    blockdev.o
  CC    blockdev-nbd.o
  CC    iothread.o
  CC    qdev-monitor.o
  CC    device-hotplug.o
  CC    os-posix.o
  CC    qemu-char.o
  CC    page_cache.o
  CC    accel.o
  CC    bt-host.o
  CC    bt-vhci.o
  CC    dma-helpers.o
  CC    vl.o
  CC    tpm.o
  CC    device_tree.o
  GEN   qmp-marshal.c
  CC    qmp.o
  CC    hmp.o
  CC    tcg-runtime.o
  CC    audio/audio.o
  CC    audio/noaudio.o
  CC    audio/wavaudio.o
  CC    audio/mixeng.o
  CC    audio/sdlaudio.o
  CC    audio/ossaudio.o
  CC    audio/wavcapture.o
  CC    backends/rng.o
  CC    backends/rng-egd.o
  CC    backends/rng-random.o
  CC    backends/msmouse.o
  CC    backends/testdev.o
  CC    backends/tpm.o
  CC    backends/hostmem.o
  CC    backends/hostmem-ram.o
/tmp/qemu-test/src/vl.c:126:40: error: qapi/qobject-input-visitor.h: No such file or directory
  CC    backends/hostmem-file.o
/tmp/qemu-test/src/vl.c: In function ‘blockdev_init_func’:
/tmp/qemu-test/src/vl.c:1182: warning: implicit declaration of function ‘qdict_crumple’
/tmp/qemu-test/src/vl.c:1182: warning: nested extern declaration of ‘qdict_crumple’
/tmp/qemu-test/src/vl.c:1182: warning: initialization makes pointer from integer without a cast
/tmp/qemu-test/src/vl.c:1187: warning: implicit declaration of function ‘qobject_string_input_visitor_new’
/tmp/qemu-test/src/vl.c:1187: warning: nested extern declaration of ‘qobject_string_input_visitor_new’
/tmp/qemu-test/src/vl.c:1187: warning: assignment makes pointer from integer without a cast
make: *** [vl.o] Error 1
make: *** Waiting for unfinished jobs....
tests/docker/Makefile.include:107: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 1
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 1/3] block: Add '-blockdev' command line option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2443 bytes --]

On 09/22/2016 10:42 AM, Kevin Wolf wrote:
> This is an option that is directly mapped to the blockdev-add QMP
> command. It works more or less like -drive, except that it doesn't
> create a BlockBackend and doesn't support legacy options.
> 
> This patch adds minimal documentation, the next patches will improve it.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c              | 12 +++++++++++
>  include/sysemu/sysemu.h |  1 +
>  qemu-options.hx         | 12 +++++++++++
>  vl.c                    | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 82 insertions(+)
> 

> +static int blockdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
> +{
> +    BlockdevOptions *options;

Uninitialized...

> +    Visitor *v = NULL;
> +    Error *local_err = NULL;
> +
> +    QDict *opts_dict = qemu_opts_to_qdict(opts, NULL);
> +    QObject *crumpled = qdict_crumple(opts_dict, true, &local_err);
> +    if (local_err) {
> +        goto fail;

...can fail without initializing it...

> +    }
> +
> +    v = qobject_string_input_visitor_new(crumpled);
> +    visit_type_BlockdevOptions(v, NULL, &options, &local_err);

This is so deceptively simple! It's taken us months to get to this
point, but I love the end result.  However,

...this initializes options, and may result in malloc'd memory...

> +    if (local_err) {
> +        goto fail;
> +    }
> +    visit_complete(v, opts);
> +
> +    qmp_blockdev_add(options, &local_err);
> +    if (local_err) {
> +        goto fail;
> +    }
> +
> +fail:
> +    QDECREF(opts_dict);
> +    qobject_decref(crumpled);
> +
> +    visit_free(v);
> +
> +    v = qapi_dealloc_visitor_new();
> +    visit_type_BlockdevOptions(v, NULL, &options, NULL);

...this can call the dealloc visitor on uninitialized memory, if we took
the first goto...

> +    visit_free(v);
> +
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +    }
> +    return !!local_err;

...and this leaks options if you did not take the first goto.

You want NULL initialization, and a qapi_free_BlockdevOptions(options)
in here.

> +}
> +
>  static int drive_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      BlockInterfaceType *block_default_type = opaque;

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/3] doc: Document generic -blockdev options

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]

On 09/22/2016 10:42 AM, Kevin Wolf wrote:
> This adds documentation for the -blockdev options that apply to all
> nodes independent of the block driver used.
> 
> All options that are shared by -blockdev and -drive are now explained in
> the section for -blockdev. The documentation of -drive mentions that all
> -blockdev options are accepted as well.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qemu-options.hx | 98 ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 69 insertions(+), 29 deletions(-)
> 

>  @item cache=@var{cache}
> -@var{cache} is "none", "writeback", "unsafe", "directsync" or "writethrough" and controls how the host cache is used to access block data.
> +@var{cache} is "none", "writeback", "unsafe", "directsync" or "writethrough"
> +and controls how the host cache is used to access block data. This is a
> +shortcut that sets the @option{cache.direct} and @option{cache.no-flush}
> +options (as in @option{-blockdev}), and additionally @option{cache.writeback},
> +which provides a default for the @option{write-cache} option of block guest
> +devices (as in @option{-device}). The modes correspond to the following
> +settings:
> +
> +@c Our texi2pod.pl script doesn't support @multitable, so fall back to using
> +@c plain ASCII art (well, UTF-8 art really). This looks okay both in the manpage
> +@c and the HTML output.
> +@example
> +@             │ cache.writeback   cache.direct   cache.no-flush
> +─────────────┼─────────────────────────────────────────────────
> +writeback    │ on                off            off
> +none         │ on                on             off
> +writethrough │ off               off            off
> +directsync   │ off               on             off
> +unsafe       │ on                off            on
> +@end example

Nice table.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/3] Add -blockdev command line option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On 09/22/2016 12:12 PM,
no-reply@ec2-52-6-146-230.compute-1.amazonaws.com wrote:
> Hi,
> 
> Your series failed automatic build test. Please find the testing commands and
> their output below. If you have docker installed, you can probably reproduce it
> locally.
> 
> Type: series
> Message-id: 1474558931-8968-1-git-send-email-kwolf@redhat.com
> Subject: [Qemu-devel] [PATCH 0/3] Add -blockdev command line option
> 

>   CC    backends/hostmem-ram.o
> /tmp/qemu-test/src/vl.c:126:40: error: qapi/qobject-input-visitor.h: No such file or directory

Because these patches are dependent on an as-yet unpushed series,
mentioned in the cover letter but not in a manner patchew understands
(well, that patchew may understand in the future...)

https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg05128.html


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/3] doc: Document driver-specific -blockdev options

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]

On 09/22/2016 10:42 AM, Kevin Wolf wrote:
> This documents the driver-specific options for the raw, qcow2 and file
> block drivers for the man page. For everything else, we refer to the
> QAPI documentation.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qemu-options.hx | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 

>  
> +@item Driver-specific options for @code{file}
> +@table @code
> +@item filename
> +The path to the image file in the local filesystem
> +@item aio
> +Specifies the AIO backend (threads/native, default: threads)
> +@end table
> +
> +@item Driver-specific options for @code{raw}
> +@table @code
> +@item file
> +Reference to or definition of the data source block device
> +@end table

Would an example command line make things any easier to read, for giving
hints on how many dotted names are needed as you describe further
nesting levels in relation to the top level?

> +@item cache-clean-interval
> +Clean unused entries in the L2 and refcount caches. The interval is in seconds.
> +The default value is 0 and it disables this feature.
> +
> +@item pass-discard-request
> +Whether discard requests to the qcow2 device should be forwarded to the data
> +source (on/off)
> +
> +@item pass-discard-snapshot
> +Whether discard requests for the data source should be issued when a snapshot
> +operation (e.g. deleting a snapshot) frees clusters in the qcow2 file (on/off)
> +
> +@item pass-discard-other
> +Whether discard requests for the data source should be issued on other
> +occasions where a cluster gets freed (on/off)

I like that cache-clean-interval mentioned a default; should these other
lines do likewise?

At any rate, those ideas can be followups if desired, as this patch is
strictly an improvement.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/3] block: Add '-blockdev' command line option

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 2797 bytes --]

Am 22.09.2016 um 20:45 hat Eric Blake geschrieben:
> On 09/22/2016 10:42 AM, Kevin Wolf wrote:
> > This is an option that is directly mapped to the blockdev-add QMP
> > command. It works more or less like -drive, except that it doesn't
> > create a BlockBackend and doesn't support legacy options.
> > 
> > This patch adds minimal documentation, the next patches will improve it.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  blockdev.c              | 12 +++++++++++
> >  include/sysemu/sysemu.h |  1 +
> >  qemu-options.hx         | 12 +++++++++++
> >  vl.c                    | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 82 insertions(+)
> > 
> 
> > +static int blockdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > +{
> > +    BlockdevOptions *options;
> 
> Uninitialized...

Oops, good catch. Thanks.

> > +    Visitor *v = NULL;
> > +    Error *local_err = NULL;
> > +
> > +    QDict *opts_dict = qemu_opts_to_qdict(opts, NULL);
> > +    QObject *crumpled = qdict_crumple(opts_dict, true, &local_err);
> > +    if (local_err) {
> > +        goto fail;
> 
> ...can fail without initializing it...
> 
> > +    }
> > +
> > +    v = qobject_string_input_visitor_new(crumpled);
> > +    visit_type_BlockdevOptions(v, NULL, &options, &local_err);
> 
> This is so deceptively simple! It's taken us months to get to this
> point, but I love the end result.  However,
> 
> ...this initializes options, and may result in malloc'd memory...
> 
> > +    if (local_err) {
> > +        goto fail;
> > +    }
> > +    visit_complete(v, opts);
> > +
> > +    qmp_blockdev_add(options, &local_err);
> > +    if (local_err) {
> > +        goto fail;
> > +    }
> > +
> > +fail:
> > +    QDECREF(opts_dict);
> > +    qobject_decref(crumpled);
> > +
> > +    visit_free(v);
> > +
> > +    v = qapi_dealloc_visitor_new();
> > +    visit_type_BlockdevOptions(v, NULL, &options, NULL);
> 
> ...this can call the dealloc visitor on uninitialized memory, if we took
> the first goto...
> 
> > +    visit_free(v);
> > +
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +    }
> > +    return !!local_err;
> 
> ...and this leaks options if you did not take the first goto.
> 
> You want NULL initialization, and a qapi_free_BlockdevOptions(options)
> in here.

Yes, I'll add the NULL initialisation.

I don't think you're right about the leak, the dealloc visitor takes
care of deallocating the top level object as well. However, it's true
that qapi_free_BlockdevOptions() already has the dealloc visitor code
internally, so instead of open-coding it here, I can replace (rather
than complement) the above code with a call to it.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH 3/3] doc: Document driver-specific -blockdev options

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

Am 22.09.2016 um 21:02 hat Eric Blake geschrieben:
> On 09/22/2016 10:42 AM, Kevin Wolf wrote:
> > This documents the driver-specific options for the raw, qcow2 and file
> > block drivers for the man page. For everything else, we refer to the
> > QAPI documentation.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  qemu-options.hx | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> 
> >  
> > +@item Driver-specific options for @code{file}
> > +@table @code
> > +@item filename
> > +The path to the image file in the local filesystem
> > +@item aio
> > +Specifies the AIO backend (threads/native, default: threads)
> > +@end table
> > +
> > +@item Driver-specific options for @code{raw}
> > +@table @code
> > +@item file
> > +Reference to or definition of the data source block device
> > +@end table
> 
> Would an example command line make things any easier to read, for giving
> hints on how many dotted names are needed as you describe further
> nesting levels in relation to the top level?

Hm, I guess I could add an example command line to each driver section.
And probably the dotted syntax should be mentioned somewhere, too.

> > +@item cache-clean-interval
> > +Clean unused entries in the L2 and refcount caches. The interval is in seconds.
> > +The default value is 0 and it disables this feature.
> > +
> > +@item pass-discard-request
> > +Whether discard requests to the qcow2 device should be forwarded to the data
> > +source (on/off)
> > +
> > +@item pass-discard-snapshot
> > +Whether discard requests for the data source should be issued when a snapshot
> > +operation (e.g. deleting a snapshot) frees clusters in the qcow2 file (on/off)
> > +
> > +@item pass-discard-other
> > +Whether discard requests for the data source should be issued on other
> > +occasions where a cluster gets freed (on/off)
> 
> I like that cache-clean-interval mentioned a default; should these other
> lines do likewise?

Actually I took most of these descriptions straight from the QAPI schema
comments, except that I added the allowed values sometimes. If the
default is missing, we should probably add it in both places.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH 1/3] block: Add '-blockdev' command line option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

On 09/23/2016 04:37 AM, Kevin Wolf wrote:
>>
>>> +static int blockdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
>>> +{
>>> +    BlockdevOptions *options;
>>
>> Uninitialized...
> 
> Oops, good catch. Thanks.
> 

>>> +    visit_free(v);
>>> +
>>> +    v = qapi_dealloc_visitor_new();
>>> +    visit_type_BlockdevOptions(v, NULL, &options, NULL);
>>

> 
> I don't think you're right about the leak, the dealloc visitor takes
> care of deallocating the top level object as well. However, it's true
> that qapi_free_BlockdevOptions() already has the dealloc visitor code
> internally, so instead of open-coding it here, I can replace (rather
> than complement) the above code with a call to it.

Indeed.  In fact, I've even documented that MOST callers should NOT be
using qapi_dealloc_visitor_new() directly.  So you are correct that the
real fix is to quit open-coding the destruction, and use the one-liner
qapi_free_* in its place.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]