* [Qemu-devel] [PATCH v2 01/17] qapi-types/visit.py: Pass whole expr dict for structs
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 02/17] qapi-types/visit.py: Inheritance " Kevin Wolf
` (16 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-types.py | 11 ++++++++---
scripts/qapi-visit.py | 8 ++++++--
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5222463..566fe5e 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -71,7 +71,7 @@ def generate_struct_fields(members):
c_name=c_var(argname))
if structured:
push_indent()
- ret += generate_struct("", argname, argentry)
+ ret += generate_struct({ "field": argname, "data": argentry})
pop_indent()
else:
ret += mcgen('''
@@ -81,7 +81,12 @@ def generate_struct_fields(members):
return ret
-def generate_struct(structname, fieldname, members):
+def generate_struct(expr):
+
+ structname = expr.get('type', "")
+ fieldname = expr.get('field', "")
+ members = expr['data']
+
ret = mcgen('''
struct %(name)s
{
@@ -417,7 +422,7 @@ if do_builtins:
for expr in exprs:
ret = "\n"
if expr.has_key('type'):
- ret += generate_struct(expr['type'], "", expr['data']) + "\n"
+ ret += generate_struct(expr) + "\n"
ret += generate_type_cleanup_decl(expr['type'] + "List")
fdef.write(generate_type_cleanup(expr['type'] + "List") + "\n")
ret += generate_type_cleanup_decl(expr['type'])
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 597cca4..1e44004 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -120,7 +120,11 @@ if (!err) {
''')
return ret
-def generate_visit_struct(name, members):
+def generate_visit_struct(expr):
+
+ name = expr['type']
+ members = expr['data']
+
ret = generate_visit_struct_fields(name, "", "", members)
ret += mcgen('''
@@ -472,7 +476,7 @@ if do_builtins:
for expr in exprs:
if expr.has_key('type'):
- ret = generate_visit_struct(expr['type'], expr['data'])
+ ret = generate_visit_struct(expr)
ret += generate_visit_list(expr['type'], expr['data'])
fdef.write(ret)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [Qemu-devel] [PATCH v2 02/17] qapi-types/visit.py: Inheritance for structs
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 01/17] qapi-types/visit.py: Pass whole expr dict for structs Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 15:21 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 03/17] blockdev: Introduce DriveInfo.enable_auto_del Kevin Wolf
` (15 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
This introduces a new 'base' key for struct definitions that refers to
another struct type. On the JSON level, the fields of the base type are
included directly into the same namespace as the fields of the defined
type, like with unions. On the C level, a pointer to a struct of the
base type is included.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/qapi-code-gen.txt | 17 +++++++++++++++++
scripts/qapi-types.py | 4 ++++
scripts/qapi-visit.py | 18 ++++++++++++++++--
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0ce045c..91f44d0 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -53,6 +53,23 @@ The use of '*' as a prefix to the name means the member is optional. Optional
members should always be added to the end of the dictionary to preserve
backwards compatibility.
+
+A complex type definition can specify another complex type as its base.
+In this case, the fields of the base type are included as top-level fields
+of the new complex type's dictionary in the QMP wire format. An example
+definition is:
+
+ { 'type': 'BlockdevOptionsGenericFormat', 'data': { 'file': 'str' } }
+ { 'type': 'BlockdevOptionsGenericCOWFormat',
+ 'base': 'BlockdevOptionsGenericFormat',
+ 'data': { '*backing': 'str' } }
+
+An example BlockdevOptionsGenericCOWFormat object on the wire could use
+both fields like this:
+
+ { "file": "/some/place/my-image",
+ "backing": "/some/place/my-backing-file" }
+
=== Enumeration types ===
An enumeration type is a dictionary containing a single key whose value is a
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 566fe5e..4a1652b 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -86,6 +86,7 @@ def generate_struct(expr):
structname = expr.get('type', "")
fieldname = expr.get('field', "")
members = expr['data']
+ base = expr.get('base')
ret = mcgen('''
struct %(name)s
@@ -93,6 +94,9 @@ struct %(name)s
''',
name=structname)
+ if base:
+ ret += generate_struct_fields({'base': base})
+
ret += generate_struct_fields(members)
if len(fieldname):
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 1e44004..c39e628 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -17,7 +17,7 @@ import os
import getopt
import errno
-def generate_visit_struct_fields(name, field_prefix, fn_prefix, members):
+def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None):
substructs = []
ret = ''
full_name = name if not fn_prefix else "%s_%s" % (name, fn_prefix)
@@ -42,6 +42,19 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error *
name=name, full_name=full_name)
push_indent()
+ if base:
+ ret += mcgen('''
+visit_start_implicit_struct(m, obj ? (void**) &(*obj)->%(c_name)s : NULL, sizeof(%(type)s), &err);
+if (!err) {
+ visit_type_%(type)s_fields(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, &err);
+ error_propagate(errp, err);
+ err = NULL;
+ visit_end_implicit_struct(m, &err);
+}
+''',
+ c_prefix=c_var(field_prefix),
+ type=type_name(base), c_name=c_var('base'))
+
for argname, argentry, optional, structured in parse_args(members):
if optional:
ret += mcgen('''
@@ -124,8 +137,9 @@ def generate_visit_struct(expr):
name = expr['type']
members = expr['data']
+ base = expr.get('base')
- ret = generate_visit_struct_fields(name, "", "", members)
+ ret = generate_visit_struct_fields(name, "", "", members, base)
ret += mcgen('''
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/17] qapi-types/visit.py: Inheritance for structs
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 02/17] qapi-types/visit.py: Inheritance " Kevin Wolf
@ 2013-10-01 15:21 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 15:21 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 773 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> This introduces a new 'base' key for struct definitions that refers to
> another struct type. On the JSON level, the fields of the base type are
> included directly into the same namespace as the fields of the defined
> type, like with unions. On the C level, a pointer to a struct of the
> base type is included.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> docs/qapi-code-gen.txt | 17 +++++++++++++++++
> scripts/qapi-types.py | 4 ++++
> scripts/qapi-visit.py | 18 ++++++++++++++++--
> 3 files changed, 37 insertions(+), 2 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 03/17] blockdev: Introduce DriveInfo.enable_auto_del
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 01/17] qapi-types/visit.py: Pass whole expr dict for structs Kevin Wolf
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 02/17] qapi-types/visit.py: Inheritance " Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command Kevin Wolf
` (14 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
BlockDriverStates shouldn't be affected by an unplugged guest device,
except if created with the legacy -drive command line option or the
drive_add HMP command.
Make the automatic deletion as well as cancelling of jobs conditional on
an enable_auto_del boolean that is only set in drive_init().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
blockdev.c | 17 ++++++++++++++++-
include/sysemu/blockdev.h | 1 +
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 8aa66a9..29a5b70 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -89,6 +89,10 @@ void blockdev_mark_auto_del(BlockDriverState *bs)
{
DriveInfo *dinfo = drive_get_by_blockdev(bs);
+ if (dinfo && !dinfo->enable_auto_del) {
+ return;
+ }
+
if (bs->job) {
block_job_cancel(bs->job);
}
@@ -750,6 +754,7 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
{
const char *value;
+ DriveInfo *dinfo;
/* Change legacy command line options into QMP ones */
qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
@@ -798,7 +803,17 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
qemu_opt_unset(all_opts, "cache");
}
- return blockdev_init(all_opts, block_default_type);
+ /* Actual block device init: Functionality shared with blockdev-add */
+ dinfo = blockdev_init(all_opts, block_default_type);
+ if (dinfo == NULL) {
+ goto fail;
+ }
+
+ /* Set legacy DriveInfo fields */
+ dinfo->enable_auto_del = true;
+
+fail:
+ return dinfo;
}
void do_commit(Monitor *mon, const QDict *qdict)
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 804ec88..1082091 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -37,6 +37,7 @@ struct DriveInfo {
int bus;
int unit;
int auto_del; /* see blockdev_mark_auto_del() */
+ bool enable_auto_del; /* Only for legacy drive_init() */
int media_cd;
int cyls, heads, secs, trans;
QemuOpts *opts;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (2 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 03/17] blockdev: Introduce DriveInfo.enable_auto_del Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 14:00 ` Benoît Canet
2013-10-01 15:41 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 05/17] blockdev: Separate ID generation from DriveInfo creation Kevin Wolf
` (13 subsequent siblings)
17 siblings, 2 replies; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
For examples see the changes to qmp-commands.hx.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 57 ++++++++++++++
qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qmp-commands.hx | 59 ++++++++++++++
3 files changed, 352 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 29a5b70..3059987 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -38,6 +38,8 @@
#include "qemu/option.h"
#include "qemu/config-file.h"
#include "qapi/qmp/types.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
#include "sysemu/sysemu.h"
#include "block/block_int.h"
#include "qmp-commands.h"
@@ -2066,6 +2068,61 @@ void qmp_block_job_complete(const char *device, Error **errp)
block_job_complete(job, errp);
}
+void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
+{
+ QmpOutputVisitor *ov = qmp_output_visitor_new();
+ QObject *obj;
+ QDict *qdict;
+ DriveInfo *dinfo;
+ Error *local_err = NULL;
+
+ /* Require an ID in the top level */
+ if (!options->has_id) {
+ error_setg(errp, "Block device needs an ID");
+ return;
+ }
+
+ /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
+ * cache.direct=false instead of silently switching to aio=threads, except
+ * if called from drive_init.
+ *
+ * For now, simply forbidding the combination for all drivers will do. */
+ if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
+ bool direct = options->cache->has_direct && options->cache->direct;
+ if (!options->has_cache && !direct) {
+ error_setg(errp, "aio=native requires cache.direct=true");
+ goto fail;
+ }
+ }
+
+ visit_type_BlockdevOptions(qmp_output_get_visitor(ov),
+ &options, NULL, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ goto fail;
+ }
+
+ obj = qmp_output_get_qobject(ov);
+ qdict = qobject_to_qdict(obj);
+
+ qdict_flatten(qdict);
+
+ QemuOpts *opts = qemu_opts_from_qdict(&qemu_drive_opts, qdict, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ goto fail;
+ }
+
+ dinfo = blockdev_init(opts, IF_NONE);
+ if (!dinfo) {
+ error_setg(errp, "Could not open image");
+ goto fail;
+ }
+
+fail:
+ qmp_output_visitor_cleanup(ov);
+}
+
static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
{
BlockJobInfoList **prev = opaque;
diff --git a/qapi-schema.json b/qapi-schema.json
index 145eca8..7192119 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3902,3 +3902,239 @@
##
{ 'command': 'query-rx-filter', 'data': { '*name': 'str' },
'returns': ['RxFilterInfo'] }
+
+
+##
+# @BlockdevDiscardOptions
+#
+# Determines how to handle discard requests.
+#
+# @ignore: Ignore the request
+# @unmap: Forward as an unmap request
+#
+# Since: 1.7
+##
+{ 'enum': 'BlockdevDiscardOptions',
+ 'data': [ 'ignore', 'unmap' ] }
+
+##
+# @BlockdevAioOptions
+#
+# Selects the AIO backend to handle I/O requests
+#
+# @threads: Use qemu's thread pool
+# @native: Use native AIO backend (only Linux and Windows)
+#
+# Since: 1.7
+##
+{ 'enum': 'BlockdevAioOptions',
+ 'data': [ 'threads', 'native' ] }
+
+##
+# @BlockdevCacheOptions
+#
+# Includes cache-related options for block devices
+#
+# @writeback: #optional enables writeback mode for any caches (default: true)
+# @direct: #optional enables use of O_DIRECT (bypass the host page cache;
+# default: false)
+# @no-flush: #optional ignore any flush requests for the device (default:
+# false)
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevCacheOptions',
+ 'data': { '*writeback': 'bool',
+ '*direct': 'bool',
+ '*no-flush': 'bool' } }
+
+##
+# @BlockdevOptionsBase
+#
+# Options that are available for all block devices, independent of the block
+# driver.
+#
+# @driver: block driver name
+# @id: #optional id by which the new block device can be referred to.
+# This is a required option on the top level of blockdev-add, and
+# currently not allowed on any other level.
+# @discard: #optional discard-related options (default: ignore)
+# @cache: #optional cache-related options
+# @aio: #optional AIO backend (default: threads)
+# @rerror: #optional how to handle read errors on the device
+# (default: report)
+# @werror: #optional how to handle write errors on the device
+# (default: enospc)
+# @read-only: #optional whether the block device should be read-only
+# (default: false)
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevOptionsBase',
+ 'data': { 'driver': 'str',
+ '*id': 'str',
+ '*discard': 'BlockdevDiscardOptions',
+ '*cache': 'BlockdevCacheOptions',
+ '*aio': 'BlockdevAioOptions',
+ '*rerror': 'BlockdevOnError',
+ '*werror': 'BlockdevOnError',
+ '*read-only': 'bool' } }
+
+##
+# @BlockdevOptionsFile
+#
+# Driver specific block device options for the file backend and similar
+# protocols.
+#
+# @filename: path to the image file
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevOptionsFile',
+ 'data': { 'filename': 'str' } }
+
+##
+# @BlockdevOptionsVVFAT
+#
+# Driver specific block device options for the vvfat protocol.
+#
+# @dir: directory to be exported as FAT image
+# @fat-type: #optional FAT type: 12, 16 or 32
+# @floppy: #optional whether to export a floppy image (true) or
+# partitioned hard disk (false; default)
+# @rw: #optional whether to allow write operations (default: false)
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevOptionsVVFAT',
+ 'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
+ '*rw': 'bool' } }
+
+##
+# @BlockdevOptionsGenericFormat
+#
+# Driver specific block device options for image format that have no option
+# besides their data source.
+#
+# @file: reference to or definition of the data source block device
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevOptionsGenericFormat',
+ 'data': { 'file': 'BlockdevRef' } }
+
+##
+# @BlockdevOptionsGenericCOWFormat
+#
+# Driver specific block device options for image format that have no option
+# besides their data source and an optional backing file.
+#
+# @backing: #optional 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.
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevOptionsGenericCOWFormat',
+ 'base': 'BlockdevOptionsGenericFormat',
+ 'data': { '*backing': 'BlockdevRef' } }
+
+##
+# @BlockdevOptionsQcow2
+#
+# Driver specific block device options for qcow2.
+#
+# @lazy-refcounts: #optional whether to enable the lazy refcounts feature
+# (default is taken from the image file)
+#
+# @pass-discard-request: #optional whether discard requests to the qcow2 device
+# should be forwarded to the data source
+#
+# @pass-discard-snapshot: #optional 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
+#
+# @pass-discard-other: #optional whether discard requests for the data source
+# should be issued on other occasions where a cluster gets
+# freed
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevOptionsQcow2',
+ 'base': 'BlockdevOptionsGenericCOWFormat',
+ 'data': { '*lazy-refcounts': 'bool',
+ '*pass-discard-request': 'bool',
+ '*pass-discard-snapshot': 'bool',
+ '*pass-discard-other': 'bool' } }
+
+##
+# @BlockdevOptions
+#
+# Options for creating a block device.
+#
+# Since: 1.7
+##
+{ 'union': 'BlockdevOptions',
+ 'base': 'BlockdevOptionsBase',
+ 'discriminator': 'driver',
+ 'data': {
+ 'file': 'BlockdevOptionsFile',
+ 'http': 'BlockdevOptionsFile',
+ 'https': 'BlockdevOptionsFile',
+ 'ftp': 'BlockdevOptionsFile',
+ 'ftps': 'BlockdevOptionsFile',
+ 'tftp': 'BlockdevOptionsFile',
+# TODO gluster: Wait for structured options
+# TODO iscsi: Wait for structured options
+# TODO nbd: Should take InetSocketAddress for 'host'?
+# TODO rbd: Wait for structured options
+# TODO sheepdog: Wait for structured options
+# TODO ssh: Should take InetSocketAddress for 'host'?
+ 'vvfat': 'BlockdevOptionsVVFAT',
+
+# TODO blkdebug: Wait for structured options
+# TODO blkverify: Wait for structured options
+
+ 'bochs': 'BlockdevOptionsGenericFormat',
+ 'cloop': 'BlockdevOptionsGenericFormat',
+ 'cow': 'BlockdevOptionsGenericCOWFormat',
+ 'dmg': 'BlockdevOptionsGenericFormat',
+ 'parallels': 'BlockdevOptionsGenericFormat',
+ 'qcow': 'BlockdevOptionsGenericCOWFormat',
+ 'qcow2': 'BlockdevOptionsQcow2',
+ 'qed': 'BlockdevOptionsGenericCOWFormat',
+ 'raw': 'BlockdevOptionsGenericFormat',
+ 'vdi': 'BlockdevOptionsGenericFormat',
+ 'vhdx': 'BlockdevOptionsGenericFormat',
+ 'vmdk': 'BlockdevOptionsGenericCOWFormat',
+ 'vpc': 'BlockdevOptionsGenericFormat'
+ } }
+
+##
+# @BlockdevRef
+#
+# Reference to a block device.
+#
+# @definition: defines a new block device inline
+# @reference: references the ID of an existing block device. An
+# empty string means that no block device should be
+# referenced.
+#
+# Since: 1.7
+##
+{ 'union': 'BlockdevRef',
+ 'discriminator': {},
+ 'data': { 'definition': 'BlockdevOptions',
+ 'reference': 'str' } }
+
+##
+# @blockdev-add:
+#
+# Creates a new block device.
+#
+# @options: block device options for the new device
+#
+# Since: 1.7
+##
+{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b17c46e..449ecea 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3240,3 +3240,62 @@ Example:
}
EQMP
+
+ {
+ .name = "blockdev-add",
+ .args_type = "options:q",
+ .mhandler.cmd_new = qmp_marshal_input_blockdev_add,
+ },
+
+SQMP
+blockdev-add
+------------
+
+Add a block device.
+
+Arguments:
+
+- "options": block driver options
+
+Example (1):
+
+-> { "execute": "blockdev-add",
+ "arguments": { "options" : { "driver": "qcow2",
+ "file": { "driver": "file",
+ "filename": "test.qcow2" } } } }
+<- { "return": {} }
+
+Example (2):
+
+-> { "execute": "blockdev-add",
+ "arguments": {
+ "options": {
+ "driver": "qcow2",
+ "id": "my_disk",
+ "discard": "unmap",
+ "throttling": {
+ "bps-total": 1234567,
+ "iops-write": 100
+ },
+ "cache": {
+ "direct": true,
+ "writeback": true
+ },
+ "file": {
+ "driver": "file",
+ "filename": "/tmp/test.qcow2"
+ },
+ "backing": {
+ "driver": "raw",
+ "file": {
+ "driver": "file",
+ "filename": "/dev/fdset/4"
+ }
+ }
+ }
+ }
+ }
+
+<- { "return": {} }
+
+EQMP
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command Kevin Wolf
@ 2013-10-01 14:00 ` Benoît Canet
2013-10-01 15:41 ` Eric Blake
1 sibling, 0 replies; 38+ messages in thread
From: Benoît Canet @ 2013-10-01 14:00 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
Le Tuesday 01 Oct 2013 à 15:20:06 (+0200), Kevin Wolf a écrit :
> For examples see the changes to qmp-commands.hx.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 57 ++++++++++++++
> qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 59 ++++++++++++++
> 3 files changed, 352 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 29a5b70..3059987 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -38,6 +38,8 @@
> #include "qemu/option.h"
> #include "qemu/config-file.h"
> #include "qapi/qmp/types.h"
> +#include "qapi-visit.h"
> +#include "qapi/qmp-output-visitor.h"
> #include "sysemu/sysemu.h"
> #include "block/block_int.h"
> #include "qmp-commands.h"
> @@ -2066,6 +2068,61 @@ void qmp_block_job_complete(const char *device, Error **errp)
> block_job_complete(job, errp);
> }
>
> +void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
> +{
> + QmpOutputVisitor *ov = qmp_output_visitor_new();
> + QObject *obj;
> + QDict *qdict;
> + DriveInfo *dinfo;
> + Error *local_err = NULL;
> +
> + /* Require an ID in the top level */
> + if (!options->has_id) {
> + error_setg(errp, "Block device needs an ID");
Here we leak ov.
Maybe goto fail;
> + return;
> + }
> +
> + /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
> + * cache.direct=false instead of silently switching to aio=threads, except
> + * if called from drive_init.
> + *
> + * For now, simply forbidding the combination for all drivers will do. */
> + if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
> + bool direct = options->cache->has_direct && options->cache->direct;
> + if (!options->has_cache && !direct) {
> + error_setg(errp, "aio=native requires cache.direct=true");
> + goto fail;
> + }
> + }
> +
> + visit_type_BlockdevOptions(qmp_output_get_visitor(ov),
> + &options, NULL, &local_err);
> + if (error_is_set(&local_err)) {
> + error_propagate(errp, local_err);
> + goto fail;
> + }
> +
> + obj = qmp_output_get_qobject(ov);
> + qdict = qobject_to_qdict(obj);
> +
> + qdict_flatten(qdict);
> +
> + QemuOpts *opts = qemu_opts_from_qdict(&qemu_drive_opts, qdict, &local_err);
> + if (error_is_set(&local_err)) {
> + error_propagate(errp, local_err);
> + goto fail;
> + }
> +
> + dinfo = blockdev_init(opts, IF_NONE);
> + if (!dinfo) {
> + error_setg(errp, "Could not open image");
> + goto fail;
> + }
> +
> +fail:
> + qmp_output_visitor_cleanup(ov);
> +}
> +
> static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
> {
> BlockJobInfoList **prev = opaque;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 145eca8..7192119 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3902,3 +3902,239 @@
> ##
> { 'command': 'query-rx-filter', 'data': { '*name': 'str' },
> 'returns': ['RxFilterInfo'] }
> +
> +
> +##
> +# @BlockdevDiscardOptions
> +#
> +# Determines how to handle discard requests.
> +#
> +# @ignore: Ignore the request
> +# @unmap: Forward as an unmap request
> +#
> +# Since: 1.7
> +##
> +{ 'enum': 'BlockdevDiscardOptions',
> + 'data': [ 'ignore', 'unmap' ] }
> +
> +##
> +# @BlockdevAioOptions
> +#
> +# Selects the AIO backend to handle I/O requests
> +#
> +# @threads: Use qemu's thread pool
> +# @native: Use native AIO backend (only Linux and Windows)
> +#
> +# Since: 1.7
> +##
> +{ 'enum': 'BlockdevAioOptions',
> + 'data': [ 'threads', 'native' ] }
> +
> +##
> +# @BlockdevCacheOptions
> +#
> +# Includes cache-related options for block devices
> +#
> +# @writeback: #optional enables writeback mode for any caches (default: true)
> +# @direct: #optional enables use of O_DIRECT (bypass the host page cache;
> +# default: false)
> +# @no-flush: #optional ignore any flush requests for the device (default:
> +# false)
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevCacheOptions',
> + 'data': { '*writeback': 'bool',
> + '*direct': 'bool',
> + '*no-flush': 'bool' } }
> +
> +##
> +# @BlockdevOptionsBase
> +#
> +# Options that are available for all block devices, independent of the block
> +# driver.
> +#
> +# @driver: block driver name
> +# @id: #optional id by which the new block device can be referred to.
> +# This is a required option on the top level of blockdev-add, and
> +# currently not allowed on any other level.
> +# @discard: #optional discard-related options (default: ignore)
> +# @cache: #optional cache-related options
> +# @aio: #optional AIO backend (default: threads)
> +# @rerror: #optional how to handle read errors on the device
> +# (default: report)
> +# @werror: #optional how to handle write errors on the device
> +# (default: enospc)
> +# @read-only: #optional whether the block device should be read-only
> +# (default: false)
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsBase',
> + 'data': { 'driver': 'str',
> + '*id': 'str',
> + '*discard': 'BlockdevDiscardOptions',
> + '*cache': 'BlockdevCacheOptions',
> + '*aio': 'BlockdevAioOptions',
> + '*rerror': 'BlockdevOnError',
> + '*werror': 'BlockdevOnError',
> + '*read-only': 'bool' } }
> +
> +##
> +# @BlockdevOptionsFile
> +#
> +# Driver specific block device options for the file backend and similar
> +# protocols.
> +#
> +# @filename: path to the image file
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsFile',
> + 'data': { 'filename': 'str' } }
> +
> +##
> +# @BlockdevOptionsVVFAT
> +#
> +# Driver specific block device options for the vvfat protocol.
> +#
> +# @dir: directory to be exported as FAT image
> +# @fat-type: #optional FAT type: 12, 16 or 32
> +# @floppy: #optional whether to export a floppy image (true) or
> +# partitioned hard disk (false; default)
> +# @rw: #optional whether to allow write operations (default: false)
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsVVFAT',
> + 'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
> + '*rw': 'bool' } }
> +
> +##
> +# @BlockdevOptionsGenericFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source.
> +#
> +# @file: reference to or definition of the data source block device
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsGenericFormat',
> + 'data': { 'file': 'BlockdevRef' } }
> +
> +##
> +# @BlockdevOptionsGenericCOWFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source and an optional backing file.
> +#
> +# @backing: #optional 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.
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsGenericCOWFormat',
> + 'base': 'BlockdevOptionsGenericFormat',
> + 'data': { '*backing': 'BlockdevRef' } }
> +
> +##
> +# @BlockdevOptionsQcow2
> +#
> +# Driver specific block device options for qcow2.
> +#
> +# @lazy-refcounts: #optional whether to enable the lazy refcounts feature
> +# (default is taken from the image file)
> +#
> +# @pass-discard-request: #optional whether discard requests to the qcow2 device
> +# should be forwarded to the data source
> +#
> +# @pass-discard-snapshot: #optional 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
> +#
> +# @pass-discard-other: #optional whether discard requests for the data source
> +# should be issued on other occasions where a cluster gets
> +# freed
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsQcow2',
> + 'base': 'BlockdevOptionsGenericCOWFormat',
> + 'data': { '*lazy-refcounts': 'bool',
> + '*pass-discard-request': 'bool',
> + '*pass-discard-snapshot': 'bool',
> + '*pass-discard-other': 'bool' } }
> +
> +##
> +# @BlockdevOptions
> +#
> +# Options for creating a block device.
> +#
> +# Since: 1.7
> +##
> +{ 'union': 'BlockdevOptions',
> + 'base': 'BlockdevOptionsBase',
> + 'discriminator': 'driver',
> + 'data': {
> + 'file': 'BlockdevOptionsFile',
> + 'http': 'BlockdevOptionsFile',
> + 'https': 'BlockdevOptionsFile',
> + 'ftp': 'BlockdevOptionsFile',
> + 'ftps': 'BlockdevOptionsFile',
> + 'tftp': 'BlockdevOptionsFile',
> +# TODO gluster: Wait for structured options
> +# TODO iscsi: Wait for structured options
> +# TODO nbd: Should take InetSocketAddress for 'host'?
> +# TODO rbd: Wait for structured options
> +# TODO sheepdog: Wait for structured options
> +# TODO ssh: Should take InetSocketAddress for 'host'?
> + 'vvfat': 'BlockdevOptionsVVFAT',
> +
> +# TODO blkdebug: Wait for structured options
> +# TODO blkverify: Wait for structured options
> +
> + 'bochs': 'BlockdevOptionsGenericFormat',
> + 'cloop': 'BlockdevOptionsGenericFormat',
> + 'cow': 'BlockdevOptionsGenericCOWFormat',
> + 'dmg': 'BlockdevOptionsGenericFormat',
> + 'parallels': 'BlockdevOptionsGenericFormat',
> + 'qcow': 'BlockdevOptionsGenericCOWFormat',
> + 'qcow2': 'BlockdevOptionsQcow2',
> + 'qed': 'BlockdevOptionsGenericCOWFormat',
> + 'raw': 'BlockdevOptionsGenericFormat',
> + 'vdi': 'BlockdevOptionsGenericFormat',
> + 'vhdx': 'BlockdevOptionsGenericFormat',
> + 'vmdk': 'BlockdevOptionsGenericCOWFormat',
> + 'vpc': 'BlockdevOptionsGenericFormat'
> + } }
> +
> +##
> +# @BlockdevRef
> +#
> +# Reference to a block device.
> +#
> +# @definition: defines a new block device inline
> +# @reference: references the ID of an existing block device. An
> +# empty string means that no block device should be
> +# referenced.
> +#
> +# Since: 1.7
> +##
> +{ 'union': 'BlockdevRef',
> + 'discriminator': {},
> + 'data': { 'definition': 'BlockdevOptions',
> + 'reference': 'str' } }
> +
> +##
> +# @blockdev-add:
> +#
> +# Creates a new block device.
> +#
> +# @options: block device options for the new device
> +#
> +# Since: 1.7
> +##
> +{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index b17c46e..449ecea 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3240,3 +3240,62 @@ Example:
> }
>
> EQMP
> +
> + {
> + .name = "blockdev-add",
> + .args_type = "options:q",
> + .mhandler.cmd_new = qmp_marshal_input_blockdev_add,
> + },
> +
> +SQMP
> +blockdev-add
> +------------
> +
> +Add a block device.
> +
> +Arguments:
> +
> +- "options": block driver options
> +
> +Example (1):
> +
> +-> { "execute": "blockdev-add",
> + "arguments": { "options" : { "driver": "qcow2",
> + "file": { "driver": "file",
> + "filename": "test.qcow2" } } } }
> +<- { "return": {} }
> +
> +Example (2):
> +
> +-> { "execute": "blockdev-add",
> + "arguments": {
> + "options": {
> + "driver": "qcow2",
> + "id": "my_disk",
> + "discard": "unmap",
> + "throttling": {
> + "bps-total": 1234567,
> + "iops-write": 100
> + },
> + "cache": {
> + "direct": true,
> + "writeback": true
> + },
> + "file": {
> + "driver": "file",
> + "filename": "/tmp/test.qcow2"
> + },
> + "backing": {
> + "driver": "raw",
> + "file": {
> + "driver": "file",
> + "filename": "/dev/fdset/4"
> + }
> + }
> + }
> + }
> + }
> +
> +<- { "return": {} }
> +
> +EQMP
> --
> 1.8.1.4
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command Kevin Wolf
2013-10-01 14:00 ` Benoît Canet
@ 2013-10-01 15:41 ` Eric Blake
1 sibling, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 15:41 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> For examples see the changes to qmp-commands.hx.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 57 ++++++++++++++
> qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 59 ++++++++++++++
> 3 files changed, 352 insertions(+)
>
> +# @BlockdevOptionsVVFAT
> +#
> +# Driver specific block device options for the vvfat protocol.
> +#
> +# @dir: directory to be exported as FAT image
> +# @fat-type: #optional FAT type: 12, 16 or 32
> +# @floppy: #optional whether to export a floppy image (true) or
> +# partitioned hard disk (false; default)
> +# @rw: #optional whether to allow write operations (default: false)
Aligned across multiple options...
> +# @BlockdevOptionsGenericFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source.
> +#
> +# @file: reference to or definition of the data source block device
...nothing to compare to here (but aligned with other commands)...
> +# @BlockdevOptionsGenericCOWFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source and an optional backing file.
> +#
> +# @backing: #optional 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.
...and again...
> +# Driver specific block device options for qcow2.
> +#
> +# @lazy-refcounts: #optional whether to enable the lazy refcounts feature
> +# (default is taken from the image file)
> +#
> +# @pass-discard-request: #optional whether discard requests to the qcow2 device
> +# should be forwarded to the data source
...but here, using exactly one space (different alignment per option).
I honestly don't care which style you use. And while it looks odd
having two different styles in the same patch, it really doesn't matter
to me, so I'm not asking for a respin just for this (then again, you
already have a fix needed in the C code).
> +SQMP
> +blockdev-add
> +------------
> +
> +Add a block device.
> +
> +Arguments:
> +
> +- "options": block driver options
> +
> +Example (1):
> +
> +-> { "execute": "blockdev-add",
> + "arguments": { "options" : { "driver": "qcow2",
> + "file": { "driver": "file",
> + "filename": "test.qcow2" } } } }
> +<- { "return": {} }
> +
> +Example (2):
> +
> +-> { "execute": "blockdev-add",
> + "arguments": {
> + "options": {
> + "driver": "qcow2",
> + "id": "my_disk",
> + "discard": "unmap",
> + "throttling": {
> + "bps-total": 1234567,
> + "iops-write": 100
> + },
Drop throttling, as it is no longer in the qapi.
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 05/17] blockdev: Separate ID generation from DriveInfo creation
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (3 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 04/17] blockdev: 'blockdev-add' QMP command Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 06/17] blockdev: Pass QDict to blockdev_init() Kevin Wolf
` (12 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
blockdev-add shouldn't automatically generate IDs, but will keep most of
the DriveInfo creation code.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 32 +++++++++++++++++---------------
include/qemu/option.h | 1 +
util/qemu-option.c | 6 ++++++
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 3059987..f19933f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -604,23 +604,25 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
return NULL;
}
- /* init */
-
- dinfo = g_malloc0(sizeof(*dinfo));
- if ((buf = qemu_opts_id(opts)) != NULL) {
- dinfo->id = g_strdup(buf);
- } else {
- /* no id supplied -> create one */
- dinfo->id = g_malloc0(32);
- if (type == IF_IDE || type == IF_SCSI)
+ /* no id supplied -> create one */
+ if (qemu_opts_id(opts) == NULL) {
+ char *new_id;
+ if (type == IF_IDE || type == IF_SCSI) {
mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd";
- if (max_devs)
- snprintf(dinfo->id, 32, "%s%i%s%i",
- if_name[type], bus_id, mediastr, unit_id);
- else
- snprintf(dinfo->id, 32, "%s%s%i",
- if_name[type], mediastr, unit_id);
+ }
+ if (max_devs) {
+ new_id = g_strdup_printf("%s%i%s%i", if_name[type], bus_id,
+ mediastr, unit_id);
+ } else {
+ new_id = g_strdup_printf("%s%s%i", if_name[type],
+ mediastr, unit_id);
+ }
+ qemu_opts_set_id(opts, new_id);
}
+
+ /* init */
+ dinfo = g_malloc0(sizeof(*dinfo));
+ dinfo->id = g_strdup(qemu_opts_id(opts));
dinfo->bdrv = bdrv_new(dinfo->id);
dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
dinfo->bdrv->read_only = ro;
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 63db4cc..5c0c6dd 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -142,6 +142,7 @@ void qemu_opts_loc_restore(QemuOpts *opts);
int qemu_opts_set(QemuOptsList *list, const char *id,
const char *name, const char *value);
const char *qemu_opts_id(QemuOpts *opts);
+void qemu_opts_set_id(QemuOpts *opts, char *id);
void qemu_opts_del(QemuOpts *opts);
void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp);
int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index e0844a9..efcb5dc 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -834,6 +834,12 @@ const char *qemu_opts_id(QemuOpts *opts)
return opts->id;
}
+/* The id string will be g_free()d by qemu_opts_del */
+void qemu_opts_set_id(QemuOpts *opts, char *id)
+{
+ opts->id = id;
+}
+
void qemu_opts_del(QemuOpts *opts)
{
QemuOpt *opt;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [Qemu-devel] [PATCH v2 06/17] blockdev: Pass QDict to blockdev_init()
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (4 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 05/17] blockdev: Separate ID generation from DriveInfo creation Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 15:53 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 07/17] blockdev: Move parsing of 'media' option to drive_init Kevin Wolf
` (11 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
Working on a QDict instead of a QemuOpts that accepts anything is more
in line with bdrv_open(). A QDict is what qmp_blockdev_add() already has
anyway, so this saves additional conversions. And last, but not least,
it allows later patches to easily extract legacy options into a
separate, typed QemuOpts for drive_init() (the untyped QemuOpts that
drive_init already has doesn't allow access to numbers, only strings,
and is therefore useless without conversion).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
blockdev.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f19933f..c0d0d34 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -217,7 +217,10 @@ static void bdrv_format_print(void *opaque, const char *name)
static void drive_uninit(DriveInfo *dinfo)
{
- qemu_opts_del(dinfo->opts);
+ if (dinfo->opts) {
+ qemu_opts_del(dinfo->opts);
+ }
+
bdrv_unref(dinfo->bdrv);
g_free(dinfo->id);
QTAILQ_REMOVE(&drives, dinfo, next);
@@ -302,7 +305,8 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
return true;
}
-static DriveInfo *blockdev_init(QemuOpts *all_opts,
+/* Takes the ownership of bs_opts */
+static DriveInfo *blockdev_init(QDict *bs_opts,
BlockInterfaceType block_default_type)
{
const char *buf;
@@ -326,7 +330,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
int ret;
Error *error = NULL;
QemuOpts *opts;
- QDict *bs_opts;
const char *id;
bool has_driver_specific_opts;
BlockDriver *drv = NULL;
@@ -334,9 +337,9 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
translation = BIOS_ATA_TRANSLATION_AUTO;
media = MEDIA_DISK;
- /* Check common options by copying from all_opts to opts, all other options
- * are stored in bs_opts. */
- id = qemu_opts_id(all_opts);
+ /* Check common options by copying from bs_opts to opts, all other options
+ * stay in bs_opts for processing by bdrv_open(). */
+ id = qdict_get_try_str(bs_opts, "id");
opts = qemu_opts_create(&qemu_common_drive_opts, id, 1, &error);
if (error_is_set(&error)) {
qerror_report_err(error);
@@ -344,8 +347,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
return NULL;
}
- bs_opts = qdict_new();
- qemu_opts_to_qdict(all_opts, bs_opts);
qemu_opts_absorb_qdict(opts, bs_opts, &error);
if (error_is_set(&error)) {
qerror_report_err(error);
@@ -634,7 +635,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
dinfo->heads = heads;
dinfo->secs = secs;
dinfo->trans = translation;
- dinfo->opts = all_opts;
dinfo->refcount = 1;
if (serial != NULL) {
dinfo->serial = g_strdup(serial);
@@ -759,6 +759,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
{
const char *value;
DriveInfo *dinfo;
+ QDict *bs_opts;
/* Change legacy command line options into QMP ones */
qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
@@ -807,14 +808,19 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
qemu_opt_unset(all_opts, "cache");
}
+ /* Get a QDict for processing the options */
+ bs_opts = qdict_new();
+ qemu_opts_to_qdict(all_opts, bs_opts);
+
/* Actual block device init: Functionality shared with blockdev-add */
- dinfo = blockdev_init(all_opts, block_default_type);
+ dinfo = blockdev_init(bs_opts, block_default_type);
if (dinfo == NULL) {
goto fail;
}
/* Set legacy DriveInfo fields */
dinfo->enable_auto_del = true;
+ dinfo->opts = all_opts;
fail:
return dinfo;
@@ -2109,13 +2115,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
qdict_flatten(qdict);
- QemuOpts *opts = qemu_opts_from_qdict(&qemu_drive_opts, qdict, &local_err);
- if (error_is_set(&local_err)) {
- error_propagate(errp, local_err);
- goto fail;
- }
-
- dinfo = blockdev_init(opts, IF_NONE);
+ dinfo = blockdev_init(qdict, IF_NONE);
if (!dinfo) {
error_setg(errp, "Could not open image");
goto fail;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 06/17] blockdev: Pass QDict to blockdev_init()
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 06/17] blockdev: Pass QDict to blockdev_init() Kevin Wolf
@ 2013-10-01 15:53 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 15:53 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 904 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> Working on a QDict instead of a QemuOpts that accepts anything is more
> in line with bdrv_open(). A QDict is what qmp_blockdev_add() already has
> anyway, so this saves additional conversions. And last, but not least,
> it allows later patches to easily extract legacy options into a
> separate, typed QemuOpts for drive_init() (the untyped QemuOpts that
> drive_init already has doesn't allow access to numbers, only strings,
> and is therefore useless without conversion).
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Benoit Canet <benoit@irqsave.net>
> ---
> blockdev.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 07/17] blockdev: Move parsing of 'media' option to drive_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (5 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 06/17] blockdev: Pass QDict to blockdev_init() Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 15:57 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' " Kevin Wolf
` (10 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
This moves as much as possible of the processing of the 'media' option
to drive_init so that it can only be accessed using legacy functions,
but never with anything blockdev-add related.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
blockdev.c | 73 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 23 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index c0d0d34..d364dca 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -305,16 +305,18 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp)
return true;
}
+typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
+
/* Takes the ownership of bs_opts */
static DriveInfo *blockdev_init(QDict *bs_opts,
- BlockInterfaceType block_default_type)
+ BlockInterfaceType block_default_type,
+ DriveMediaType media)
{
const char *buf;
const char *file = NULL;
const char *serial;
const char *mediastr = "";
BlockInterfaceType type;
- enum { MEDIA_DISK, MEDIA_CDROM } media;
int bus_id, unit_id;
int cyls, heads, secs, translation;
int max_devs;
@@ -335,7 +337,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
BlockDriver *drv = NULL;
translation = BIOS_ATA_TRANSLATION_AUTO;
- media = MEDIA_DISK;
/* Check common options by copying from bs_opts to opts, all other options
* stay in bs_opts for processing by bdrv_open(). */
@@ -422,19 +423,11 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
}
}
- if ((buf = qemu_opt_get(opts, "media")) != NULL) {
- if (!strcmp(buf, "disk")) {
- media = MEDIA_DISK;
- } else if (!strcmp(buf, "cdrom")) {
- if (cyls || secs || heads) {
- error_report("CHS can't be set with media=%s", buf);
- return NULL;
- }
- media = MEDIA_CDROM;
- } else {
- error_report("'%s' invalid media", buf);
- return NULL;
- }
+ if (media == MEDIA_CDROM) {
+ if (cyls || secs || heads) {
+ error_report("CHS can't be set with media=cdrom");
+ return NULL;
+ }
}
if ((buf = qemu_opt_get(opts, "discard")) != NULL) {
@@ -755,11 +748,27 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
}
}
+QemuOptsList qemu_legacy_drive_opts = {
+ .name = "drive",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_legacy_drive_opts.head),
+ .desc = {
+ {
+ .name = "media",
+ .type = QEMU_OPT_STRING,
+ .help = "media type (disk, cdrom)",
+ },
+ { /* end of list */ }
+ },
+};
+
DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
{
const char *value;
- DriveInfo *dinfo;
+ DriveInfo *dinfo = NULL;
QDict *bs_opts;
+ QemuOpts *legacy_opts;
+ DriveMediaType media = MEDIA_DISK;
+ Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
@@ -812,8 +821,29 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
bs_opts = qdict_new();
qemu_opts_to_qdict(all_opts, bs_opts);
+ legacy_opts = qemu_opts_create_nofail(&qemu_legacy_drive_opts);
+ qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ goto fail;
+ }
+
+ /* Media type */
+ value = qemu_opt_get(legacy_opts, "media");
+ if (value) {
+ if (!strcmp(value, "disk")) {
+ media = MEDIA_DISK;
+ } else if (!strcmp(value, "cdrom")) {
+ media = MEDIA_CDROM;
+ } else {
+ error_report("'%s' invalid media", value);
+ goto fail;
+ }
+ }
+
/* Actual block device init: Functionality shared with blockdev-add */
- dinfo = blockdev_init(bs_opts, block_default_type);
+ dinfo = blockdev_init(bs_opts, block_default_type, media);
if (dinfo == NULL) {
goto fail;
}
@@ -823,6 +853,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo->opts = all_opts;
fail:
+ qemu_opts_del(legacy_opts);
return dinfo;
}
@@ -2115,7 +2146,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
qdict_flatten(qdict);
- dinfo = blockdev_init(qdict, IF_NONE);
+ dinfo = blockdev_init(qdict, IF_NONE, MEDIA_DISK);
if (!dinfo) {
error_setg(errp, "Could not open image");
goto fail;
@@ -2184,10 +2215,6 @@ QemuOptsList qemu_common_drive_opts = {
.type = QEMU_OPT_STRING,
.help = "chs translation (auto, lba. none)",
},{
- .name = "media",
- .type = QEMU_OPT_STRING,
- .help = "media type (disk, cdrom)",
- },{
.name = "snapshot",
.type = QEMU_OPT_BOOL,
.help = "enable/disable snapshot mode",
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 07/17] blockdev: Move parsing of 'media' option to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 07/17] blockdev: Move parsing of 'media' option to drive_init Kevin Wolf
@ 2013-10-01 15:57 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 15:57 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> This moves as much as possible of the processing of the 'media' option
> to drive_init so that it can only be accessed using legacy functions,
> but never with anything blockdev-add related.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Benoit Canet <benoit@irqsave.net>
> ---
> blockdev.c | 73 ++++++++++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 50 insertions(+), 23 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' option to drive_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (6 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 07/17] blockdev: Move parsing of 'media' option to drive_init Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 14:44 ` Benoît Canet
2013-10-01 16:01 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 09/17] blockdev: Moving parsing of geometry options " Kevin Wolf
` (9 subsequent siblings)
17 siblings, 2 replies; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
It's always IF_NONE for blockdev-add.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index d364dca..ce1208f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -309,14 +309,13 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
/* Takes the ownership of bs_opts */
static DriveInfo *blockdev_init(QDict *bs_opts,
- BlockInterfaceType block_default_type,
+ BlockInterfaceType type,
DriveMediaType media)
{
const char *buf;
const char *file = NULL;
const char *serial;
const char *mediastr = "";
- BlockInterfaceType type;
int bus_id, unit_id;
int cyls, heads, secs, translation;
int max_devs;
@@ -377,17 +376,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
file = qemu_opt_get(opts, "file");
serial = qemu_opt_get(opts, "serial");
- if ((buf = qemu_opt_get(opts, "if")) != NULL) {
- for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++)
- ;
- if (type == IF_COUNT) {
- error_report("unsupported bus type '%s'", buf);
- return NULL;
- }
- } else {
- type = block_default_type;
- }
-
max_devs = if_max_devs[type];
if (cyls || heads || secs) {
@@ -756,6 +744,10 @@ QemuOptsList qemu_legacy_drive_opts = {
.name = "media",
.type = QEMU_OPT_STRING,
.help = "media type (disk, cdrom)",
+ },{
+ .name = "if",
+ .type = QEMU_OPT_STRING,
+ .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
},
{ /* end of list */ }
},
@@ -768,6 +760,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
QDict *bs_opts;
QemuOpts *legacy_opts;
DriveMediaType media = MEDIA_DISK;
+ BlockInterfaceType type;
Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
@@ -842,8 +835,23 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
}
}
+ /* Controller type */
+ value = qemu_opt_get(legacy_opts, "if");
+ if (value) {
+ for (type = 0;
+ type < IF_COUNT && strcmp(value, if_name[type]);
+ type++) {
+ }
+ if (type == IF_COUNT) {
+ error_report("unsupported bus type '%s'", value);
+ goto fail;
+ }
+ } else {
+ type = block_default_type;
+ }
+
/* Actual block device init: Functionality shared with blockdev-add */
- dinfo = blockdev_init(bs_opts, block_default_type, media);
+ dinfo = blockdev_init(bs_opts, type, media);
if (dinfo == NULL) {
goto fail;
}
@@ -2191,10 +2199,6 @@ QemuOptsList qemu_common_drive_opts = {
.type = QEMU_OPT_NUMBER,
.help = "unit number (i.e. lun for scsi)",
},{
- .name = "if",
- .type = QEMU_OPT_STRING,
- .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
- },{
.name = "index",
.type = QEMU_OPT_NUMBER,
.help = "index number",
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' option to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' " Kevin Wolf
@ 2013-10-01 14:44 ` Benoît Canet
2013-10-01 16:01 ` Eric Blake
1 sibling, 0 replies; 38+ messages in thread
From: Benoît Canet @ 2013-10-01 14:44 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
Le Tuesday 01 Oct 2013 à 15:20:10 (+0200), Kevin Wolf a écrit :
> It's always IF_NONE for blockdev-add.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 40 ++++++++++++++++++++++------------------
> 1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index d364dca..ce1208f 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -309,14 +309,13 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
>
> /* Takes the ownership of bs_opts */
> static DriveInfo *blockdev_init(QDict *bs_opts,
> - BlockInterfaceType block_default_type,
> + BlockInterfaceType type,
> DriveMediaType media)
> {
> const char *buf;
> const char *file = NULL;
> const char *serial;
> const char *mediastr = "";
> - BlockInterfaceType type;
> int bus_id, unit_id;
> int cyls, heads, secs, translation;
> int max_devs;
> @@ -377,17 +376,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> file = qemu_opt_get(opts, "file");
> serial = qemu_opt_get(opts, "serial");
>
> - if ((buf = qemu_opt_get(opts, "if")) != NULL) {
> - for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++)
> - ;
> - if (type == IF_COUNT) {
> - error_report("unsupported bus type '%s'", buf);
> - return NULL;
> - }
> - } else {
> - type = block_default_type;
> - }
> -
> max_devs = if_max_devs[type];
>
> if (cyls || heads || secs) {
> @@ -756,6 +744,10 @@ QemuOptsList qemu_legacy_drive_opts = {
> .name = "media",
> .type = QEMU_OPT_STRING,
> .help = "media type (disk, cdrom)",
> + },{
> + .name = "if",
> + .type = QEMU_OPT_STRING,
> + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
> },
> { /* end of list */ }
> },
> @@ -768,6 +760,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
> QDict *bs_opts;
> QemuOpts *legacy_opts;
> DriveMediaType media = MEDIA_DISK;
> + BlockInterfaceType type;
> Error *local_err = NULL;
>
> /* Change legacy command line options into QMP ones */
> @@ -842,8 +835,23 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
> }
> }
>
> + /* Controller type */
> + value = qemu_opt_get(legacy_opts, "if");
> + if (value) {
> + for (type = 0;
> + type < IF_COUNT && strcmp(value, if_name[type]);
> + type++) {
> + }
> + if (type == IF_COUNT) {
> + error_report("unsupported bus type '%s'", value);
> + goto fail;
> + }
> + } else {
> + type = block_default_type;
> + }
> +
> /* Actual block device init: Functionality shared with blockdev-add */
> - dinfo = blockdev_init(bs_opts, block_default_type, media);
> + dinfo = blockdev_init(bs_opts, type, media);
> if (dinfo == NULL) {
> goto fail;
> }
> @@ -2191,10 +2199,6 @@ QemuOptsList qemu_common_drive_opts = {
> .type = QEMU_OPT_NUMBER,
> .help = "unit number (i.e. lun for scsi)",
> },{
> - .name = "if",
> - .type = QEMU_OPT_STRING,
> - .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
> - },{
> .name = "index",
> .type = QEMU_OPT_NUMBER,
> .help = "index number",
> --
> 1.8.1.4
>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' option to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' " Kevin Wolf
2013-10-01 14:44 ` Benoît Canet
@ 2013-10-01 16:01 ` Eric Blake
1 sibling, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 16:01 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 420 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> It's always IF_NONE for blockdev-add.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 40 ++++++++++++++++++++++------------------
> 1 file changed, 22 insertions(+), 18 deletions(-)
>
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 09/17] blockdev: Moving parsing of geometry options to drive_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (7 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 08/17] blockdev: Move parsing of 'if' " Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 16:09 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 10/17] blockdev: Move parsing of 'boot' option " Kevin Wolf
` (8 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
This moves all of the geometry options (cyls/heads/secs/trans) to
drive_init so that they can only be accessed using legacy functions, but
never with anything blockdev-add related.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 136 +++++++++++++++++++++++++++++++------------------------------
1 file changed, 69 insertions(+), 67 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index ce1208f..033202d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -317,7 +317,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
const char *serial;
const char *mediastr = "";
int bus_id, unit_id;
- int cyls, heads, secs, translation;
int max_devs;
int index;
int ro = 0;
@@ -335,8 +334,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
bool has_driver_specific_opts;
BlockDriver *drv = NULL;
- translation = BIOS_ATA_TRANSLATION_AUTO;
-
/* Check common options by copying from bs_opts to opts, all other options
* stay in bs_opts for processing by bdrv_open(). */
id = qdict_get_try_str(bs_opts, "id");
@@ -365,10 +362,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
unit_id = qemu_opt_get_number(opts, "unit", -1);
index = qemu_opt_get_number(opts, "index", -1);
- cyls = qemu_opt_get_number(opts, "cyls", 0);
- heads = qemu_opt_get_number(opts, "heads", 0);
- secs = qemu_opt_get_number(opts, "secs", 0);
-
snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
ro = qemu_opt_get_bool(opts, "read-only", 0);
copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false);
@@ -378,46 +371,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
max_devs = if_max_devs[type];
- if (cyls || heads || secs) {
- if (cyls < 1) {
- error_report("invalid physical cyls number");
- return NULL;
- }
- if (heads < 1) {
- error_report("invalid physical heads number");
- return NULL;
- }
- if (secs < 1) {
- error_report("invalid physical secs number");
- return NULL;
- }
- }
-
- if ((buf = qemu_opt_get(opts, "trans")) != NULL) {
- if (!cyls) {
- error_report("'%s' trans must be used with cyls, heads and secs",
- buf);
- return NULL;
- }
- if (!strcmp(buf, "none"))
- translation = BIOS_ATA_TRANSLATION_NONE;
- else if (!strcmp(buf, "lba"))
- translation = BIOS_ATA_TRANSLATION_LBA;
- else if (!strcmp(buf, "auto"))
- translation = BIOS_ATA_TRANSLATION_AUTO;
- else {
- error_report("'%s' invalid translation type", buf);
- return NULL;
- }
- }
-
- if (media == MEDIA_CDROM) {
- if (cyls || secs || heads) {
- error_report("CHS can't be set with media=cdrom");
- return NULL;
- }
- }
-
if ((buf = qemu_opt_get(opts, "discard")) != NULL) {
if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) {
error_report("invalid discard option");
@@ -612,10 +565,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
dinfo->type = type;
dinfo->bus = bus_id;
dinfo->unit = unit_id;
- dinfo->cyls = cyls;
- dinfo->heads = heads;
- dinfo->secs = secs;
- dinfo->trans = translation;
dinfo->refcount = 1;
if (serial != NULL) {
dinfo->serial = g_strdup(serial);
@@ -748,6 +697,22 @@ QemuOptsList qemu_legacy_drive_opts = {
.name = "if",
.type = QEMU_OPT_STRING,
.help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
+ },{
+ .name = "cyls",
+ .type = QEMU_OPT_NUMBER,
+ .help = "number of cylinders (ide disk geometry)",
+ },{
+ .name = "heads",
+ .type = QEMU_OPT_NUMBER,
+ .help = "number of heads (ide disk geometry)",
+ },{
+ .name = "secs",
+ .type = QEMU_OPT_NUMBER,
+ .help = "number of sectors (ide disk geometry)",
+ },{
+ .name = "trans",
+ .type = QEMU_OPT_STRING,
+ .help = "chs translation (auto, lba. none)",
},
{ /* end of list */ }
},
@@ -761,6 +726,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
QemuOpts *legacy_opts;
DriveMediaType media = MEDIA_DISK;
BlockInterfaceType type;
+ int cyls, heads, secs, translation;
Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
@@ -850,6 +816,53 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
type = block_default_type;
}
+ /* Geometry */
+ cyls = qemu_opt_get_number(legacy_opts, "cyls", 0);
+ heads = qemu_opt_get_number(legacy_opts, "heads", 0);
+ secs = qemu_opt_get_number(legacy_opts, "secs", 0);
+
+ if (cyls || heads || secs) {
+ if (cyls < 1) {
+ error_report("invalid physical cyls number");
+ goto fail;
+ }
+ if (heads < 1) {
+ error_report("invalid physical heads number");
+ goto fail;
+ }
+ if (secs < 1) {
+ error_report("invalid physical secs number");
+ goto fail;
+ }
+ }
+
+ translation = BIOS_ATA_TRANSLATION_AUTO;
+ value = qemu_opt_get(legacy_opts, "trans");
+ if (value != NULL) {
+ if (!cyls) {
+ error_report("'%s' trans must be used with cyls, heads and secs",
+ value);
+ goto fail;
+ }
+ if (!strcmp(value, "none")) {
+ translation = BIOS_ATA_TRANSLATION_NONE;
+ } else if (!strcmp(value, "lba")) {
+ translation = BIOS_ATA_TRANSLATION_LBA;
+ } else if (!strcmp(value, "auto")) {
+ translation = BIOS_ATA_TRANSLATION_AUTO;
+ } else {
+ error_report("'%s' invalid translation type", value);
+ goto fail;
+ }
+ }
+
+ if (media == MEDIA_CDROM) {
+ if (cyls || secs || heads) {
+ error_report("CHS can't be set with media=cdrom");
+ goto fail;
+ }
+ }
+
/* Actual block device init: Functionality shared with blockdev-add */
dinfo = blockdev_init(bs_opts, type, media);
if (dinfo == NULL) {
@@ -860,6 +873,11 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo->enable_auto_del = true;
dinfo->opts = all_opts;
+ dinfo->cyls = cyls;
+ dinfo->heads = heads;
+ dinfo->secs = secs;
+ dinfo->trans = translation;
+
fail:
qemu_opts_del(legacy_opts);
return dinfo;
@@ -2203,22 +2221,6 @@ QemuOptsList qemu_common_drive_opts = {
.type = QEMU_OPT_NUMBER,
.help = "index number",
},{
- .name = "cyls",
- .type = QEMU_OPT_NUMBER,
- .help = "number of cylinders (ide disk geometry)",
- },{
- .name = "heads",
- .type = QEMU_OPT_NUMBER,
- .help = "number of heads (ide disk geometry)",
- },{
- .name = "secs",
- .type = QEMU_OPT_NUMBER,
- .help = "number of sectors (ide disk geometry)",
- },{
- .name = "trans",
- .type = QEMU_OPT_STRING,
- .help = "chs translation (auto, lba. none)",
- },{
.name = "snapshot",
.type = QEMU_OPT_BOOL,
.help = "enable/disable snapshot mode",
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 09/17] blockdev: Moving parsing of geometry options to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 09/17] blockdev: Moving parsing of geometry options " Kevin Wolf
@ 2013-10-01 16:09 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 16:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> This moves all of the geometry options (cyls/heads/secs/trans) to
> drive_init so that they can only be accessed using legacy functions, but
> never with anything blockdev-add related.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 136 +++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 69 insertions(+), 67 deletions(-)
>
> + },{
> + .name = "trans",
> + .type = QEMU_OPT_STRING,
> + .help = "chs translation (auto, lba. none)",
s/lba\./lba,/
> - },{
> - .name = "trans",
> - .type = QEMU_OPT_STRING,
> - .help = "chs translation (auto, lba. none)",
Of course, that's just a code motion of an existing typo. And trivial
enough that it can be fixed by the maintainer without a respin.
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 10/17] blockdev: Move parsing of 'boot' option to drive_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (8 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 09/17] blockdev: Moving parsing of geometry options " Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 16:19 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 11/17] blockdev: Move bus/unit/index processing " Kevin Wolf
` (7 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
It's already ignored and only prints a deprecation message. No use in
making it available in new interfaces.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 033202d..1888941 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -456,12 +456,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
return NULL;
}
- if (qemu_opt_get(opts, "boot") != NULL) {
- fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be "
- "ignored. Future versions will reject this parameter. Please "
- "update your scripts.\n");
- }
-
on_write_error = BLOCKDEV_ON_ERROR_ENOSPC;
if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) {
@@ -713,6 +707,10 @@ QemuOptsList qemu_legacy_drive_opts = {
.name = "trans",
.type = QEMU_OPT_STRING,
.help = "chs translation (auto, lba. none)",
+ },{
+ .name = "boot",
+ .type = QEMU_OPT_BOOL,
+ .help = "(deprecated, ignored)",
},
{ /* end of list */ }
},
@@ -788,6 +786,13 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
goto fail;
}
+ /* Deprecated option boot=[on|off] */
+ if (qemu_opt_get(legacy_opts, "boot") != NULL) {
+ fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be "
+ "ignored. Future versions will reject this parameter. Please "
+ "update your scripts.\n");
+ }
+
/* Media type */
value = qemu_opt_get(legacy_opts, "media");
if (value) {
@@ -2328,10 +2333,6 @@ QemuOptsList qemu_common_drive_opts = {
.name = "copy-on-read",
.type = QEMU_OPT_BOOL,
.help = "copy read data from backing file into image file",
- },{
- .name = "boot",
- .type = QEMU_OPT_BOOL,
- .help = "(deprecated, ignored)",
},
{ /* end of list */ }
},
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 10/17] blockdev: Move parsing of 'boot' option to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 10/17] blockdev: Move parsing of 'boot' option " Kevin Wolf
@ 2013-10-01 16:19 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 16:19 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 475 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> It's already ignored and only prints a deprecation message. No use in
> making it available in new interfaces.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 11/17] blockdev: Move bus/unit/index processing to drive_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (9 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 10/17] blockdev: Move parsing of 'boot' option " Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 16:25 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 12/17] blockdev: Move virtio-blk device creation " Kevin Wolf
` (6 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
This requires moving the automatic ID generation at the same time, so
let's do that as well.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 157 ++++++++++++++++++++++++++++---------------------------------
1 file changed, 73 insertions(+), 84 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 1888941..4a0ee17 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -315,10 +315,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
const char *buf;
const char *file = NULL;
const char *serial;
- const char *mediastr = "";
- int bus_id, unit_id;
- int max_devs;
- int index;
int ro = 0;
int bdrv_flags = 0;
int on_read_error, on_write_error;
@@ -358,10 +354,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
has_driver_specific_opts = !!qdict_size(bs_opts);
/* extract parameters */
- bus_id = qemu_opt_get_number(opts, "bus", 0);
- unit_id = qemu_opt_get_number(opts, "unit", -1);
- index = qemu_opt_get_number(opts, "index", -1);
-
snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
ro = qemu_opt_get_bool(opts, "read-only", 0);
copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false);
@@ -369,8 +361,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
file = qemu_opt_get(opts, "file");
serial = qemu_opt_get(opts, "serial");
- max_devs = if_max_devs[type];
-
if ((buf = qemu_opt_get(opts, "discard")) != NULL) {
if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) {
error_report("invalid discard option");
@@ -489,66 +479,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
}
}
- /* compute bus and unit according index */
-
- if (index != -1) {
- if (bus_id != 0 || unit_id != -1) {
- error_report("index cannot be used with bus and unit");
- return NULL;
- }
- bus_id = drive_index_to_bus_id(type, index);
- unit_id = drive_index_to_unit_id(type, index);
- }
-
- /* if user doesn't specify a unit_id,
- * try to find the first free
- */
-
- if (unit_id == -1) {
- unit_id = 0;
- while (drive_get(type, bus_id, unit_id) != NULL) {
- unit_id++;
- if (max_devs && unit_id >= max_devs) {
- unit_id -= max_devs;
- bus_id++;
- }
- }
- }
-
- /* check unit id */
-
- if (max_devs && unit_id >= max_devs) {
- error_report("unit %d too big (max is %d)",
- unit_id, max_devs - 1);
- return NULL;
- }
-
- /*
- * catch multiple definitions
- */
-
- if (drive_get(type, bus_id, unit_id) != NULL) {
- error_report("drive with bus=%d, unit=%d (index=%d) exists",
- bus_id, unit_id, index);
- return NULL;
- }
-
- /* no id supplied -> create one */
- if (qemu_opts_id(opts) == NULL) {
- char *new_id;
- if (type == IF_IDE || type == IF_SCSI) {
- mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd";
- }
- if (max_devs) {
- new_id = g_strdup_printf("%s%i%s%i", if_name[type], bus_id,
- mediastr, unit_id);
- } else {
- new_id = g_strdup_printf("%s%s%i", if_name[type],
- mediastr, unit_id);
- }
- qemu_opts_set_id(opts, new_id);
- }
-
/* init */
dinfo = g_malloc0(sizeof(*dinfo));
dinfo->id = g_strdup(qemu_opts_id(opts));
@@ -557,8 +487,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
dinfo->bdrv->read_only = ro;
dinfo->devaddr = devaddr;
dinfo->type = type;
- dinfo->bus = bus_id;
- dinfo->unit = unit_id;
dinfo->refcount = 1;
if (serial != NULL) {
dinfo->serial = g_strdup(serial);
@@ -684,6 +612,18 @@ QemuOptsList qemu_legacy_drive_opts = {
.head = QTAILQ_HEAD_INITIALIZER(qemu_legacy_drive_opts.head),
.desc = {
{
+ .name = "bus",
+ .type = QEMU_OPT_NUMBER,
+ .help = "bus number",
+ },{
+ .name = "unit",
+ .type = QEMU_OPT_NUMBER,
+ .help = "unit number (i.e. lun for scsi)",
+ },{
+ .name = "index",
+ .type = QEMU_OPT_NUMBER,
+ .help = "index number",
+ },{
.name = "media",
.type = QEMU_OPT_STRING,
.help = "media type (disk, cdrom)",
@@ -725,6 +665,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
DriveMediaType media = MEDIA_DISK;
BlockInterfaceType type;
int cyls, heads, secs, translation;
+ int max_devs, bus_id, unit_id, index;
Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
@@ -868,6 +809,63 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
}
}
+ /* Device address specified by bus/unit or index.
+ * If none was specified, try to find the first free one. */
+ bus_id = qemu_opt_get_number(legacy_opts, "bus", 0);
+ unit_id = qemu_opt_get_number(legacy_opts, "unit", -1);
+ index = qemu_opt_get_number(legacy_opts, "index", -1);
+
+ max_devs = if_max_devs[type];
+
+ if (index != -1) {
+ if (bus_id != 0 || unit_id != -1) {
+ error_report("index cannot be used with bus and unit");
+ goto fail;
+ }
+ bus_id = drive_index_to_bus_id(type, index);
+ unit_id = drive_index_to_unit_id(type, index);
+ }
+
+ if (unit_id == -1) {
+ unit_id = 0;
+ while (drive_get(type, bus_id, unit_id) != NULL) {
+ unit_id++;
+ if (max_devs && unit_id >= max_devs) {
+ unit_id -= max_devs;
+ bus_id++;
+ }
+ }
+ }
+
+ if (max_devs && unit_id >= max_devs) {
+ error_report("unit %d too big (max is %d)", unit_id, max_devs - 1);
+ goto fail;
+ }
+
+ if (drive_get(type, bus_id, unit_id) != NULL) {
+ error_report("drive with bus=%d, unit=%d (index=%d) exists",
+ bus_id, unit_id, index);
+ goto fail;
+ }
+
+ /* no id supplied -> create one */
+ if (qemu_opts_id(all_opts) == NULL) {
+ char *new_id;
+ const char *mediastr = "";
+ if (type == IF_IDE || type == IF_SCSI) {
+ mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd";
+ }
+ if (max_devs) {
+ new_id = g_strdup_printf("%s%i%s%i", if_name[type], bus_id,
+ mediastr, unit_id);
+ } else {
+ new_id = g_strdup_printf("%s%s%i", if_name[type],
+ mediastr, unit_id);
+ }
+ qdict_put(bs_opts, "id", qstring_from_str(new_id));
+ g_free(new_id);
+ }
+
/* Actual block device init: Functionality shared with blockdev-add */
dinfo = blockdev_init(bs_opts, type, media);
if (dinfo == NULL) {
@@ -883,6 +881,9 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo->secs = secs;
dinfo->trans = translation;
+ dinfo->bus = bus_id;
+ dinfo->unit = unit_id;
+
fail:
qemu_opts_del(legacy_opts);
return dinfo;
@@ -2214,18 +2215,6 @@ QemuOptsList qemu_common_drive_opts = {
.head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
.desc = {
{
- .name = "bus",
- .type = QEMU_OPT_NUMBER,
- .help = "bus number",
- },{
- .name = "unit",
- .type = QEMU_OPT_NUMBER,
- .help = "unit number (i.e. lun for scsi)",
- },{
- .name = "index",
- .type = QEMU_OPT_NUMBER,
- .help = "index number",
- },{
.name = "snapshot",
.type = QEMU_OPT_BOOL,
.help = "enable/disable snapshot mode",
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 11/17] blockdev: Move bus/unit/index processing to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 11/17] blockdev: Move bus/unit/index processing " Kevin Wolf
@ 2013-10-01 16:25 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 16:25 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 542 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> This requires moving the automatic ID generation at the same time, so
> let's do that as well.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> blockdev.c | 157 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 73 insertions(+), 84 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 12/17] blockdev: Move virtio-blk device creation to drive_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (10 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 11/17] blockdev: Move bus/unit/index processing " Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 16:34 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 13/17] blockdev: Remove IF_* check for read-only blockdev_init Kevin Wolf
` (5 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 54 +++++++++++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 4a0ee17..8b1a0c1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -318,7 +318,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
int ro = 0;
int bdrv_flags = 0;
int on_read_error, on_write_error;
- const char *devaddr;
DriveInfo *dinfo;
ThrottleConfig cfg;
int snapshot = 0;
@@ -472,20 +471,12 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
}
}
- if ((devaddr = qemu_opt_get(opts, "addr")) != NULL) {
- if (type != IF_VIRTIO) {
- error_report("addr is not supported by this bus type");
- return NULL;
- }
- }
-
/* init */
dinfo = g_malloc0(sizeof(*dinfo));
dinfo->id = g_strdup(qemu_opts_id(opts));
dinfo->bdrv = bdrv_new(dinfo->id);
dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
dinfo->bdrv->read_only = ro;
- dinfo->devaddr = devaddr;
dinfo->type = type;
dinfo->refcount = 1;
if (serial != NULL) {
@@ -512,22 +503,8 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
case IF_FLOPPY:
case IF_PFLASH:
case IF_MTD:
- break;
case IF_VIRTIO:
- {
- /* add virtio block device */
- QemuOpts *devopts;
- devopts = qemu_opts_create_nofail(qemu_find_opts("device"));
- if (arch_type == QEMU_ARCH_S390X) {
- qemu_opt_set(devopts, "driver", "virtio-blk-s390");
- } else {
- qemu_opt_set(devopts, "driver", "virtio-blk-pci");
- }
- qemu_opt_set(devopts, "drive", dinfo->id);
- if (devaddr)
- qemu_opt_set(devopts, "addr", devaddr);
break;
- }
default:
abort();
}
@@ -651,6 +628,10 @@ QemuOptsList qemu_legacy_drive_opts = {
.name = "boot",
.type = QEMU_OPT_BOOL,
.help = "(deprecated, ignored)",
+ },{
+ .name = "addr",
+ .type = QEMU_OPT_STRING,
+ .help = "pci address (virtio only)",
},
{ /* end of list */ }
},
@@ -666,6 +647,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
BlockInterfaceType type;
int cyls, heads, secs, translation;
int max_devs, bus_id, unit_id, index;
+ const char *devaddr;
Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
@@ -866,6 +848,27 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
g_free(new_id);
}
+ /* Add virtio block device */
+ devaddr = qemu_opt_get(legacy_opts, "addr");
+ if (devaddr && type != IF_VIRTIO) {
+ error_report("addr is not supported by this bus type");
+ goto fail;
+ }
+
+ if (type == IF_VIRTIO) {
+ QemuOpts *devopts;
+ devopts = qemu_opts_create_nofail(qemu_find_opts("device"));
+ if (arch_type == QEMU_ARCH_S390X) {
+ qemu_opt_set(devopts, "driver", "virtio-blk-s390");
+ } else {
+ qemu_opt_set(devopts, "driver", "virtio-blk-pci");
+ }
+ qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"));
+ if (devaddr) {
+ qemu_opt_set(devopts, "addr", devaddr);
+ }
+ }
+
/* Actual block device init: Functionality shared with blockdev-add */
dinfo = blockdev_init(bs_opts, type, media);
if (dinfo == NULL) {
@@ -883,6 +886,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo->bus = bus_id;
dinfo->unit = unit_id;
+ dinfo->devaddr = devaddr;
fail:
qemu_opts_del(legacy_opts);
@@ -2259,10 +2263,6 @@ QemuOptsList qemu_common_drive_opts = {
.type = QEMU_OPT_STRING,
.help = "write error action",
},{
- .name = "addr",
- .type = QEMU_OPT_STRING,
- .help = "pci address (virtio only)",
- },{
.name = "read-only",
.type = QEMU_OPT_BOOL,
.help = "open drive file as read-only",
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 12/17] blockdev: Move virtio-blk device creation to drive_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 12/17] blockdev: Move virtio-blk device creation " Kevin Wolf
@ 2013-10-01 16:34 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 16:34 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> blockdev.c | 54 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 27 insertions(+), 27 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 13/17] blockdev: Remove IF_* check for read-only blockdev_init
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (11 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 12/17] blockdev: Move virtio-blk device creation " Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 16:51 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del Kevin Wolf
` (4 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
IF_NONE allows read-only, which makes forbidding it in this place
for other types pretty much pointless.
Instead, make sure that all devices for which the check would have
errored out check in their init function that they don't get a read-only
BlockDriverState. This catches even cases where IF_NONE and -device is
used.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 6 ------
hw/block/m25p80.c | 5 +++++
hw/block/xen_disk.c | 5 +++++
hw/sd/milkymist-memcard.c | 4 ++++
hw/sd/omap_mmc.c | 6 ++++++
hw/sd/pl181.c | 4 ++++
hw/sd/pxa2xx_mmci.c | 3 +++
hw/sd/sd.c | 5 +++++
hw/sd/sdhci.c | 3 +++
hw/sd/ssi-sd.c | 3 +++
tests/qemu-iotests/051.out | 5 ++++-
11 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 8b1a0c1..c465dff 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -532,12 +532,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
if (media == MEDIA_CDROM) {
/* CDROM is fine for any interface, don't check. */
ro = 1;
- } else if (ro == 1) {
- if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
- type != IF_NONE && type != IF_PFLASH) {
- error_report("read-only not supported by this bus type");
- goto err;
- }
}
bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8c3b7f0..02a1544 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -624,6 +624,11 @@ static int m25p80_init(SSISlave *ss)
if (dinfo && dinfo->bdrv) {
DB_PRINT_L(0, "Binding to IF_MTD drive\n");
s->bdrv = dinfo->bdrv;
+ if (bdrv_is_read_only(s->bdrv)) {
+ fprintf(stderr, "Can't use a read-only drive");
+ return 1;
+ }
+
/* FIXME: Move to late init */
if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size,
BDRV_SECTOR_SIZE))) {
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index f35fc59..253a45f 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -829,6 +829,11 @@ static int blk_connect(struct XenDevice *xendev)
/* setup via qemu cmdline -> already setup for us */
xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
blkdev->bs = blkdev->dinfo->bdrv;
+ if (bdrv_is_read_only(blkdev->bs) && !readonly) {
+ xen_be_printf(&blkdev->xendev, 0, "Unexpected read-only drive");
+ blkdev->bs = NULL;
+ return -1;
+ }
/* blkdev->bs is not create by us, we get a reference
* so we can bdrv_unref() unconditionally */
bdrv_ref(blkdev->bs);
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 42613b3..d1168c9 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -255,6 +255,10 @@ static int milkymist_memcard_init(SysBusDevice *dev)
dinfo = drive_get_next(IF_SD);
s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false);
+ if (s->card == NULL) {
+ return -1;
+ }
+
s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index bf5d1fb..937a478 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -593,6 +593,9 @@ struct omap_mmc_s *omap_mmc_init(hwaddr base,
/* Instantiate the storage */
s->card = sd_init(bd, false);
+ if (s->card == NULL) {
+ exit(1);
+ }
return s;
}
@@ -618,6 +621,9 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
/* Instantiate the storage */
s->card = sd_init(bd, false);
+ if (s->card == NULL) {
+ exit(1);
+ }
s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];
sd_set_cb(s->card, NULL, s->cdet);
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 03875bf..c35896d 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -491,6 +491,10 @@ static int pl181_init(SysBusDevice *sbd)
qdev_init_gpio_out(dev, s->cardstatus, 2);
dinfo = drive_get_next(IF_SD);
s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false);
+ if (s->card == NULL) {
+ return -1;
+ }
+
return 0;
}
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 90c955f..b9d8b1a 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -539,6 +539,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
/* Instantiate the actual storage */
s->card = sd_init(bd, false);
+ if (s->card == NULL) {
+ exit(1);
+ }
register_savevm(NULL, "pxa2xx_mmci", 0, 0,
pxa2xx_mmci_save, pxa2xx_mmci_load, s);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 346d86f..7380f06 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -494,6 +494,11 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
{
SDState *sd;
+ if (bdrv_is_read_only(bs)) {
+ fprintf(stderr, "sd_init: Cannot use read-only drive\n");
+ return NULL;
+ }
+
sd = (SDState *) g_malloc0(sizeof(SDState));
sd->buf = qemu_blockalign(bs, 512);
sd->spi = is_spi;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1483e19..0906a1d 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1166,6 +1166,9 @@ static void sdhci_initfn(Object *obj)
di = drive_get_next(IF_SD);
s->card = sd_init(di ? di->bdrv : NULL, false);
+ if (s->card == NULL) {
+ exit(1);
+ }
s->eject_cb = qemu_allocate_irqs(sdhci_insert_eject_cb, s, 1)[0];
s->ro_cb = qemu_allocate_irqs(sdhci_card_readonly_cb, s, 1)[0];
sd_set_cb(s->card, s->ro_cb, s->eject_cb);
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index d47e237..1bb56c4 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -246,6 +246,9 @@ static int ssi_sd_init(SSISlave *dev)
s->mode = SSI_SD_CMD;
dinfo = drive_get_next(IF_SD);
s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, true);
+ if (s->sd == NULL) {
+ return -1;
+ }
register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
return 0;
}
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 04bb236..95e3686 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -139,7 +139,10 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: read-only not supported by this bus type
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) QEMU_PROG: Can't use a read-only drive
+QEMU_PROG: Device initialization failed.
+QEMU_PROG: Initialization of device ide-hd failed
Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on
QEMU X.Y.Z monitor - type 'help' for more information
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 13/17] blockdev: Remove IF_* check for read-only blockdev_init
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 13/17] blockdev: Remove IF_* check for read-only blockdev_init Kevin Wolf
@ 2013-10-01 16:51 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 16:51 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> IF_NONE allows read-only, which makes forbidding it in this place
> for other types pretty much pointless.
>
> Instead, make sure that all devices for which the check would have
> errored out check in their init function that they don't get a read-only
> BlockDriverState. This catches even cases where IF_NONE and -device is
> used.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 6 ------
> hw/block/m25p80.c | 5 +++++
> hw/block/xen_disk.c | 5 +++++
> hw/sd/milkymist-memcard.c | 4 ++++
> hw/sd/omap_mmc.c | 6 ++++++
> hw/sd/pl181.c | 4 ++++
> hw/sd/pxa2xx_mmci.c | 3 +++
> hw/sd/sd.c | 5 +++++
> hw/sd/sdhci.c | 3 +++
> hw/sd/ssi-sd.c | 3 +++
> tests/qemu-iotests/051.out | 5 ++++-
> 11 files changed, 42 insertions(+), 7 deletions(-)
> +++ b/hw/sd/omap_mmc.c
> @@ -593,6 +593,9 @@ struct omap_mmc_s *omap_mmc_init(hwaddr base,
>
> /* Instantiate the storage */
> s->card = sd_init(bd, false);
> + if (s->card == NULL) {
> + exit(1);
No error message about why the exit? Also, is it worth using
EXIT_FAILURE instead of a magic number?
> +++ b/hw/sd/sd.c
> @@ -494,6 +494,11 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
> {
> SDState *sd;
>
> + if (bdrv_is_read_only(bs)) {
> + fprintf(stderr, "sd_init: Cannot use read-only drive\n");
> + return NULL;
Oh, I guess there IS an error message before exit.
And you're not the first person to not use EXIT_FAILURE. So I can live
with the patch as-is.
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (12 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 13/17] blockdev: Remove IF_* check for read-only blockdev_init Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 17:06 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 15/17] blockdev: Remove 'media' parameter from blockdev_init() Kevin Wolf
` (3 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
Block devices creates with -drive and drive_add should automatically
disappear if the guest device is unplugged. blockdev-add ones shouldn't.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/064 | 133 +++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/064.out | 80 +++++++++++++++++++++++
tests/qemu-iotests/common.filter | 8 +++
tests/qemu-iotests/group | 1 +
4 files changed, 222 insertions(+)
create mode 100755 tests/qemu-iotests/064
create mode 100644 tests/qemu-iotests/064.out
diff --git a/tests/qemu-iotests/064 b/tests/qemu-iotests/064
new file mode 100755
index 0000000..79dc38b
--- /dev/null
+++ b/tests/qemu-iotests/064
@@ -0,0 +1,133 @@
+#!/bin/bash
+#
+# Test automatic deletion of BDSes created by -drive/drive_add
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+ echo Testing: "$@"
+ $QEMU -nographic -qmp stdio -serial none "$@"
+ echo
+}
+
+function run_qemu()
+{
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp
+}
+
+size=128M
+
+_make_test_img $size
+
+echo
+echo === -drive/-device and device_del ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0 <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "device_del", "arguments": { "id": "virtio0" } }
+{ "execute": "system_reset" }
+{ "execute": "query-block" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === -drive/device_add and device_del ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "device_add",
+ "arguments": { "driver": "virtio-blk-pci", "drive": "disk",
+ "id": "virtio0" } }
+{ "execute": "device_del", "arguments": { "id": "virtio0" } }
+{ "execute": "system_reset" }
+{ "execute": "query-block" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === drive_add/device_add and device_del ===
+echo
+
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "human-monitor-command",
+ "arguments": { "command-line": "drive_add 0 file=$TEST_IMG,format=$IMGFMT,if=none,id=disk" } }
+{ "execute": "query-block" }
+{ "execute": "device_add",
+ "arguments": { "driver": "virtio-blk-pci", "drive": "disk",
+ "id": "virtio0" } }
+{ "execute": "device_del", "arguments": { "id": "virtio0" } }
+{ "execute": "system_reset" }
+{ "execute": "query-block" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === blockdev_add/device_add and device_del ===
+echo
+
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+ "arguments": {
+ "options": {
+ "driver": "$IMGFMT",
+ "id": "disk",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
+ }
+ }
+ }
+ }
+{ "execute": "query-block" }
+{ "execute": "device_add",
+ "arguments": { "driver": "virtio-blk-pci", "drive": "disk",
+ "id": "virtio0" } }
+{ "execute": "device_del", "arguments": { "id": "virtio0" } }
+{ "execute": "system_reset" }
+{ "execute": "query-block" }
+{ "execute": "quit" }
+EOF
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/064.out b/tests/qemu-iotests/064.out
new file mode 100644
index 0000000..8c7a334
--- /dev/null
+++ b/tests/qemu-iotests/064.out
@@ -0,0 +1,80 @@
+QA output created by 064
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+
+=== -drive/-device and device_del ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": 139264, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"device": "virtio0", "path": "/machine/peripheral/virtio0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESET"}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
+
+=== -drive/device_add and device_del ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
+QMP_VERSION
+{"return": {}}
+{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": 139264, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"device": "virtio0", "path": "/machine/peripheral/virtio0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESET"}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
+
+=== drive_add/device_add and device_del ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": "OK\r\n"}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": 139264, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"device": "virtio0", "path": "/machine/peripheral/virtio0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESET"}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
+
+=== blockdev_add/device_add and device_del ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": 139264, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"device": "virtio0", "path": "/machine/peripheral/virtio0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESET"}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": 139264, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
+*** done
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 5dfda63..8e7b1a4 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -159,5 +159,13 @@ _filter_qemu()
-e 's#^QEMU [0-9]\+\.[0-9]\+\.[0-9]\+ monitor#QEMU X.Y.Z monitor#'
}
+# replace problematic QMP output like timestamps
+_filter_qmp()
+{
+ _filter_win32 | \
+ sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \
+ -e 's#^{"QMP":.*}$#QMP_VERSION#'
+}
+
# make sure this script returns success
/bin/true
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1ad02e5..9e950fd 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -69,3 +69,4 @@
061 rw auto
062 rw auto
063 rw auto
+064 rw auto
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del Kevin Wolf
@ 2013-10-01 17:06 ` Eric Blake
2013-10-08 9:44 ` Kevin Wolf
0 siblings, 1 reply; 38+ messages in thread
From: Eric Blake @ 2013-10-01 17:06 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 950 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> Block devices creates with -drive and drive_add should automatically
> disappear if the guest device is unplugged. blockdev-add ones shouldn't.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/064 | 133 +++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/064.out | 80 +++++++++++++++++++++++
> tests/qemu-iotests/common.filter | 8 +++
> tests/qemu-iotests/group | 1 +
> 4 files changed, 222 insertions(+)
> create mode 100755 tests/qemu-iotests/064
> create mode 100644 tests/qemu-iotests/064.out
Reviewed-by: Eric Blake <eblake@redhat.com>
> +
> +_supported_fmt qcow2
> +_supported_proto file
> +_supported_os Linux
Is this test really Linux-only?
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del
2013-10-01 17:06 ` Eric Blake
@ 2013-10-08 9:44 ` Kevin Wolf
0 siblings, 0 replies; 38+ messages in thread
From: Kevin Wolf @ 2013-10-08 9:44 UTC (permalink / raw)
To: Eric Blake; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
Am 01.10.2013 um 19:06 hat Eric Blake geschrieben:
> On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> > Block devices creates with -drive and drive_add should automatically
> > disappear if the guest device is unplugged. blockdev-add ones shouldn't.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > ---
> > tests/qemu-iotests/064 | 133 +++++++++++++++++++++++++++++++++++++++
> > tests/qemu-iotests/064.out | 80 +++++++++++++++++++++++
> > tests/qemu-iotests/common.filter | 8 +++
> > tests/qemu-iotests/group | 1 +
> > 4 files changed, 222 insertions(+)
> > create mode 100755 tests/qemu-iotests/064
> > create mode 100644 tests/qemu-iotests/064.out
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>
> > +
> > +_supported_fmt qcow2
> > +_supported_proto file
> > +_supported_os Linux
>
> Is this test really Linux-only?
Probably most tests aren't, but all of them have "_supported_os Linux".
If anyone wanted to use this for something, they'd have to make a bigger
change than just to this patch.
Kevin
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 15/17] blockdev: Remove 'media' parameter from blockdev_init()
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (13 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 17:07 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 16/17] blockdev: Don't disable COR automatically with blockdev-add Kevin Wolf
` (2 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
The remaining users shouldn't be there with blockdev-add and are easy to
move to drive_init().
Bonus bug fix: As a side effect, CD-ROM drives can now use block drivers
on the read-only whitelist without explicitly specifying read-only=on,
even if a format is explicitly specified.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 40 +++++++++++++++-------------------------
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index c465dff..bb6a1d5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -309,8 +309,7 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
/* Takes the ownership of bs_opts */
static DriveInfo *blockdev_init(QDict *bs_opts,
- BlockInterfaceType type,
- DriveMediaType media)
+ BlockInterfaceType type)
{
const char *buf;
const char *file = NULL;
@@ -492,22 +491,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
bdrv_set_io_limits(dinfo->bdrv, &cfg);
}
- switch(type) {
- case IF_IDE:
- case IF_SCSI:
- case IF_XEN:
- case IF_NONE:
- dinfo->media_cd = media == MEDIA_CDROM;
- break;
- case IF_SD:
- case IF_FLOPPY:
- case IF_PFLASH:
- case IF_MTD:
- case IF_VIRTIO:
- break;
- default:
- abort();
- }
if (!file || !*file) {
if (has_driver_specific_opts) {
file = NULL;
@@ -529,11 +512,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
bdrv_flags |= BDRV_O_INCOMING;
}
- if (media == MEDIA_CDROM) {
- /* CDROM is fine for any interface, don't check. */
- ro = 1;
- }
-
bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
if (ro && copy_on_read) {
@@ -717,6 +695,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
media = MEDIA_DISK;
} else if (!strcmp(value, "cdrom")) {
media = MEDIA_CDROM;
+ qdict_put(bs_opts, "read-only", qstring_from_str("on"));
} else {
error_report("'%s' invalid media", value);
goto fail;
@@ -864,7 +843,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
}
/* Actual block device init: Functionality shared with blockdev-add */
- dinfo = blockdev_init(bs_opts, type, media);
+ dinfo = blockdev_init(bs_opts, type);
if (dinfo == NULL) {
goto fail;
}
@@ -882,6 +861,17 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
dinfo->unit = unit_id;
dinfo->devaddr = devaddr;
+ switch(type) {
+ case IF_IDE:
+ case IF_SCSI:
+ case IF_XEN:
+ case IF_NONE:
+ dinfo->media_cd = media == MEDIA_CDROM;
+ break;
+ default:
+ break;
+ }
+
fail:
qemu_opts_del(legacy_opts);
return dinfo;
@@ -2176,7 +2166,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
qdict_flatten(qdict);
- dinfo = blockdev_init(qdict, IF_NONE, MEDIA_DISK);
+ dinfo = blockdev_init(qdict, IF_NONE);
if (!dinfo) {
error_setg(errp, "Could not open image");
goto fail;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 15/17] blockdev: Remove 'media' parameter from blockdev_init()
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 15/17] blockdev: Remove 'media' parameter from blockdev_init() Kevin Wolf
@ 2013-10-01 17:07 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 17:07 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 721 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> The remaining users shouldn't be there with blockdev-add and are easy to
> move to drive_init().
>
> Bonus bug fix: As a side effect, CD-ROM drives can now use block drivers
> on the read-only whitelist without explicitly specifying read-only=on,
> even if a format is explicitly specified.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> blockdev.c | 40 +++++++++++++++-------------------------
> 1 file changed, 15 insertions(+), 25 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 16/17] blockdev: Don't disable COR automatically with blockdev-add
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (14 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 15/17] blockdev: Remove 'media' parameter from blockdev_init() Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 17:09 ` Eric Blake
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 17/17] blockdev: blockdev_init() error conversion Kevin Wolf
2013-10-01 15:41 ` [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Eric Blake
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
If a read-only device is configured with copy-on-read=on, the old code
only prints a warning and automatically disables copy on read. Make it
a real error for blockdev-add.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 9 +++++++--
blockdev.c | 31 +++++++++++++++++++++++++++----
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 93e113a..dc63f02 100644
--- a/block.c
+++ b/block.c
@@ -774,8 +774,13 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
}
assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
- if (!bs->read_only && (flags & BDRV_O_COPY_ON_READ)) {
- bdrv_enable_copy_on_read(bs);
+ if (flags & BDRV_O_COPY_ON_READ) {
+ if (!bs->read_only) {
+ bdrv_enable_copy_on_read(bs);
+ } else {
+ error_setg(errp, "Can't use copy-on-read on read-only device");
+ return -EINVAL;
+ }
}
if (filename != NULL) {
diff --git a/blockdev.c b/blockdev.c
index bb6a1d5..4f015af 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -514,10 +514,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
- if (ro && copy_on_read) {
- error_report("warning: disabling copy_on_read on read-only drive");
- }
-
QINCREF(bs_opts);
ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
@@ -605,6 +601,18 @@ QemuOptsList qemu_legacy_drive_opts = {
.type = QEMU_OPT_STRING,
.help = "pci address (virtio only)",
},
+
+ /* Options that are passed on, but have special semantics with -drive */
+ {
+ .name = "read-only",
+ .type = QEMU_OPT_BOOL,
+ .help = "open drive file as read-only",
+ },{
+ .name = "copy-on-read",
+ .type = QEMU_OPT_BOOL,
+ .help = "copy read data from backing file into image file",
+ },
+
{ /* end of list */ }
},
};
@@ -620,6 +628,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
int cyls, heads, secs, translation;
int max_devs, bus_id, unit_id, index;
const char *devaddr;
+ bool read_only, copy_on_read;
Error *local_err = NULL;
/* Change legacy command line options into QMP ones */
@@ -702,6 +711,20 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
}
}
+ /* copy-on-read is disabled with a warning for read-only devices */
+ read_only = qemu_opt_get_bool(legacy_opts, "read-only", false);
+ copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
+
+ if (read_only && copy_on_read) {
+ error_report("warning: disabling copy-on-read on read-only drive");
+ copy_on_read = false;
+ }
+
+ qdict_put(bs_opts, "read-only",
+ qstring_from_str(read_only ? "on" : "off"));
+ qdict_put(bs_opts, "copy-on-read",
+ qstring_from_str(copy_on_read ? "on" :"off"));
+
/* Controller type */
value = qemu_opt_get(legacy_opts, "if");
if (value) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 16/17] blockdev: Don't disable COR automatically with blockdev-add
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 16/17] blockdev: Don't disable COR automatically with blockdev-add Kevin Wolf
@ 2013-10-01 17:09 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 17:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 624 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> If a read-only device is configured with copy-on-read=on, the old code
> only prints a warning and automatically disables copy on read. Make it
> a real error for blockdev-add.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 9 +++++++--
> blockdev.c | 31 +++++++++++++++++++++++++++----
> 2 files changed, 34 insertions(+), 6 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 17/17] blockdev: blockdev_init() error conversion
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (15 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 16/17] blockdev: Don't disable COR automatically with blockdev-add Kevin Wolf
@ 2013-10-01 13:20 ` Kevin Wolf
2013-10-01 17:11 ` Eric Blake
2013-10-01 15:41 ` [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Eric Blake
17 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 13:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, armbru, mreitz, stefanha, xiawenc
This gives us meaningful error messages for the blockdev-add QMP
command.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 59 +++++++++++++++++++++++++++++++++--------------------------
1 file changed, 33 insertions(+), 26 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 4f015af..7d18ec4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -272,7 +272,7 @@ static void bdrv_put_ref_bh_schedule(BlockDriverState *bs)
qemu_bh_schedule(s->bh);
}
-static int parse_block_error_action(const char *buf, bool is_read)
+static int parse_block_error_action(const char *buf, bool is_read, Error **errp)
{
if (!strcmp(buf, "ignore")) {
return BLOCKDEV_ON_ERROR_IGNORE;
@@ -283,8 +283,8 @@ static int parse_block_error_action(const char *buf, bool is_read)
} else if (!strcmp(buf, "report")) {
return BLOCKDEV_ON_ERROR_REPORT;
} else {
- error_report("'%s' invalid %s error action",
- buf, is_read ? "read" : "write");
+ error_setg(errp, "'%s' invalid %s error action",
+ buf, is_read ? "read" : "write");
return -1;
}
}
@@ -309,7 +309,8 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
/* Takes the ownership of bs_opts */
static DriveInfo *blockdev_init(QDict *bs_opts,
- BlockInterfaceType type)
+ BlockInterfaceType type,
+ Error **errp)
{
const char *buf;
const char *file = NULL;
@@ -333,15 +334,13 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
id = qdict_get_try_str(bs_opts, "id");
opts = qemu_opts_create(&qemu_common_drive_opts, id, 1, &error);
if (error_is_set(&error)) {
- qerror_report_err(error);
- error_free(error);
+ error_propagate(errp, error);
return NULL;
}
qemu_opts_absorb_qdict(opts, bs_opts, &error);
if (error_is_set(&error)) {
- qerror_report_err(error);
- error_free(error);
+ error_propagate(errp, error);
return NULL;
}
@@ -361,7 +360,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
if ((buf = qemu_opt_get(opts, "discard")) != NULL) {
if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) {
- error_report("invalid discard option");
+ error_setg(errp, "invalid discard option");
return NULL;
}
}
@@ -383,7 +382,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
} else if (!strcmp(buf, "threads")) {
/* this is the default */
} else {
- error_report("invalid aio option");
+ error_setg(errp, "invalid aio option");
return NULL;
}
}
@@ -400,9 +399,10 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
drv = bdrv_find_whitelisted_format(buf, ro);
if (!drv) {
if (!ro && bdrv_find_whitelisted_format(buf, !ro)) {
- error_report("'%s' can be only used as read-only device.", buf);
+ error_setg(errp, "'%s' can be only used as read-only device.",
+ buf);
} else {
- error_report("'%s' invalid format", buf);
+ error_setg(errp, "'%s' invalid format", buf);
}
return NULL;
}
@@ -439,20 +439,20 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
cfg.op_size = qemu_opt_get_number(opts, "throttling.iops-size", 0);
if (!check_throttle_config(&cfg, &error)) {
- error_report("%s", error_get_pretty(error));
- error_free(error);
+ error_propagate(errp, error);
return NULL;
}
on_write_error = BLOCKDEV_ON_ERROR_ENOSPC;
if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) {
- error_report("werror is not supported by this bus type");
+ error_setg(errp, "werror is not supported by this bus type");
return NULL;
}
- on_write_error = parse_block_error_action(buf, 0);
- if (on_write_error < 0) {
+ on_write_error = parse_block_error_action(buf, 0, &error);
+ if (error_is_set(&error)) {
+ error_propagate(errp, error);
return NULL;
}
}
@@ -464,8 +464,9 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
return NULL;
}
- on_read_error = parse_block_error_action(buf, 1);
- if (on_read_error < 0) {
+ on_read_error = parse_block_error_action(buf, 1, &error);
+ if (error_is_set(&error)) {
+ error_propagate(errp, error);
return NULL;
}
}
@@ -518,8 +519,9 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
if (ret < 0) {
- error_report("could not open disk image %s: %s",
- file ?: dinfo->id, error_get_pretty(error));
+ error_setg(errp, "could not open disk image %s: %s",
+ file ?: dinfo->id, error_get_pretty(error));
+ error_free(error);
goto err;
}
@@ -866,9 +868,15 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
}
/* Actual block device init: Functionality shared with blockdev-add */
- dinfo = blockdev_init(bs_opts, type);
+ dinfo = blockdev_init(bs_opts, type, &local_err);
if (dinfo == NULL) {
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ }
goto fail;
+ } else {
+ assert(!error_is_set(&local_err));
}
/* Set legacy DriveInfo fields */
@@ -2155,7 +2163,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
QmpOutputVisitor *ov = qmp_output_visitor_new();
QObject *obj;
QDict *qdict;
- DriveInfo *dinfo;
Error *local_err = NULL;
/* Require an ID in the top level */
@@ -2189,9 +2196,9 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
qdict_flatten(qdict);
- dinfo = blockdev_init(qdict, IF_NONE);
- if (!dinfo) {
- error_setg(errp, "Could not open image");
+ blockdev_init(qdict, IF_NONE, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
goto fail;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 17/17] blockdev: blockdev_init() error conversion
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 17/17] blockdev: blockdev_init() error conversion Kevin Wolf
@ 2013-10-01 17:11 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 17:11 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> This gives us meaningful error messages for the blockdev-add QMP
> command.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> blockdev.c | 59 +++++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 33 insertions(+), 26 deletions(-)
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command
2013-10-01 13:20 [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Kevin Wolf
` (16 preceding siblings ...)
2013-10-01 13:20 ` [Qemu-devel] [PATCH v2 17/17] blockdev: blockdev_init() error conversion Kevin Wolf
@ 2013-10-01 15:41 ` Eric Blake
2013-10-01 15:50 ` Kevin Wolf
17 siblings, 1 reply; 38+ messages in thread
From: Eric Blake @ 2013-10-01 15:41 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]
On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> This series adds a blockdev-add QMP command and a row of cleanup patches to
> separate legacy -drive/drive_add behaviour from blockdev_init() to the wrapper
> function drive_init().
>
> v2:
>
> - Removed I/O throttling and copy-on-read options from the schema: Both are
> candidates for becoming block filters instead. For the time being, you can
> use the existing QMP commands in order to enable I/O throttling on a device
> created with blockdev-add in a second step.
Tolerable as a stop-gap; but doesn't that mean that there is a window
where throttling is not active? Anything that can restrict block
operations must ultimately be specified atomically up front to avoid a
non-deterministic burst during the race window. But in the interest of
incremental improvements, I can live with the approach used here, while
still waiting for throttling to be implemented as a true filter device.
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command
2013-10-01 15:41 ` [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command Eric Blake
@ 2013-10-01 15:50 ` Kevin Wolf
2013-10-01 15:54 ` Eric Blake
0 siblings, 1 reply; 38+ messages in thread
From: Kevin Wolf @ 2013-10-01 15:50 UTC (permalink / raw)
To: Eric Blake; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
Am 01.10.2013 um 17:41 hat Eric Blake geschrieben:
> On 10/01/2013 07:20 AM, Kevin Wolf wrote:
> > This series adds a blockdev-add QMP command and a row of cleanup patches to
> > separate legacy -drive/drive_add behaviour from blockdev_init() to the wrapper
> > function drive_init().
> >
> > v2:
> >
> > - Removed I/O throttling and copy-on-read options from the schema: Both are
> > candidates for becoming block filters instead. For the time being, you can
> > use the existing QMP commands in order to enable I/O throttling on a device
> > created with blockdev-add in a second step.
>
> Tolerable as a stop-gap; but doesn't that mean that there is a window
> where throttling is not active? Anything that can restrict block
> operations must ultimately be specified atomically up front to avoid a
> non-deterministic burst during the race window. But in the interest of
> incremental improvements, I can live with the approach used here, while
> still waiting for throttling to be implemented as a true filter device.
Immediately after blockdev-add, the block device is still completely
unused. You can set the throttling options before you do the
corresponding device-add or block job command or whatever you're
planning to do with the device.
Kevin
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/17] blockdev-add QMP command
2013-10-01 15:50 ` Kevin Wolf
@ 2013-10-01 15:54 ` Eric Blake
0 siblings, 0 replies; 38+ messages in thread
From: Eric Blake @ 2013-10-01 15:54 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, armbru, qemu-devel, mreitz, stefanha, xiawenc
[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]
On 10/01/2013 09:50 AM, Kevin Wolf wrote:
> Am 01.10.2013 um 17:41 hat Eric Blake geschrieben:
>> On 10/01/2013 07:20 AM, Kevin Wolf wrote:
>>> This series adds a blockdev-add QMP command and a row of cleanup patches to
>>> separate legacy -drive/drive_add behaviour from blockdev_init() to the wrapper
>>> function drive_init().
>>>
>>> v2:
>>>
>>> - Removed I/O throttling and copy-on-read options from the schema: Both are
>>> candidates for becoming block filters instead. For the time being, you can
>>> use the existing QMP commands in order to enable I/O throttling on a device
>>> created with blockdev-add in a second step.
>>
>> Tolerable as a stop-gap; but doesn't that mean that there is a window
>> where throttling is not active? Anything that can restrict block
>> operations must ultimately be specified atomically up front to avoid a
>> non-deterministic burst during the race window. But in the interest of
>> incremental improvements, I can live with the approach used here, while
>> still waiting for throttling to be implemented as a true filter device.
>
> Immediately after blockdev-add, the block device is still completely
> unused. You can set the throttling options before you do the
> corresponding device-add or block job command or whatever you're
> planning to do with the device.
Ah. So there's no race window after all. Good to know.
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread