* [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
@ 2016-02-15 14:33 Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
` (10 more replies)
0 siblings, 11 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
This series of patches expands the syntax of the qemu-img,
qemu-nbd and qemu-io commands to make them more flexible.
v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html
First all three gain a --object parameter, which allows
instantiation of user creatable object types. The immediate
use case is to allow for creation of the 'secret' object
type to pass passwords for curl, iscsi and rbd drivers.
For qemu-nbd this will also be needed to create TLS
certificates for encryption support.
Then all three gain a '--image-opts' parameter which causes
the positional filenames to be interepreted as option strings
rather tha nplain filenames. This avoids the need to use the
JSON syntax, or to add custom CLI args for each block backend
option that exists. The immediate use case is to allow the
user to specify the ID of the 'secret' object they just created.
Finally, there are a few small cleanup patches
NB, this series will conflict with my series adding TLS
support to NBD. After one of the series wins the race to
merge, I will rebase the other series. I don't mind which
merges first really, but the sooner the better considering
the forthcoming soft-freeze and that this is a dep for
several other patch series.
Changed in v6:
- Add missing docs of --image-opts for qemu-nbd man page (Eric)
- Remove left over special casing of 'file' in QemuOpts (Kevin)
- Pass NULL to getopt_long when optionindex is not needed (Eric)
- Misc typos in qemu-img docs (Eric)
- Fix qemu-io logic error checking argv/argc opt bounds (Eric)
- Ensure 'id' is passed into blk_open in qemu-img (Kevin)
- Share code for doing block dev passwd prompting
Changed in v5:
- Move more common object creation code into qom/ (Kevin)
- Add missing @var{} syntax in CLI help definition (Kevin)
- Declare QemuOpts closer to time of use (Kevin)
- Directly reference registered opts instead of calling
qemu_find_opts (Kevin)
- Use consistent exit/return/goto pattern in qemu-img (Kevin)
- Remove special casing of 'file' in QemuOpts handling
for bdrv_open (Kevin)
- Split file file opening code out into separate method
(Kevin)
Changed in v4:
- Fix error reporting when object_create fails
Changed in v3:
- Rebase to resolve with conflicts against recently
merged code
- Remove use of errx()
Changed in v2:
- Share more common code in qom/object_interfaces.c to
avoid duplicating so much of 'object_create' in each
command
- Remove previously added '--source optstring' parameter
which replaced the positional filenames, in favour of
keeping the positional filenames but using a --image-opts
boolean arg to change their interpretation
- Added docs for --image-opts to qemu-img man page
- Use printf instead of echo -n in examples
- Line wrap help string based on user terminal width not
source code width
- Update qemu-nbd/qemu-io to use constants for options
- Update qemu-nbd to avoid overlapping option values
Daniel P. Berrange (10):
qom: add helpers for UserCreatable object types
qemu-io: add support for --object command line arg
qemu-nbd: add support for --object command line arg
qemu-img: add support for --object command line arg
qemu-io: allow specifying image as a set of options args
qemu-nbd: allow specifying image as a set of options args
qemu-img: allow specifying image as a set of options args
qemu-nbd: don't overlap long option values with short options
qemu-nbd: use no_argument/required_argument constants
qemu-io: use no_argument/required_argument constants
hmp.c | 52 +----
include/monitor/monitor.h | 3 -
include/qom/object_interfaces.h | 92 ++++++++
qemu-img-cmds.hx | 44 ++--
qemu-img.c | 466 ++++++++++++++++++++++++++++++++++++----
qemu-img.texi | 14 ++
qemu-io.c | 116 ++++++++--
qemu-nbd.c | 128 ++++++++---
qemu-nbd.texi | 13 +-
qmp.c | 76 +------
qom/object_interfaces.c | 174 +++++++++++++++
vl.c | 68 +-----
12 files changed, 954 insertions(+), 292 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-04-27 9:26 ` Markus Armbruster
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 02/10] qemu-io: add support for --object command line arg Daniel P. Berrange
` (9 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
The QMP monitor code has two helper methods object_add
and qmp_object_del that are called from several places
in the code (QMP, HMP and main emulator startup).
The HMP and main emulator startup code also share
further logic that extracts the qom-type & id
values from a qdict.
We soon need to use this logic from qemu-img, qemu-io
and qemu-nbd too, but don't want those to depend on
the monitor, nor do we want to duplicate the code.
To avoid this, move some code out of qmp.c and hmp.c
adding new methods to qom/object_interfaces.c
- user_creatable_add - takes a QDict holding a full
object definition & instantiates it
- user_creatable_add_type - takes an ID, type name,
and QDict holding object properties & instantiates
it
- user_creatable_add_opts - takes a QemuOpts holding
a full object definition & instantiates it
- user_creatable_add_opts_foreach - variant on
user_creatable_add_opts which can be directly used
in conjunction with qemu_opts_foreach.
- user_creatable_del - takes an ID and deletes the
corresponding object
The existing code is updated to use these new methods.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
hmp.c | 52 +++---------
include/monitor/monitor.h | 3 -
include/qom/object_interfaces.h | 92 +++++++++++++++++++++
qmp.c | 76 ++----------------
qom/object_interfaces.c | 174 ++++++++++++++++++++++++++++++++++++++++
vl.c | 68 ++--------------
6 files changed, 290 insertions(+), 175 deletions(-)
diff --git a/hmp.c b/hmp.c
index c6419da..41fb9ca 100644
--- a/hmp.c
+++ b/hmp.c
@@ -30,6 +30,7 @@
#include "qapi/string-output-visitor.h"
#include "qapi/util.h"
#include "qapi-visit.h"
+#include "qom/object_interfaces.h"
#include "ui/console.h"
#include "block/qapi.h"
#include "qemu-io.h"
@@ -1655,58 +1656,27 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
- Error *err_end = NULL;
QemuOpts *opts;
- char *type = NULL;
- char *id = NULL;
OptsVisitor *ov;
- QDict *pdict;
- Visitor *v;
+ Object *obj = NULL;
opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
if (err) {
- goto out;
+ hmp_handle_error(mon, &err);
+ return;
}
ov = opts_visitor_new(opts);
- pdict = qdict_clone_shallow(qdict);
- v = opts_get_visitor(ov);
-
- visit_start_struct(v, NULL, NULL, 0, &err);
- if (err) {
- goto out_clean;
- }
-
- qdict_del(pdict, "qom-type");
- visit_type_str(v, "qom-type", &type, &err);
- if (err) {
- goto out_end;
- }
+ obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
+ opts_visitor_cleanup(ov);
+ qemu_opts_del(opts);
- qdict_del(pdict, "id");
- visit_type_str(v, "id", &id, &err);
if (err) {
- goto out_end;
+ hmp_handle_error(mon, &err);
}
-
- object_add(type, id, pdict, v, &err);
-
-out_end:
- visit_end_struct(v, &err_end);
- if (!err && err_end) {
- qmp_object_del(id, NULL);
+ if (obj) {
+ object_unref(obj);
}
- error_propagate(&err, err_end);
-out_clean:
- opts_visitor_cleanup(ov);
-
- QDECREF(pdict);
- qemu_opts_del(opts);
- g_free(id);
- g_free(type);
-
-out:
- hmp_handle_error(mon, &err);
}
void hmp_getfd(Monitor *mon, const QDict *qdict)
@@ -1934,7 +1904,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
const char *id = qdict_get_str(qdict, "id");
Error *err = NULL;
- qmp_object_del(id, &err);
+ user_creatable_del(id, &err);
hmp_handle_error(mon, &err);
}
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 91b95ae..aa0f373 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -43,9 +43,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque);
-void object_add(const char *type, const char *id, const QDict *qdict,
- Visitor *v, Error **errp);
-
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
bool has_opaque, const char *opaque,
Error **errp);
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 283ae0d..d579746 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -2,6 +2,8 @@
#define OBJECT_INTERFACES_H
#include "qom/object.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/visitor.h"
#define TYPE_USER_CREATABLE "user-creatable"
@@ -72,4 +74,94 @@ void user_creatable_complete(Object *obj, Error **errp);
* from implements USER_CREATABLE interface.
*/
bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
+
+/**
+ * user_creatable_add:
+ * @qdict: the object definition
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object whose type
+ * is defined in @qdict by the 'qom-type' field, placing it
+ * in the object composition tree with name provided by the
+ * 'id' field. The remaining fields in @qdict are used to
+ * initialize the object properties.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add(const QDict *qdict,
+ Visitor *v, Error **errp);
+
+/**
+ * user_creatable_add_type:
+ * @type: the object type name
+ * @id: the unique ID for the object
+ * @qdict: the object properties
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object @type, placing
+ * it in the object composition tree with name @id, initializing
+ * it with properties from @qdict
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add_type(const char *type, const char *id,
+ const QDict *qdict,
+ Visitor *v, Error **errp);
+
+/**
+ * user_creatable_add_opts:
+ * @opts: the object definition
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object whose type
+ * is defined in @opts by the 'qom-type' option, placing it
+ * in the object composition tree with name provided by the
+ * 'id' field. The remaining options in @opts are used to
+ * initialize the object properties.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
+
+
+/**
+ * user_creatable_add_opts_predicate:
+ * @type: the QOM type to be added
+ *
+ * A callback function to determine whether an object
+ * of type @type should be created. Instances of this
+ * callback should be passed to user_creatable_add_opts_foreach
+ */
+typedef bool (*user_creatable_add_opts_predicate)(const char *type);
+
+/**
+ * user_creatable_add_opts_foreach:
+ * @opaque: a user_creatable_add_opts_predicate callback or NULL
+ * @opts: options to create
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * An iterator callback to be used in conjunction with
+ * the qemu_opts_foreach() method for creating a list of
+ * objects from a set of QemuOpts
+ *
+ * The @opaque parameter can be passed a user_creatable_add_opts_predicate
+ * callback to filter which types of object are created during iteration.
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int user_creatable_add_opts_foreach(void *opaque,
+ QemuOpts *opts, Error **errp);
+
+/**
+ * user_creatable_del:
+ * @id: the unique ID for the object
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Delete an instance of the user creatable object identified
+ * by @id.
+ */
+void user_creatable_del(const char *id, Error **errp);
+
#endif
diff --git a/qmp.c b/qmp.c
index 6ae7230..9a93d5e 100644
--- a/qmp.c
+++ b/qmp.c
@@ -633,65 +633,13 @@ void qmp_add_client(const char *protocol, const char *fdname,
close(fd);
}
-void object_add(const char *type, const char *id, const QDict *qdict,
- Visitor *v, Error **errp)
-{
- Object *obj;
- ObjectClass *klass;
- const QDictEntry *e;
- Error *local_err = NULL;
-
- klass = object_class_by_name(type);
- if (!klass) {
- error_setg(errp, "invalid object type: %s", type);
- return;
- }
-
- if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
- error_setg(errp, "object type '%s' isn't supported by object-add",
- type);
- return;
- }
-
- if (object_class_is_abstract(klass)) {
- error_setg(errp, "object type '%s' is abstract", type);
- return;
- }
-
- obj = object_new(type);
- if (qdict) {
- for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
- object_property_set(obj, v, e->key, &local_err);
- if (local_err) {
- goto out;
- }
- }
- }
-
- object_property_add_child(object_get_objects_root(),
- id, obj, &local_err);
- if (local_err) {
- goto out;
- }
-
- user_creatable_complete(obj, &local_err);
- if (local_err) {
- object_property_del(object_get_objects_root(),
- id, &error_abort);
- goto out;
- }
-out:
- if (local_err) {
- error_propagate(errp, local_err);
- }
- object_unref(obj);
-}
void qmp_object_add(const char *type, const char *id,
bool has_props, QObject *props, Error **errp)
{
const QDict *pdict = NULL;
QmpInputVisitor *qiv;
+ Object *obj;
if (props) {
pdict = qobject_to_qdict(props);
@@ -702,27 +650,17 @@ void qmp_object_add(const char *type, const char *id,
}
qiv = qmp_input_visitor_new(props);
- object_add(type, id, pdict, qmp_input_get_visitor(qiv), errp);
+ obj = user_creatable_add_type(type, id, pdict,
+ qmp_input_get_visitor(qiv), errp);
qmp_input_visitor_cleanup(qiv);
+ if (obj) {
+ object_unref(obj);
+ }
}
void qmp_object_del(const char *id, Error **errp)
{
- Object *container;
- Object *obj;
-
- container = object_get_objects_root();
- obj = object_resolve_path_component(container, id);
- if (!obj) {
- error_setg(errp, "object id not found");
- return;
- }
-
- if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
- error_setg(errp, "%s is in use, can not be deleted", id);
- return;
- }
- object_unparent(obj);
+ user_creatable_del(id, errp);
}
MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index f1218f0..c2f6e29 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -1,6 +1,9 @@
#include "qemu/osdep.h"
#include "qom/object_interfaces.h"
#include "qemu/module.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/opts-visitor.h"
void user_creatable_complete(Object *obj, Error **errp)
{
@@ -31,6 +34,177 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp)
}
}
+
+Object *user_creatable_add(const QDict *qdict,
+ Visitor *v, Error **errp)
+{
+ char *type = NULL;
+ char *id = NULL;
+ Object *obj = NULL;
+ Error *local_err = NULL, *end_err = NULL;
+ QDict *pdict;
+
+ pdict = qdict_clone_shallow(qdict);
+
+ visit_start_struct(v, NULL, NULL, 0, &local_err);
+ if (local_err) {
+ goto out;
+ }
+
+ qdict_del(pdict, "qom-type");
+ visit_type_str(v, "qom-type", &type, &local_err);
+ if (local_err) {
+ goto out_visit;
+ }
+
+ qdict_del(pdict, "id");
+ visit_type_str(v, "id", &id, &local_err);
+ if (local_err) {
+ goto out_visit;
+ }
+
+ obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+ if (local_err) {
+ goto out_visit;
+ }
+
+ out_visit:
+ visit_end_struct(v, &end_err);
+ if (end_err) {
+ error_propagate(&local_err, end_err);
+ if (obj) {
+ user_creatable_del(id, NULL);
+ }
+ goto out;
+ }
+
+out:
+ QDECREF(pdict);
+ g_free(id);
+ g_free(type);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ object_unref(obj);
+ return NULL;
+ }
+ return obj;
+}
+
+
+Object *user_creatable_add_type(const char *type, const char *id,
+ const QDict *qdict,
+ Visitor *v, Error **errp)
+{
+ Object *obj;
+ ObjectClass *klass;
+ const QDictEntry *e;
+ Error *local_err = NULL;
+
+ klass = object_class_by_name(type);
+ if (!klass) {
+ error_setg(errp, "invalid object type: %s", type);
+ return NULL;
+ }
+
+ if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
+ error_setg(errp, "object type '%s' isn't supported by object-add",
+ type);
+ return NULL;
+ }
+
+ if (object_class_is_abstract(klass)) {
+ error_setg(errp, "object type '%s' is abstract", type);
+ return NULL;
+ }
+
+ obj = object_new(type);
+ if (qdict) {
+ for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+ object_property_set(obj, v, e->key, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ }
+ }
+
+ object_property_add_child(object_get_objects_root(),
+ id, obj, &local_err);
+ if (local_err) {
+ goto out;
+ }
+
+ user_creatable_complete(obj, &local_err);
+ if (local_err) {
+ object_property_del(object_get_objects_root(),
+ id, &error_abort);
+ goto out;
+ }
+out:
+ if (local_err) {
+ error_propagate(errp, local_err);
+ object_unref(obj);
+ return NULL;
+ }
+ return obj;
+}
+
+
+Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
+{
+ OptsVisitor *ov;
+ QDict *pdict;
+ Object *obj = NULL;
+
+ ov = opts_visitor_new(opts);
+ pdict = qemu_opts_to_qdict(opts, NULL);
+
+ obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
+ opts_visitor_cleanup(ov);
+ QDECREF(pdict);
+ return obj;
+}
+
+
+int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
+{
+ bool (*type_predicate)(const char *) = opaque;
+ Object *obj = NULL;
+ const char *type;
+
+ type = qemu_opt_get(opts, "qom-type");
+ if (type && type_predicate &&
+ !type_predicate(type)) {
+ return 0;
+ }
+
+ obj = user_creatable_add_opts(opts, errp);
+ if (!obj) {
+ return -1;
+ }
+ object_unref(obj);
+ return 0;
+}
+
+
+void user_creatable_del(const char *id, Error **errp)
+{
+ Object *container;
+ Object *obj;
+
+ container = object_get_objects_root();
+ obj = object_resolve_path_component(container, id);
+ if (!obj) {
+ error_setg(errp, "object '%s' not found", id);
+ return;
+ }
+
+ if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
+ error_setg(errp, "object '%s' is in use, can not be deleted", id);
+ return;
+ }
+ object_unparent(obj);
+}
+
static void register_types(void)
{
static const TypeInfo uc_interface_info = {
diff --git a/vl.c b/vl.c
index 175ebcc..9bd881e 100644
--- a/vl.c
+++ b/vl.c
@@ -2816,64 +2816,6 @@ static bool object_create_delayed(const char *type)
}
-static int object_create(void *opaque, QemuOpts *opts, Error **errp)
-{
- Error *err = NULL;
- Error *err_end = NULL;
- char *type = NULL;
- char *id = NULL;
- OptsVisitor *ov;
- QDict *pdict;
- bool (*type_predicate)(const char *) = opaque;
- Visitor *v;
-
- ov = opts_visitor_new(opts);
- pdict = qemu_opts_to_qdict(opts, NULL);
- v = opts_get_visitor(ov);
-
- visit_start_struct(v, NULL, NULL, 0, &err);
- if (err) {
- goto out;
- }
-
- qdict_del(pdict, "qom-type");
- visit_type_str(v, "qom-type", &type, &err);
- if (err) {
- goto out;
- }
- if (!type_predicate(type)) {
- visit_end_struct(v, NULL);
- goto out;
- }
-
- qdict_del(pdict, "id");
- visit_type_str(v, "id", &id, &err);
- if (err) {
- goto out_end;
- }
-
- object_add(type, id, pdict, v, &err);
-
-out_end:
- visit_end_struct(v, &err_end);
- if (!err && err_end) {
- qmp_object_del(id, NULL);
- }
- error_propagate(&err, err_end);
-
-out:
- opts_visitor_cleanup(ov);
-
- QDECREF(pdict);
- g_free(id);
- g_free(type);
- if (err) {
- error_report_err(err);
- return -1;
- }
- return 0;
-}
-
static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
MachineClass *mc)
{
@@ -4299,8 +4241,9 @@ int main(int argc, char **argv, char **envp)
socket_init();
if (qemu_opts_foreach(qemu_find_opts("object"),
- object_create,
- object_create_initial, NULL)) {
+ user_creatable_add_opts_foreach,
+ object_create_initial, &err)) {
+ error_report_err(err);
exit(1);
}
@@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
}
if (qemu_opts_foreach(qemu_find_opts("object"),
- object_create,
- object_create_delayed, NULL)) {
+ user_creatable_add_opts_foreach,
+ object_create_delayed, &err)) {
+ error_report_err(err);
exit(1);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 02/10] qemu-io: add support for --object command line arg
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 03/10] qemu-nbd: " Daniel P. Berrange
` (8 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
Allow creation of user creatable object types with qemu-io
via a new --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# printf letmein > mypasswd.txt
# qemu-io --object secret,id=sec0,file=mypasswd.txt \
...other args...
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-io.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/qemu-io.c b/qemu-io.c
index 83c48f4..969c8bf 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -18,6 +18,7 @@
#include "qemu/config-file.h"
#include "qemu/readline.h"
#include "qapi/qmp/qstring.h"
+#include "qom/object_interfaces.h"
#include "sysemu/block-backend.h"
#include "block/block_int.h"
#include "trace/control.h"
@@ -200,6 +201,8 @@ static void usage(const char *name)
"Usage: %s [-h] [-V] [-rsnm] [-f FMT] [-c STRING] ... [file]\n"
"QEMU Disk exerciser\n"
"\n"
+" --object OBJECTDEF define an object such as 'secret' for\n"
+" passwords and/or encryption keys\n"
" -c, --cmd STRING execute command with its arguments\n"
" from the given string\n"
" -f, --format FMT specifies the block driver to use\n"
@@ -361,6 +364,20 @@ static void reenable_tty_echo(void)
qemu_set_tty_echo(STDIN_FILENO, true);
}
+enum {
+ OPTION_OBJECT = 256,
+};
+
+static QemuOptsList qemu_object_opts = {
+ .name = "object",
+ .implied_opt_name = "qom-type",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
+ .desc = {
+ { }
+ },
+};
+
+
int main(int argc, char **argv)
{
int readonly = 0;
@@ -379,6 +396,7 @@ int main(int argc, char **argv)
{ "discard", 1, NULL, 'd' },
{ "cache", 1, NULL, 't' },
{ "trace", 1, NULL, 'T' },
+ { "object", 1, NULL, OPTION_OBJECT },
{ NULL, 0, NULL, 0 }
};
int c;
@@ -394,6 +412,8 @@ int main(int argc, char **argv)
progname = basename(argv[0]);
qemu_init_exec_dir(argv[0]);
+ module_call_init(MODULE_INIT_QOM);
+ qemu_add_opts(&qemu_object_opts);
bdrv_init();
while ((c = getopt_long(argc, argv, sopt, lopt, &opt_index)) != -1) {
@@ -445,6 +465,14 @@ int main(int argc, char **argv)
case 'h':
usage(progname);
exit(0);
+ case OPTION_OBJECT: {
+ QemuOpts *qopts = NULL;
+ qopts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!qopts) {
+ exit(1);
+ }
+ } break;
default:
usage(progname);
exit(1);
@@ -461,6 +489,13 @@ int main(int argc, char **argv)
exit(1);
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_error)) {
+ error_report_err(local_error);
+ exit(1);
+ }
+
/* initialize commands */
qemuio_add_command(&quit_cmd);
qemuio_add_command(&open_cmd);
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 03/10] qemu-nbd: add support for --object command line arg
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 02/10] qemu-io: add support for --object command line arg Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 04/10] qemu-img: " Daniel P. Berrange
` (7 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
Allow creation of user creatable object types with qemu-nbd
via a new --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# printf letmein > mypasswd.txt
# qemu-nbd --object secret,id=sec0,file=mypasswd.txt \
...other nbd args...
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-nbd.c | 34 ++++++++++++++++++++++++++++++++++
qemu-nbd.texi | 6 ++++++
2 files changed, 40 insertions(+)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d374015..130c306 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -24,9 +24,11 @@
#include "qemu/main-loop.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
+#include "qemu/config-file.h"
#include "block/snapshot.h"
#include "qapi/util.h"
#include "qapi/qmp/qstring.h"
+#include "qom/object_interfaces.h"
#include <getopt.h>
#include <sys/socket.h>
@@ -41,6 +43,7 @@
#define QEMU_NBD_OPT_AIO 2
#define QEMU_NBD_OPT_DISCARD 3
#define QEMU_NBD_OPT_DETECT_ZEROES 4
+#define QEMU_NBD_OPT_OBJECT 5
static NBDExport *exp;
static int verbose;
@@ -74,6 +77,9 @@ static void usage(const char *name)
" -o, --offset=OFFSET offset into the image\n"
" -P, --partition=NUM only expose partition NUM\n"
"\n"
+"General purpose options:\n"
+" --object type,id=ID,... define an object such as 'secret' for providing\n"
+" passwords and/or encryption keys\n"
#ifdef __linux__
"Kernel NBD client support:\n"
" -c, --connect=DEV connect FILE to the local NBD device DEV\n"
@@ -371,6 +377,16 @@ static SocketAddress *nbd_build_socket_address(const char *sockpath,
}
+static QemuOptsList qemu_object_opts = {
+ .name = "object",
+ .implied_opt_name = "qom-type",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
+ .desc = {
+ { }
+ },
+};
+
+
int main(int argc, char **argv)
{
BlockBackend *blk;
@@ -408,6 +424,7 @@ int main(int argc, char **argv)
{ "format", 1, NULL, 'f' },
{ "persistent", 0, NULL, 't' },
{ "verbose", 0, NULL, 'v' },
+ { "object", 1, NULL, QEMU_NBD_OPT_OBJECT },
{ NULL, 0, NULL, 0 }
};
int ch;
@@ -433,6 +450,8 @@ int main(int argc, char **argv)
memset(&sa_sigterm, 0, sizeof(sa_sigterm));
sa_sigterm.sa_handler = termsig_handler;
sigaction(SIGTERM, &sa_sigterm, NULL);
+ module_call_init(MODULE_INIT_QOM);
+ qemu_add_opts(&qemu_object_opts);
qemu_init_exec_dir(argv[0]);
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
@@ -588,6 +607,14 @@ int main(int argc, char **argv)
case '?':
error_report("Try `%s --help' for more information.", argv[0]);
exit(EXIT_FAILURE);
+ case QEMU_NBD_OPT_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ exit(EXIT_FAILURE);
+ }
+ } break;
}
}
@@ -597,6 +624,13 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ exit(EXIT_FAILURE);
+ }
+
if (disconnect) {
fd = open(argv[optind], O_RDWR);
if (fd < 0) {
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 0027841..a56ebc3 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -18,6 +18,12 @@ Export a QEMU disk image using the NBD protocol.
@var{dev} is an NBD device.
@table @option
+@item --object type,id=@var{id},...props...
+Define a new instance of the @var{type} object class identified by @var{id}.
+See the @code{qemu(1)} manual page for full details of the properties
+supported. The common object type that it makes sense to define is the
+@code{secret} object, which is used to supply passwords and/or encryption
+keys.
@item -p, --port=@var{port}
The TCP port to listen on (default @samp{10809})
@item -o, --offset=@var{offset}
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 04/10] qemu-img: add support for --object command line arg
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (2 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 03/10] qemu-nbd: " Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
` (6 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
Allow creation of user creatable object types with qemu-img
via a new --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# printf letmein > mypasswd.txt
# qemu-img info --object secret,id=sec0,file=mypasswd.txt \
...other info args...
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-img-cmds.hx | 44 +++++-----
qemu-img.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
qemu-img.texi | 8 ++
3 files changed, 283 insertions(+), 30 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 9567774..0eaf307 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,68 +10,68 @@ STEXI
ETEXI
DEF("check", img_check,
- "check [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
+ "check [-q] [--object objectdef] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
STEXI
-@item check [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
+@item check [--object @var{objectdef}] [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
ETEXI
DEF("create", img_create,
- "create [-q] [-f fmt] [-o options] filename [size]")
+ "create [-q] [--object objectdef] [-f fmt] [-o options] filename [size]")
STEXI
-@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
ETEXI
DEF("commit", img_commit,
- "commit [-q] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
+ "commit [-q] [--object objectdef] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
+@item commit [--object @var{objectdef}] [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
ETEXI
DEF("compare", img_compare,
- "compare [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2")
+ "compare [--object objectdef] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2")
STEXI
-@item compare [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+@item compare [--object @var{objectdef}] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2}
ETEXI
DEF("convert", img_convert,
- "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
+ "convert [--object objectdef] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
STEXI
-@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("info", img_info,
- "info [-f fmt] [--output=ofmt] [--backing-chain] filename")
+ "info [--object objectdef] [-f fmt] [--output=ofmt] [--backing-chain] filename")
STEXI
-@item info [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
+@item info [--object @var{objectdef}] [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
ETEXI
DEF("map", img_map,
- "map [-f fmt] [--output=ofmt] filename")
+ "map [--object objectdef] [-f fmt] [--output=ofmt] filename")
STEXI
-@item map [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item map [--object @var{objectdef}] [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
ETEXI
DEF("snapshot", img_snapshot,
- "snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+ "snapshot [--object objectdef] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
STEXI
-@item snapshot [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [--object @var{objectdef}] [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
ETEXI
DEF("rebase", img_rebase,
- "rebase [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+ "rebase [--object objectdef] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
STEXI
-@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [--object @var{objectdef}] [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
ETEXI
DEF("resize", img_resize,
- "resize [-q] filename [+ | -]size")
+ "resize [--object objectdef] [-q] filename [+ | -]size")
STEXI
-@item resize [-q] @var{filename} [+ | -]@var{size}
+@item resize [--object @var{objectdef}] [-q] @var{filename} [+ | -]@var{size}
ETEXI
DEF("amend", img_amend,
- "amend [-p] [-q] [-f fmt] [-t cache] -o options filename")
+ "amend [--object objectdef] [-p] [-q] [-f fmt] [-t cache] -o options filename")
STEXI
-@item amend [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [--object @var{objectdef}] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
@end table
ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 163d8c1..5b8c7a3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -27,8 +27,10 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
#include "qemu-common.h"
+#include "qemu/config-file.h"
#include "qemu/option.h"
#include "qemu/error-report.h"
+#include "qom/object_interfaces.h"
#include "sysemu/sysemu.h"
#include "sysemu/block-backend.h"
#include "block/block_int.h"
@@ -47,6 +49,7 @@ typedef struct img_cmd_t {
enum {
OPTION_OUTPUT = 256,
OPTION_BACKING_CHAIN = 257,
+ OPTION_OBJECT = 258,
};
typedef enum OutputFormat {
@@ -94,6 +97,10 @@ static void QEMU_NORETURN help(void)
"\n"
"Command parameters:\n"
" 'filename' is a disk image filename\n"
+ " 'objectdef' is a QEMU user creatable object definition. See the qemu(1)\n"
+ " manual page for a description of the object properties. The most common\n"
+ " object type is a 'secret', which is used to supply passwords and/or\n"
+ " encryption keys.\n"
" 'fmt' is the disk image format. It is guessed automatically in most cases\n"
" 'cache' is the cache mode used to write the output disk image, the valid\n"
" options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n"
@@ -154,6 +161,15 @@ static void QEMU_NORETURN help(void)
exit(EXIT_SUCCESS);
}
+static QemuOptsList qemu_object_opts = {
+ .name = "object",
+ .implied_opt_name = "qom-type",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
+ .desc = {
+ { }
+ },
+};
+
static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
{
int ret = 0;
@@ -275,7 +291,13 @@ static int img_create(int argc, char **argv)
bool quiet = false;
for(;;) {
- c = getopt(argc, argv, "F:b:f:he6o:q");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "F:b:f:he6o:q",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -317,6 +339,14 @@ static int img_create(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ goto fail;
+ }
+ } break;
}
}
@@ -332,6 +362,13 @@ static int img_create(int argc, char **argv)
}
optind++;
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ goto fail;
+ }
+
/* Get image size, if specified */
if (optind < argc) {
int64_t sval;
@@ -489,6 +526,7 @@ static int img_check(int argc, char **argv)
int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
ImageCheck *check;
bool quiet = false;
+ Error *local_err = NULL;
fmt = NULL;
output = NULL;
@@ -500,6 +538,7 @@ static int img_check(int argc, char **argv)
{"format", required_argument, 0, 'f'},
{"repair", required_argument, 0, 'r'},
{"output", required_argument, 0, OPTION_OUTPUT},
+ {"object", required_argument, 0, OPTION_OBJECT},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "hf:r:T:q",
@@ -536,6 +575,14 @@ static int img_check(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
if (optind != argc - 1) {
@@ -552,6 +599,13 @@ static int img_check(int argc, char **argv)
return 1;
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ return 1;
+ }
+
ret = bdrv_parse_cache_flags(cache, &flags);
if (ret < 0) {
error_report("Invalid source cache option: %s", cache);
@@ -675,7 +729,13 @@ static int img_commit(int argc, char **argv)
cache = BDRV_DEFAULT_CACHE;
base = NULL;
for(;;) {
- c = getopt(argc, argv, "f:ht:b:dpq");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "f:ht:b:dpq",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -704,6 +764,14 @@ static int img_commit(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
@@ -717,6 +785,13 @@ static int img_commit(int argc, char **argv)
}
filename = argv[optind++];
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ return 1;
+ }
+
flags = BDRV_O_RDWR | BDRV_O_UNMAP;
ret = bdrv_parse_cache_flags(cache, &flags);
if (ret < 0) {
@@ -973,10 +1048,17 @@ static int img_compare(int argc, char **argv)
int64_t nb_sectors;
int c, pnum;
uint64_t progress_base;
+ Error *local_err = NULL;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
- c = getopt(argc, argv, "hf:F:T:pqs");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "hf:F:T:pqs",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -1003,6 +1085,15 @@ static int img_compare(int argc, char **argv)
case 's':
strict = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ ret = 2;
+ goto out4;
+ }
+ } break;
}
}
@@ -1018,6 +1109,14 @@ static int img_compare(int argc, char **argv)
filename1 = argv[optind++];
filename2 = argv[optind++];
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ ret = 2;
+ goto out4;
+ }
+
/* Initialize before goto out */
qemu_progress_init(progress, 2.0);
@@ -1225,6 +1324,7 @@ out2:
blk_unref(blk1);
out3:
qemu_progress_end();
+out4:
return ret;
}
@@ -1555,7 +1655,13 @@ static int img_convert(int argc, char **argv)
compress = 0;
skip_create = 0;
for(;;) {
- c = getopt(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -1646,9 +1752,23 @@ static int img_convert(int argc, char **argv)
case 'n':
skip_create = 1;
break;
+ case OPTION_OBJECT:
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ goto fail_getopt;
+ }
+ break;
}
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ goto fail_getopt;
+ }
+
/* Initialize before goto out */
if (quiet) {
progress = 0;
@@ -2077,6 +2197,7 @@ static int img_info(int argc, char **argv)
bool chain = false;
const char *filename, *fmt, *output;
ImageInfoList *list;
+ Error *local_err = NULL;
fmt = NULL;
output = NULL;
@@ -2087,6 +2208,7 @@ static int img_info(int argc, char **argv)
{"format", required_argument, 0, 'f'},
{"output", required_argument, 0, OPTION_OUTPUT},
{"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
+ {"object", required_argument, 0, OPTION_OBJECT},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:h",
@@ -2108,6 +2230,14 @@ static int img_info(int argc, char **argv)
case OPTION_BACKING_CHAIN:
chain = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
if (optind != argc - 1) {
@@ -2124,6 +2254,13 @@ static int img_info(int argc, char **argv)
return 1;
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ return 1;
+ }
+
list = collect_image_info_list(filename, fmt, chain);
if (!list) {
return 1;
@@ -2265,6 +2402,7 @@ static int img_map(int argc, char **argv)
int64_t length;
MapEntry curr = { .length = 0 }, next;
int ret = 0;
+ Error *local_err = NULL;
fmt = NULL;
output = NULL;
@@ -2274,6 +2412,7 @@ static int img_map(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"format", required_argument, 0, 'f'},
{"output", required_argument, 0, OPTION_OUTPUT},
+ {"object", required_argument, 0, OPTION_OBJECT},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:h",
@@ -2292,6 +2431,14 @@ static int img_map(int argc, char **argv)
case OPTION_OUTPUT:
output = optarg;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
if (optind != argc - 1) {
@@ -2308,6 +2455,13 @@ static int img_map(int argc, char **argv)
return 1;
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ return 1;
+ }
+
blk = img_open("image", filename, fmt, BDRV_O_FLAGS, true, false);
if (!blk) {
return 1;
@@ -2374,7 +2528,13 @@ static int img_snapshot(int argc, char **argv)
bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
/* Parse commandline parameters */
for(;;) {
- c = getopt(argc, argv, "la:c:d:hq");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "la:c:d:hq",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -2418,6 +2578,14 @@ static int img_snapshot(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
@@ -2426,6 +2594,13 @@ static int img_snapshot(int argc, char **argv)
}
filename = argv[optind++];
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &err)) {
+ error_report_err(err);
+ return 1;
+ }
+
/* Open the image */
blk = img_open("image", filename, NULL, bdrv_oflags, true, quiet);
if (!blk) {
@@ -2499,7 +2674,13 @@ static int img_rebase(int argc, char **argv)
out_baseimg = NULL;
out_basefmt = NULL;
for(;;) {
- c = getopt(argc, argv, "hf:F:b:upt:T:q");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "hf:F:b:upt:T:q",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -2532,6 +2713,14 @@ static int img_rebase(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
@@ -2547,6 +2736,13 @@ static int img_rebase(int argc, char **argv)
}
filename = argv[optind++];
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ return 1;
+ }
+
qemu_progress_init(progress, 2.0);
qemu_progress_print(0, 100);
@@ -2807,6 +3003,8 @@ static int img_resize(int argc, char **argv)
bool quiet = false;
BlockBackend *blk = NULL;
QemuOpts *param;
+ Error *local_err = NULL;
+
static QemuOptsList resize_options = {
.name = "resize_options",
.head = QTAILQ_HEAD_INITIALIZER(resize_options.head),
@@ -2833,7 +3031,13 @@ static int img_resize(int argc, char **argv)
/* Parse getopt arguments */
fmt = NULL;
for(;;) {
- c = getopt(argc, argv, "f:hq");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "f:hq",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -2848,6 +3052,14 @@ static int img_resize(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT: {
+ QemuOpts *opts;
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ return 1;
+ }
+ } break;
}
}
if (optind != argc - 1) {
@@ -2855,6 +3067,13 @@ static int img_resize(int argc, char **argv)
}
filename = argv[optind++];
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ return 1;
+ }
+
/* Choose grow, shrink, or absolute resize mode */
switch (size[0]) {
case '+':
@@ -2942,10 +3161,17 @@ static int img_amend(int argc, char **argv)
bool quiet = false, progress = false;
BlockBackend *blk = NULL;
BlockDriverState *bs = NULL;
+ Error *local_err = NULL;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
- c = getopt(argc, argv, "ho:f:t:pq");
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "ho:f:t:pq",
+ long_options, NULL);
if (c == -1) {
break;
}
@@ -2981,6 +3207,14 @@ static int img_amend(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case OPTION_OBJECT:
+ opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!opts) {
+ ret = -1;
+ goto out_no_progress;
+ }
+ break;
}
}
@@ -2988,6 +3222,14 @@ static int img_amend(int argc, char **argv)
error_exit("Must specify options (-o)");
}
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &local_err)) {
+ error_report_err(local_err);
+ ret = -1;
+ goto out_no_progress;
+ }
+
if (quiet) {
progress = false;
}
@@ -3104,12 +3346,15 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ module_call_init(MODULE_INIT_QOM);
bdrv_init();
if (argc < 2) {
error_exit("Not enough arguments");
}
cmdname = argv[1];
+ qemu_add_opts(&qemu_object_opts);
+
/* find the command */
for (cmd = img_cmds; cmd->name != NULL; cmd++) {
if (!strcmp(cmdname, cmd->name)) {
diff --git a/qemu-img.texi b/qemu-img.texi
index 7163a10..9e47543 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -24,6 +24,14 @@ Command parameters:
@table @var
@item filename
is a disk image filename
+
+@item --object @var{objectdef}
+
+is a QEMU user creatable object definition. See the @code{qemu(1)} manual
+page for a description of the object properties. The most common object
+type is a @code{secret}, which is used to supply passwords and/or encryption
+keys.
+
@item fmt
is the disk image format. It is guessed automatically in most cases. See below
for a description of the supported disk formats.
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 05/10] qemu-io: allow specifying image as a set of options args
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (3 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 04/10] qemu-img: " Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 06/10] qemu-nbd: " Daniel P. Berrange
` (5 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
Currently qemu-io allows an image filename to be passed on the
command line, but unless using the JSON format, it does not have
a way to set any options except the format eg
qemu-io https://127.0.0.1/images/centos7.iso
qemu-io /home/berrange/demo.qcow2
By contrast when using the interactive shell, it is possible to
use --option with the 'open' command, or to omit the filename.
This adds a --image-opts arg that indicates that the positional
filename should be interpreted as a full option string, not
just a filename.
qemu-io --image-opts driver=https,url=https://127.0.0.1/images,sslverify=off
qemu-io --image-opts driver=qcow2,file.filename=/home/berrange/demo.qcow2
This flag is mutually exclusive with the '-f' flag and with
the '-o' flag to the 'open' command
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-io.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 51 insertions(+), 6 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 969c8bf..4a0d5f0 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -32,6 +32,7 @@ static BlockBackend *qemuio_blk;
/* qemu-io commands passed using -c */
static int ncmdline;
static char **cmdline;
+static bool imageOpts;
static ReadLineState *readline_state;
@@ -151,6 +152,10 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
readonly = 1;
break;
case 'o':
+ if (imageOpts) {
+ printf("--image-opts and 'open -o' are mutually exclusive\n");
+ return 0;
+ }
if (!qemu_opts_parse_noisily(&empty_opts, optarg, false)) {
qemu_opts_reset(&empty_opts);
return 0;
@@ -166,6 +171,14 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
flags |= BDRV_O_RDWR;
}
+ if (imageOpts && (optind == argc - 1)) {
+ if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
+ qemu_opts_reset(&empty_opts);
+ return 0;
+ }
+ optind++;
+ }
+
qopts = qemu_opts_find(&empty_opts, NULL);
opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
qemu_opts_reset(&empty_opts);
@@ -366,6 +379,7 @@ static void reenable_tty_echo(void)
enum {
OPTION_OBJECT = 256,
+ OPTION_IMAGE_OPTS = 257,
};
static QemuOptsList qemu_object_opts = {
@@ -378,6 +392,16 @@ static QemuOptsList qemu_object_opts = {
};
+static QemuOptsList file_opts = {
+ .name = "file",
+ .implied_opt_name = "file",
+ .head = QTAILQ_HEAD_INITIALIZER(file_opts.head),
+ .desc = {
+ /* no elements => accept any params */
+ { /* end of list */ }
+ },
+};
+
int main(int argc, char **argv)
{
int readonly = 0;
@@ -397,6 +421,7 @@ int main(int argc, char **argv)
{ "cache", 1, NULL, 't' },
{ "trace", 1, NULL, 'T' },
{ "object", 1, NULL, OPTION_OBJECT },
+ { "image-opts", 0, NULL, OPTION_IMAGE_OPTS },
{ NULL, 0, NULL, 0 }
};
int c;
@@ -404,6 +429,7 @@ int main(int argc, char **argv)
int flags = BDRV_O_UNMAP;
Error *local_error = NULL;
QDict *opts = NULL;
+ const char *format = NULL;
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -431,10 +457,7 @@ int main(int argc, char **argv)
}
break;
case 'f':
- if (!opts) {
- opts = qdict_new();
- }
- qdict_put(opts, "driver", qstring_from_str(optarg));
+ format = optarg;
break;
case 'c':
add_user_command(optarg);
@@ -466,13 +489,16 @@ int main(int argc, char **argv)
usage(progname);
exit(0);
case OPTION_OBJECT: {
- QemuOpts *qopts = NULL;
+ QemuOpts *qopts;
qopts = qemu_opts_parse_noisily(&qemu_object_opts,
optarg, true);
if (!qopts) {
exit(1);
}
} break;
+ case OPTION_IMAGE_OPTS:
+ imageOpts = true;
+ break;
default:
usage(progname);
exit(1);
@@ -484,6 +510,11 @@ int main(int argc, char **argv)
exit(1);
}
+ if (format && imageOpts) {
+ error_report("--image-opts and -f are mutually exclusive");
+ exit(1);
+ }
+
if (qemu_init_main_loop(&local_error)) {
error_report_err(local_error);
exit(1);
@@ -516,7 +547,21 @@ int main(int argc, char **argv)
}
if ((argc - optind) == 1) {
- openfile(argv[optind], flags, opts);
+ if (imageOpts) {
+ QemuOpts *qopts = NULL;
+ qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
+ if (!qopts) {
+ exit(1);
+ }
+ opts = qemu_opts_to_qdict(qopts, NULL);
+ openfile(NULL, flags, opts);
+ } else {
+ if (format) {
+ opts = qdict_new();
+ qdict_put(opts, "driver", qstring_from_str(format));
+ }
+ openfile(argv[optind], flags, opts);
+ }
}
command_loop();
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 06/10] qemu-nbd: allow specifying image as a set of options args
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (4 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 07/10] qemu-img: " Daniel P. Berrange
` (4 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
Currently qemu-nbd allows an image filename to be passed on the
command line, but unless using the JSON format, it does not have
a way to set any options except the format eg
qemu-nbd https://127.0.0.1/images/centos7.iso
qemu-nbd /home/berrange/demo.qcow2
This adds a --image-opts arg that indicates that the positional
filename should be interpreted as a full option string, not
just a filename.
qemu-nbd --image-opts driver=https,url=https://127.0.0.1/images,sslverify=off
qemu-nbd --image-opts driver=file,filename=/home/berrange/demo.qcow2
This flag is mutually exclusive with the '-f' flag.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-nbd.c | 43 ++++++++++++++++++++++++++++++++++++++-----
qemu-nbd.texi | 7 ++++++-
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 130c306..fd658ba 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -44,6 +44,7 @@
#define QEMU_NBD_OPT_DISCARD 3
#define QEMU_NBD_OPT_DETECT_ZEROES 4
#define QEMU_NBD_OPT_OBJECT 5
+#define QEMU_NBD_OPT_IMAGE_OPTS 7
static NBDExport *exp;
static int verbose;
@@ -103,6 +104,7 @@ static void usage(const char *name)
" --aio=MODE set AIO mode (native or threads)\n"
" --discard=MODE set discard mode (ignore, unmap)\n"
" --detect-zeroes=MODE set detect-zeroes mode (off, on, unmap)\n"
+" --image-opts treat FILE as a full set of image options\n"
"\n"
"Report bugs to <qemu-devel@nongnu.org>\n"
, name, NBD_DEFAULT_PORT, "DEVICE");
@@ -377,6 +379,16 @@ static SocketAddress *nbd_build_socket_address(const char *sockpath,
}
+static QemuOptsList file_opts = {
+ .name = "file",
+ .implied_opt_name = "file",
+ .head = QTAILQ_HEAD_INITIALIZER(file_opts.head),
+ .desc = {
+ /* no elements => accept any params */
+ { /* end of list */ }
+ },
+};
+
static QemuOptsList qemu_object_opts = {
.name = "object",
.implied_opt_name = "qom-type",
@@ -425,6 +437,7 @@ int main(int argc, char **argv)
{ "persistent", 0, NULL, 't' },
{ "verbose", 0, NULL, 'v' },
{ "object", 1, NULL, QEMU_NBD_OPT_OBJECT },
+ { "image-opts", 0, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
{ NULL, 0, NULL, 0 }
};
int ch;
@@ -442,6 +455,7 @@ int main(int argc, char **argv)
Error *local_err = NULL;
BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
QDict *options = NULL;
+ bool imageOpts = false;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -615,6 +629,9 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
} break;
+ case QEMU_NBD_OPT_IMAGE_OPTS:
+ imageOpts = true;
+ break;
}
}
@@ -721,13 +738,29 @@ int main(int argc, char **argv)
bdrv_init();
atexit(bdrv_close_all);
- if (fmt) {
- options = qdict_new();
- qdict_put(options, "driver", qstring_from_str(fmt));
+ srcpath = argv[optind];
+ if (imageOpts) {
+ QemuOpts *opts;
+ if (fmt) {
+ error_report("--image-opts and -f are mutually exclusive");
+ exit(EXIT_FAILURE);
+ }
+ opts = qemu_opts_parse_noisily(&file_opts, srcpath, true);
+ if (!opts) {
+ qemu_opts_reset(&file_opts);
+ exit(EXIT_FAILURE);
+ }
+ options = qemu_opts_to_qdict(opts, NULL);
+ qemu_opts_reset(&file_opts);
+ blk = blk_new_open("hda", NULL, NULL, options, flags, &local_err);
+ } else {
+ if (fmt) {
+ options = qdict_new();
+ qdict_put(options, "driver", qstring_from_str(fmt));
+ }
+ blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
}
- srcpath = argv[optind];
- blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
if (!blk) {
error_reportf_err(local_err, "Failed to blk_new_open '%s': ",
argv[optind]);
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index a56ebc3..53d4063 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -13,7 +13,8 @@ Export a QEMU disk image using the NBD protocol.
@c man end
@c man begin OPTIONS
-@var{filename} is a disk image filename.
+@var{filename} is a disk image filename, or a set of block
+driver options if @var{--image-opts} is specified.
@var{dev} is an NBD device.
@@ -32,6 +33,10 @@ The offset into the image
The interface to bind to (default @samp{0.0.0.0})
@item -k, --socket=@var{path}
Use a unix socket with path @var{path}
+@item --image-opts
+Treat @var{filename} as a set of image options, instead of a plain
+filename. If this flag is specified, the @var{-f} flag should
+not be used, instead the '@code{format=}' option should be set.
@item -f, --format=@var{fmt}
Force the use of the block driver for format @var{fmt} instead of
auto-detecting
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 07/10] qemu-img: allow specifying image as a set of options args
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (5 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 06/10] qemu-nbd: " Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 08/10] qemu-nbd: don't overlap long option values with short options Daniel P. Berrange
` (3 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
Currently qemu-img allows an image filename to be passed on the
command line, but unless using the JSON format, it does not have
a way to set any options except the format eg
qemu-img info https://127.0.0.1/images/centos7.iso
This adds a --image-opts arg that indicates that the positional
filename should be interpreted as a full option string, not
just a filename.
qemu-img info --image-opts driver=https,url=https://127.0.0.1/images,sslverify=off
This flag is mutually exclusive with the '-f' / '-F' flags.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-img-cmds.hx | 44 ++++++------
qemu-img.c | 205 ++++++++++++++++++++++++++++++++++++++++++++-----------
qemu-img.texi | 6 ++
3 files changed, 195 insertions(+), 60 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 0eaf307..e7cded6 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,68 +10,68 @@ STEXI
ETEXI
DEF("check", img_check,
- "check [-q] [--object objectdef] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
+ "check [-q] [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
STEXI
-@item check [--object @var{objectdef}] [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
+@item check [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
ETEXI
DEF("create", img_create,
- "create [-q] [--object objectdef] [-f fmt] [-o options] filename [size]")
+ "create [-q] [--object objectdef] [--image-opts] [-f fmt] [-o options] filename [size]")
STEXI
-@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
ETEXI
DEF("commit", img_commit,
- "commit [-q] [--object objectdef] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
+ "commit [-q] [--object objectdef] [--image-opts] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
STEXI
-@item commit [--object @var{objectdef}] [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
+@item commit [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
ETEXI
DEF("compare", img_compare,
- "compare [--object objectdef] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2")
+ "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2")
STEXI
-@item compare [--object @var{objectdef}] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+@item compare [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2}
ETEXI
DEF("convert", img_convert,
- "convert [--object objectdef] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
+ "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
STEXI
-@item convert [--object @var{objectdef}] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("info", img_info,
- "info [--object objectdef] [-f fmt] [--output=ofmt] [--backing-chain] filename")
+ "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [--backing-chain] filename")
STEXI
-@item info [--object @var{objectdef}] [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
+@item info [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
ETEXI
DEF("map", img_map,
- "map [--object objectdef] [-f fmt] [--output=ofmt] filename")
+ "map [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] filename")
STEXI
-@item map [--object @var{objectdef}] [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item map [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
ETEXI
DEF("snapshot", img_snapshot,
- "snapshot [--object objectdef] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+ "snapshot [--object objectdef] [--image-opts] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
STEXI
-@item snapshot [--object @var{objectdef}] [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [--object @var{objectdef}] [--image-opts] [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
ETEXI
DEF("rebase", img_rebase,
- "rebase [--object objectdef] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+ "rebase [--object objectdef] [--image-opts] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
STEXI
-@item rebase [--object @var{objectdef}] [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
ETEXI
DEF("resize", img_resize,
- "resize [--object objectdef] [-q] filename [+ | -]size")
+ "resize [--object objectdef] [--image-opts] [-q] filename [+ | -]size")
STEXI
-@item resize [--object @var{objectdef}] [-q] @var{filename} [+ | -]@var{size}
+@item resize [--object @var{objectdef}] [--image-opts] [-q] @var{filename} [+ | -]@var{size}
ETEXI
DEF("amend", img_amend,
- "amend [--object objectdef] [-p] [-q] [-f fmt] [-t cache] -o options filename")
+ "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
STEXI
-@item amend [--object @var{objectdef}] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
@end table
ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 5b8c7a3..36b6150 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -50,6 +50,7 @@ enum {
OPTION_OUTPUT = 256,
OPTION_BACKING_CHAIN = 257,
OPTION_OBJECT = 258,
+ OPTION_IMAGE_OPTS = 259,
};
typedef enum OutputFormat {
@@ -170,6 +171,15 @@ static QemuOptsList qemu_object_opts = {
},
};
+static QemuOptsList qemu_source_opts = {
+ .name = "source",
+ .implied_opt_name = "file",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_source_opts.head),
+ .desc = {
+ { }
+ },
+};
+
static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
{
int ret = 0;
@@ -212,13 +222,56 @@ static int print_block_option_help(const char *filename, const char *fmt)
return 0;
}
-static BlockBackend *img_open(const char *id, const char *filename,
- const char *fmt, int flags,
- bool require_io, bool quiet)
+
+static int img_open_password(BlockBackend *blk, const char *filename,
+ bool require_io, bool quiet)
{
- BlockBackend *blk;
BlockDriverState *bs;
char password[256];
+
+ bs = blk_bs(blk);
+ if (bdrv_is_encrypted(bs) && require_io) {
+ qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
+ if (qemu_read_password(password, sizeof(password)) < 0) {
+ error_report("No password given");
+ return -1;
+ }
+ if (bdrv_set_key(bs, password) < 0) {
+ error_report("invalid password");
+ return -1;
+ }
+ }
+ return 0;
+}
+
+
+static BlockBackend *img_open_opts(const char *id,
+ const char *optstr,
+ QemuOpts *opts, int flags,
+ bool require_io, bool quiet)
+{
+ QDict *options;
+ Error *local_err = NULL;
+ BlockBackend *blk;
+ options = qemu_opts_to_qdict(opts, NULL);
+ blk = blk_new_open(id, NULL, NULL, options, flags, &local_err);
+ if (!blk) {
+ error_reportf_err(local_err, "Could not open '%s'", optstr);
+ return NULL;
+ }
+
+ if (img_open_password(blk, optstr, require_io, quiet) < 0) {
+ blk_unref(blk);
+ return NULL;
+ }
+ return blk;
+}
+
+static BlockBackend *img_open_file(const char *id, const char *filename,
+ const char *fmt, int flags,
+ bool require_io, bool quiet)
+{
+ BlockBackend *blk;
Error *local_err = NULL;
QDict *options = NULL;
@@ -230,27 +283,43 @@ static BlockBackend *img_open(const char *id, const char *filename,
blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
if (!blk) {
error_reportf_err(local_err, "Could not open '%s': ", filename);
- goto fail;
+ return NULL;
}
- bs = blk_bs(blk);
- if (bdrv_is_encrypted(bs) && require_io) {
- qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
- if (qemu_read_password(password, sizeof(password)) < 0) {
- error_report("No password given");
- goto fail;
- }
- if (bdrv_set_key(bs, password) < 0) {
- error_report("invalid password");
- goto fail;
- }
+ if (img_open_password(blk, filename, require_io, quiet) < 0) {
+ blk_unref(blk);
+ return NULL;
+ }
+ return blk;
+}
+
+
+static BlockBackend *img_open(const char *id,
+ bool image_opts,
+ const char *filename,
+ const char *fmt, int flags,
+ bool require_io, bool quiet)
+{
+ BlockBackend *blk;
+ if (image_opts) {
+ QemuOpts *opts;
+ if (fmt) {
+ error_report("--image-opts and --format are mutually exclusive");
+ return NULL;
+ }
+ opts = qemu_opts_parse_noisily(qemu_find_opts("source"),
+ filename, true);
+ if (!opts) {
+ return NULL;
+ }
+ blk = img_open_opts(id, filename, opts, flags, true, quiet);
+ } else {
+ blk = img_open_file(id, filename, fmt, flags, true, quiet);
}
return blk;
-fail:
- blk_unref(blk);
- return NULL;
}
+
static int add_old_style_options(const char *fmt, QemuOpts *opts,
const char *base_filename,
const char *base_fmt)
@@ -527,6 +596,7 @@ static int img_check(int argc, char **argv)
ImageCheck *check;
bool quiet = false;
Error *local_err = NULL;
+ bool image_opts = false;
fmt = NULL;
output = NULL;
@@ -539,6 +609,7 @@ static int img_check(int argc, char **argv)
{"repair", required_argument, 0, 'r'},
{"output", required_argument, 0, OPTION_OUTPUT},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "hf:r:T:q",
@@ -583,6 +654,9 @@ static int img_check(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -612,7 +686,7 @@ static int img_check(int argc, char **argv)
return 1;
}
- blk = img_open("image", filename, fmt, flags, true, quiet);
+ blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
return 1;
}
@@ -724,6 +798,7 @@ static int img_commit(int argc, char **argv)
bool progress = false, quiet = false, drop = false;
Error *local_err = NULL;
CommonBlockJobCBInfo cbi;
+ bool image_opts = false;
fmt = NULL;
cache = BDRV_DEFAULT_CACHE;
@@ -732,6 +807,7 @@ static int img_commit(int argc, char **argv)
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:ht:b:dpq",
@@ -772,6 +848,9 @@ static int img_commit(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -799,7 +878,7 @@ static int img_commit(int argc, char **argv)
return 1;
}
- blk = img_open("image", filename, fmt, flags, true, quiet);
+ blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
return 1;
}
@@ -1049,12 +1128,14 @@ static int img_compare(int argc, char **argv)
int c, pnum;
uint64_t progress_base;
Error *local_err = NULL;
+ bool image_opts = false;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "hf:F:T:pqs",
@@ -1094,6 +1175,9 @@ static int img_compare(int argc, char **argv)
goto out4;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -1128,18 +1212,18 @@ static int img_compare(int argc, char **argv)
goto out3;
}
- blk1 = img_open("image_1", filename1, fmt1, flags, true, quiet);
+ blk1 = img_open("image_1", image_opts, filename1, fmt1, flags, true, quiet);
if (!blk1) {
ret = 2;
goto out3;
}
- bs1 = blk_bs(blk1);
- blk2 = img_open("image_2", filename2, fmt2, flags, true, quiet);
+ blk2 = img_open("image_2", image_opts, filename2, fmt2, flags, true, quiet);
if (!blk2) {
ret = 2;
goto out2;
}
+ bs1 = blk_bs(blk1);
bs2 = blk_bs(blk2);
buf1 = blk_blockalign(blk1, IO_BUF_SIZE);
@@ -1646,6 +1730,7 @@ static int img_convert(int argc, char **argv)
Error *local_err = NULL;
QemuOpts *sn_opts = NULL;
ImgConvertState state;
+ bool image_opts = false;
fmt = NULL;
out_fmt = "raw";
@@ -1658,6 +1743,7 @@ static int img_convert(int argc, char **argv)
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn",
@@ -1759,6 +1845,9 @@ static int img_convert(int argc, char **argv)
goto fail_getopt;
}
break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -1775,7 +1864,6 @@ static int img_convert(int argc, char **argv)
}
qemu_progress_init(progress, 1.0);
-
bs_n = argc - optind - 1;
out_filename = bs_n >= 1 ? argv[argc - 1] : NULL;
@@ -1813,8 +1901,8 @@ static int img_convert(int argc, char **argv)
for (bs_i = 0; bs_i < bs_n; bs_i++) {
char *id = bs_n > 1 ? g_strdup_printf("source_%d", bs_i)
: g_strdup("source");
- blk[bs_i] = img_open(id, argv[optind + bs_i], fmt, src_flags,
- true, quiet);
+ blk[bs_i] = img_open(id, image_opts, argv[optind + bs_i],
+ fmt, src_flags, true, quiet);
g_free(id);
if (!blk[bs_i]) {
ret = -1;
@@ -1955,7 +2043,13 @@ static int img_convert(int argc, char **argv)
goto out;
}
- out_blk = img_open("target", out_filename, out_fmt, flags, true, quiet);
+ /* XXX we should allow --image-opts to trigger use of
+ * img_open() here, but then we have trouble with
+ * the bdrv_create() call which takes different params.
+ * Not critical right now, so fix can wait...
+ */
+ out_blk = img_open_file("target", out_filename,
+ out_fmt, flags, true, quiet);
if (!out_blk) {
ret = -1;
goto out;
@@ -2121,7 +2215,8 @@ static gboolean str_equal_func(gconstpointer a, gconstpointer b)
* image file. If there was an error a message will have been printed to
* stderr.
*/
-static ImageInfoList *collect_image_info_list(const char *filename,
+static ImageInfoList *collect_image_info_list(bool image_opts,
+ const char *filename,
const char *fmt,
bool chain)
{
@@ -2145,8 +2240,9 @@ static ImageInfoList *collect_image_info_list(const char *filename,
}
g_hash_table_insert(filenames, (gpointer)filename, NULL);
- blk = img_open("image", filename, fmt,
- BDRV_O_FLAGS | BDRV_O_NO_BACKING, false, false);
+ blk = img_open("image", image_opts, filename, fmt,
+ BDRV_O_FLAGS | BDRV_O_NO_BACKING,
+ false, false);
if (!blk) {
goto err;
}
@@ -2198,6 +2294,7 @@ static int img_info(int argc, char **argv)
const char *filename, *fmt, *output;
ImageInfoList *list;
Error *local_err = NULL;
+ bool image_opts = false;
fmt = NULL;
output = NULL;
@@ -2209,6 +2306,7 @@ static int img_info(int argc, char **argv)
{"output", required_argument, 0, OPTION_OUTPUT},
{"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:h",
@@ -2238,6 +2336,9 @@ static int img_info(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -2261,7 +2362,7 @@ static int img_info(int argc, char **argv)
return 1;
}
- list = collect_image_info_list(filename, fmt, chain);
+ list = collect_image_info_list(image_opts, filename, fmt, chain);
if (!list) {
return 1;
}
@@ -2403,6 +2504,7 @@ static int img_map(int argc, char **argv)
MapEntry curr = { .length = 0 }, next;
int ret = 0;
Error *local_err = NULL;
+ bool image_opts = false;
fmt = NULL;
output = NULL;
@@ -2413,6 +2515,7 @@ static int img_map(int argc, char **argv)
{"format", required_argument, 0, 'f'},
{"output", required_argument, 0, OPTION_OUTPUT},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:h",
@@ -2439,6 +2542,9 @@ static int img_map(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -2462,7 +2568,8 @@ static int img_map(int argc, char **argv)
return 1;
}
- blk = img_open("image", filename, fmt, BDRV_O_FLAGS, true, false);
+ blk = img_open("image", image_opts, filename, fmt,
+ BDRV_O_FLAGS, true, false);
if (!blk) {
return 1;
}
@@ -2524,6 +2631,7 @@ static int img_snapshot(int argc, char **argv)
qemu_timeval tv;
bool quiet = false;
Error *err = NULL;
+ bool image_opts = false;
bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
/* Parse commandline parameters */
@@ -2531,6 +2639,7 @@ static int img_snapshot(int argc, char **argv)
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "la:c:d:hq",
@@ -2586,6 +2695,9 @@ static int img_snapshot(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -2602,7 +2714,8 @@ static int img_snapshot(int argc, char **argv)
}
/* Open the image */
- blk = img_open("image", filename, NULL, bdrv_oflags, true, quiet);
+ blk = img_open("image", image_opts, filename, NULL,
+ bdrv_oflags, true, quiet);
if (!blk) {
return 1;
}
@@ -2666,6 +2779,7 @@ static int img_rebase(int argc, char **argv)
int progress = 0;
bool quiet = false;
Error *local_err = NULL;
+ bool image_opts = false;
/* Parse commandline parameters */
fmt = NULL;
@@ -2677,6 +2791,7 @@ static int img_rebase(int argc, char **argv)
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "hf:F:b:upt:T:q",
@@ -2721,6 +2836,9 @@ static int img_rebase(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -2766,7 +2884,7 @@ static int img_rebase(int argc, char **argv)
* Ignore the old backing file for unsafe rebase in case we want to correct
* the reference to a renamed or moved backing file.
*/
- blk = img_open("image", filename, fmt, flags, true, quiet);
+ blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
ret = -1;
goto out;
@@ -3018,6 +3136,7 @@ static int img_resize(int argc, char **argv)
}
},
};
+ bool image_opts = false;
/* Remove size from argv manually so that negative numbers are not treated
* as options by getopt. */
@@ -3034,6 +3153,7 @@ static int img_resize(int argc, char **argv)
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:hq",
@@ -3060,6 +3180,9 @@ static int img_resize(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -3101,8 +3224,8 @@ static int img_resize(int argc, char **argv)
n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
qemu_opts_del(param);
- blk = img_open("image", filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR,
- true, quiet);
+ blk = img_open("image", image_opts, filename, fmt,
+ BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
if (!blk) {
ret = -1;
goto out;
@@ -3162,12 +3285,14 @@ static int img_amend(int argc, char **argv)
BlockBackend *blk = NULL;
BlockDriverState *bs = NULL;
Error *local_err = NULL;
+ bool image_opts = false;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
static const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "ho:f:t:pq",
@@ -3215,6 +3340,9 @@ static int img_amend(int argc, char **argv)
goto out_no_progress;
}
break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -3256,7 +3384,7 @@ static int img_amend(int argc, char **argv)
goto out;
}
- blk = img_open("image", filename, fmt, flags, true, quiet);
+ blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
ret = -1;
goto out;
@@ -3354,6 +3482,7 @@ int main(int argc, char **argv)
cmdname = argv[1];
qemu_add_opts(&qemu_object_opts);
+ qemu_add_opts(&qemu_source_opts);
/* find the command */
for (cmd = img_cmds; cmd->name != NULL; cmd++) {
diff --git a/qemu-img.texi b/qemu-img.texi
index 9e47543..afaebdd 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -32,6 +32,12 @@ page for a description of the object properties. The most common object
type is a @code{secret}, which is used to supply passwords and/or encryption
keys.
+@item --image-opts
+
+Indicates that the @var{filename} parameter is to be interpreted as a
+full option string, not a plain filename. This parameter is mutually
+exclusive with the @var{-f} and @var{-F} parameters.
+
@item fmt
is the disk image format. It is guessed automatically in most cases. See below
for a description of the supported disk formats.
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 08/10] qemu-nbd: don't overlap long option values with short options
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (6 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 07/10] qemu-img: " Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 09/10] qemu-nbd: use no_argument/required_argument constants Daniel P. Berrange
` (2 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
When defining values for long options, the normal practice is
to start numbering from 256, to avoid overlap with the range
of valid values for short options.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-nbd.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index fd658ba..3805ac3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -39,12 +39,12 @@
#include <pthread.h>
#define SOCKET_PATH "/var/lock/qemu-nbd-%s"
-#define QEMU_NBD_OPT_CACHE 1
-#define QEMU_NBD_OPT_AIO 2
-#define QEMU_NBD_OPT_DISCARD 3
-#define QEMU_NBD_OPT_DETECT_ZEROES 4
-#define QEMU_NBD_OPT_OBJECT 5
-#define QEMU_NBD_OPT_IMAGE_OPTS 7
+#define QEMU_NBD_OPT_CACHE 256
+#define QEMU_NBD_OPT_AIO 257
+#define QEMU_NBD_OPT_DISCARD 258
+#define QEMU_NBD_OPT_DETECT_ZEROES 259
+#define QEMU_NBD_OPT_OBJECT 260
+#define QEMU_NBD_OPT_IMAGE_OPTS 261
static NBDExport *exp;
static int verbose;
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 09/10] qemu-nbd: use no_argument/required_argument constants
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (7 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 08/10] qemu-nbd: don't overlap long option values with short options Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 10/10] qemu-io: " Daniel P. Berrange
2016-02-15 15:53 ` [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
When declaring the 'struct option' array, use the standard
constants no_argument/required_argument, instead of magic
values 0 and 1.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-nbd.c | 47 ++++++++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 3805ac3..ab7c038 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -415,29 +415,30 @@ int main(int argc, char **argv)
const char *sn_id_or_name = NULL;
const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
struct option lopt[] = {
- { "help", 0, NULL, 'h' },
- { "version", 0, NULL, 'V' },
- { "bind", 1, NULL, 'b' },
- { "port", 1, NULL, 'p' },
- { "socket", 1, NULL, 'k' },
- { "offset", 1, NULL, 'o' },
- { "read-only", 0, NULL, 'r' },
- { "partition", 1, NULL, 'P' },
- { "connect", 1, NULL, 'c' },
- { "disconnect", 0, NULL, 'd' },
- { "snapshot", 0, NULL, 's' },
- { "load-snapshot", 1, NULL, 'l' },
- { "nocache", 0, NULL, 'n' },
- { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
- { "aio", 1, NULL, QEMU_NBD_OPT_AIO },
- { "discard", 1, NULL, QEMU_NBD_OPT_DISCARD },
- { "detect-zeroes", 1, NULL, QEMU_NBD_OPT_DETECT_ZEROES },
- { "shared", 1, NULL, 'e' },
- { "format", 1, NULL, 'f' },
- { "persistent", 0, NULL, 't' },
- { "verbose", 0, NULL, 'v' },
- { "object", 1, NULL, QEMU_NBD_OPT_OBJECT },
- { "image-opts", 0, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
+ { "help", no_argument, NULL, 'h' },
+ { "version", no_argument, NULL, 'V' },
+ { "bind", required_argument, NULL, 'b' },
+ { "port", required_argument, NULL, 'p' },
+ { "socket", required_argument, NULL, 'k' },
+ { "offset", required_argument, NULL, 'o' },
+ { "read-only", no_argument, NULL, 'r' },
+ { "partition", required_argument, NULL, 'P' },
+ { "connect", required_argument, NULL, 'c' },
+ { "disconnect", no_argument, NULL, 'd' },
+ { "snapshot", no_argument, NULL, 's' },
+ { "load-snapshot", required_argument, NULL, 'l' },
+ { "nocache", no_argument, NULL, 'n' },
+ { "cache", required_argument, NULL, QEMU_NBD_OPT_CACHE },
+ { "aio", required_argument, NULL, QEMU_NBD_OPT_AIO },
+ { "discard", required_argument, NULL, QEMU_NBD_OPT_DISCARD },
+ { "detect-zeroes", required_argument, NULL,
+ QEMU_NBD_OPT_DETECT_ZEROES },
+ { "shared", required_argument, NULL, 'e' },
+ { "format", required_argument, NULL, 'f' },
+ { "persistent", no_argument, NULL, 't' },
+ { "verbose", no_argument, NULL, 'v' },
+ { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
+ { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
{ NULL, 0, NULL, 0 }
};
int ch;
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v6 10/10] qemu-io: use no_argument/required_argument constants
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (8 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 09/10] qemu-nbd: use no_argument/required_argument constants Daniel P. Berrange
@ 2016-02-15 14:33 ` Daniel P. Berrange
2016-02-15 15:53 ` [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
10 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 14:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, qemu-block
When declaring the 'struct option' array, use the standard
constants no_argument/required_argument, instead of magic
values 0 and 1.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-io.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 4a0d5f0..8c31257 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -407,21 +407,21 @@ int main(int argc, char **argv)
int readonly = 0;
const char *sopt = "hVc:d:f:rsnmgkt:T:";
const struct option lopt[] = {
- { "help", 0, NULL, 'h' },
- { "version", 0, NULL, 'V' },
- { "offset", 1, NULL, 'o' },
- { "cmd", 1, NULL, 'c' },
- { "format", 1, NULL, 'f' },
- { "read-only", 0, NULL, 'r' },
- { "snapshot", 0, NULL, 's' },
- { "nocache", 0, NULL, 'n' },
- { "misalign", 0, NULL, 'm' },
- { "native-aio", 0, NULL, 'k' },
- { "discard", 1, NULL, 'd' },
- { "cache", 1, NULL, 't' },
- { "trace", 1, NULL, 'T' },
- { "object", 1, NULL, OPTION_OBJECT },
- { "image-opts", 0, NULL, OPTION_IMAGE_OPTS },
+ { "help", no_argument, NULL, 'h' },
+ { "version", no_argument, NULL, 'V' },
+ { "offset", required_argument, NULL, 'o' },
+ { "cmd", required_argument, NULL, 'c' },
+ { "format", required_argument, NULL, 'f' },
+ { "read-only", no_argument, NULL, 'r' },
+ { "snapshot", no_argument, NULL, 's' },
+ { "nocache", no_argument, NULL, 'n' },
+ { "misalign", no_argument, NULL, 'm' },
+ { "native-aio", no_argument, NULL, 'k' },
+ { "discard", required_argument, NULL, 'd' },
+ { "cache", required_argument, NULL, 't' },
+ { "trace", required_argument, NULL, 'T' },
+ { "object", required_argument, NULL, OPTION_OBJECT },
+ { "image-opts", no_argument, NULL, OPTION_IMAGE_OPTS },
{ NULL, 0, NULL, 0 }
};
int c;
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (9 preceding siblings ...)
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 10/10] qemu-io: " Daniel P. Berrange
@ 2016-02-15 15:53 ` Kevin Wolf
2016-02-17 10:10 ` Daniel P. Berrange
10 siblings, 1 reply; 20+ messages in thread
From: Kevin Wolf @ 2016-02-15 15:53 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Paolo Bonzini, qemu-devel, qemu-block
Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:
> This series of patches expands the syntax of the qemu-img,
> qemu-nbd and qemu-io commands to make them more flexible.
>
> v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
> v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
> v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
> v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html
>
> First all three gain a --object parameter, which allows
> instantiation of user creatable object types. The immediate
> use case is to allow for creation of the 'secret' object
> type to pass passwords for curl, iscsi and rbd drivers.
> For qemu-nbd this will also be needed to create TLS
> certificates for encryption support.
>
> Then all three gain a '--image-opts' parameter which causes
> the positional filenames to be interepreted as option strings
> rather tha nplain filenames. This avoids the need to use the
> JSON syntax, or to add custom CLI args for each block backend
> option that exists. The immediate use case is to allow the
> user to specify the ID of the 'secret' object they just created.
>
> Finally, there are a few small cleanup patches
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
2016-02-15 15:53 ` [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
@ 2016-02-17 10:10 ` Daniel P. Berrange
2016-02-17 10:37 ` Kevin Wolf
0 siblings, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-17 10:10 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, qemu-devel, qemu-block
On Mon, Feb 15, 2016 at 04:53:27PM +0100, Kevin Wolf wrote:
> Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:
> > This series of patches expands the syntax of the qemu-img,
> > qemu-nbd and qemu-io commands to make them more flexible.
> >
> > v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
> > v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> > v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
> > v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html
> >
> > First all three gain a --object parameter, which allows
> > instantiation of user creatable object types. The immediate
> > use case is to allow for creation of the 'secret' object
> > type to pass passwords for curl, iscsi and rbd drivers.
> > For qemu-nbd this will also be needed to create TLS
> > certificates for encryption support.
> >
> > Then all three gain a '--image-opts' parameter which causes
> > the positional filenames to be interepreted as option strings
> > rather tha nplain filenames. This avoids the need to use the
> > JSON syntax, or to add custom CLI args for each block backend
> > option that exists. The immediate use case is to allow the
> > user to specify the ID of the 'secret' object they just created.
> >
> > Finally, there are a few small cleanup patches
>
> Thanks, applied to the block branch.
The NBD patches just merged, so I've rebased this series and sent you
a v7 to resolve the conflicts
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
2016-02-17 10:10 ` Daniel P. Berrange
@ 2016-02-17 10:37 ` Kevin Wolf
2016-02-17 10:41 ` Daniel P. Berrange
0 siblings, 1 reply; 20+ messages in thread
From: Kevin Wolf @ 2016-02-17 10:37 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Paolo Bonzini, qemu-devel, qemu-block
Am 17.02.2016 um 11:10 hat Daniel P. Berrange geschrieben:
> On Mon, Feb 15, 2016 at 04:53:27PM +0100, Kevin Wolf wrote:
> > Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:
> > > This series of patches expands the syntax of the qemu-img,
> > > qemu-nbd and qemu-io commands to make them more flexible.
> > >
> > > v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> > > v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
> > > v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
> > > v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> > > v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
> > > v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html
> > >
> > > First all three gain a --object parameter, which allows
> > > instantiation of user creatable object types. The immediate
> > > use case is to allow for creation of the 'secret' object
> > > type to pass passwords for curl, iscsi and rbd drivers.
> > > For qemu-nbd this will also be needed to create TLS
> > > certificates for encryption support.
> > >
> > > Then all three gain a '--image-opts' parameter which causes
> > > the positional filenames to be interepreted as option strings
> > > rather tha nplain filenames. This avoids the need to use the
> > > JSON syntax, or to add custom CLI args for each block backend
> > > option that exists. The immediate use case is to allow the
> > > user to specify the ID of the 'secret' object they just created.
> > >
> > > Finally, there are a few small cleanup patches
> >
> > Thanks, applied to the block branch.
>
> The NBD patches just merged, so I've rebased this series and sent you
> a v7 to resolve the conflicts
*sigh* Okay, I'll drop this series and re-review sometime later.
Kevin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
2016-02-17 10:37 ` Kevin Wolf
@ 2016-02-17 10:41 ` Daniel P. Berrange
0 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-02-17 10:41 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, qemu-devel, qemu-block
On Wed, Feb 17, 2016 at 11:37:52AM +0100, Kevin Wolf wrote:
> Am 17.02.2016 um 11:10 hat Daniel P. Berrange geschrieben:
> > On Mon, Feb 15, 2016 at 04:53:27PM +0100, Kevin Wolf wrote:
> > > Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:
> > > > This series of patches expands the syntax of the qemu-img,
> > > > qemu-nbd and qemu-io commands to make them more flexible.
> > > >
> > > > v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> > > > v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
> > > > v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
> > > > v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> > > > v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
> > > > v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html
> > > >
> > > > First all three gain a --object parameter, which allows
> > > > instantiation of user creatable object types. The immediate
> > > > use case is to allow for creation of the 'secret' object
> > > > type to pass passwords for curl, iscsi and rbd drivers.
> > > > For qemu-nbd this will also be needed to create TLS
> > > > certificates for encryption support.
> > > >
> > > > Then all three gain a '--image-opts' parameter which causes
> > > > the positional filenames to be interepreted as option strings
> > > > rather tha nplain filenames. This avoids the need to use the
> > > > JSON syntax, or to add custom CLI args for each block backend
> > > > option that exists. The immediate use case is to allow the
> > > > user to specify the ID of the 'secret' object they just created.
> > > >
> > > > Finally, there are a few small cleanup patches
> > >
> > > Thanks, applied to the block branch.
> >
> > The NBD patches just merged, so I've rebased this series and sent you
> > a v7 to resolve the conflicts
>
> *sigh* Okay, I'll drop this series and re-review sometime later.
NB it was a pretty clear rebase with no functional changes. Mostly
just resolving conflicts in the "struct option" list entries used
for getopt_long
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
@ 2016-04-27 9:26 ` Markus Armbruster
2016-04-27 9:58 ` Daniel P. Berrange
2016-04-27 13:37 ` Markus Armbruster
0 siblings, 2 replies; 20+ messages in thread
From: Markus Armbruster @ 2016-04-27 9:26 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Kevin Wolf, Paolo Bonzini, qemu-block, Eric Blake
This commit regresses error message quality from
$ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
to just
qemu-system-x86_64: Property '.foo' not found
Clue: cur_loc points to garbage.
(gdb) p cur_loc
$1 = (Location *) 0x7fffffffdc10
(gdb) p *cur_loc
$2 = {kind = (unknown: 4294958128), num = 32767,
ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}
Looks like cur_loc is dangling. Happens when you forget to loc_pop() a
Location before it dies. This one is on the stack.
*Might* be release critical.
For comparison, this is how it looks before the patch:
(gdb) p cur_loc
$1 = (Location *) 0x7fffffffdc10
(gdb) p *cur_loc
$2 = {kind = LOC_CMDLINE, num = 2, ptr = 0x7fffffffe018, prev =
0x5555565d2770 <std_loc>}
Reported-by: Eric Blake <eblake@redhat.com>
"Daniel P. Berrange" <berrange@redhat.com> writes:
> The QMP monitor code has two helper methods object_add
> and qmp_object_del that are called from several places
> in the code (QMP, HMP and main emulator startup).
>
> The HMP and main emulator startup code also share
> further logic that extracts the qom-type & id
> values from a qdict.
>
> We soon need to use this logic from qemu-img, qemu-io
> and qemu-nbd too, but don't want those to depend on
> the monitor, nor do we want to duplicate the code.
>
> To avoid this, move some code out of qmp.c and hmp.c
> adding new methods to qom/object_interfaces.c
>
> - user_creatable_add - takes a QDict holding a full
> object definition & instantiates it
> - user_creatable_add_type - takes an ID, type name,
> and QDict holding object properties & instantiates
> it
> - user_creatable_add_opts - takes a QemuOpts holding
> a full object definition & instantiates it
> - user_creatable_add_opts_foreach - variant on
> user_creatable_add_opts which can be directly used
> in conjunction with qemu_opts_foreach.
> - user_creatable_del - takes an ID and deletes the
> corresponding object
>
> The existing code is updated to use these new methods.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> hmp.c | 52 +++---------
> include/monitor/monitor.h | 3 -
> include/qom/object_interfaces.h | 92 +++++++++++++++++++++
> qmp.c | 76 ++----------------
> qom/object_interfaces.c | 174 ++++++++++++++++++++++++++++++++++++++++
> vl.c | 68 ++--------------
> 6 files changed, 290 insertions(+), 175 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index c6419da..41fb9ca 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -30,6 +30,7 @@
> #include "qapi/string-output-visitor.h"
> #include "qapi/util.h"
> #include "qapi-visit.h"
> +#include "qom/object_interfaces.h"
> #include "ui/console.h"
> #include "block/qapi.h"
> #include "qemu-io.h"
> @@ -1655,58 +1656,27 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> void hmp_object_add(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
> - Error *err_end = NULL;
> QemuOpts *opts;
> - char *type = NULL;
> - char *id = NULL;
> OptsVisitor *ov;
> - QDict *pdict;
> - Visitor *v;
> + Object *obj = NULL;
>
> opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
> if (err) {
> - goto out;
> + hmp_handle_error(mon, &err);
> + return;
> }
>
> ov = opts_visitor_new(opts);
> - pdict = qdict_clone_shallow(qdict);
> - v = opts_get_visitor(ov);
> -
> - visit_start_struct(v, NULL, NULL, 0, &err);
> - if (err) {
> - goto out_clean;
> - }
> -
> - qdict_del(pdict, "qom-type");
> - visit_type_str(v, "qom-type", &type, &err);
> - if (err) {
> - goto out_end;
> - }
> + obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
> + opts_visitor_cleanup(ov);
> + qemu_opts_del(opts);
>
> - qdict_del(pdict, "id");
> - visit_type_str(v, "id", &id, &err);
> if (err) {
> - goto out_end;
> + hmp_handle_error(mon, &err);
> }
> -
> - object_add(type, id, pdict, v, &err);
> -
> -out_end:
> - visit_end_struct(v, &err_end);
> - if (!err && err_end) {
> - qmp_object_del(id, NULL);
> + if (obj) {
> + object_unref(obj);
> }
> - error_propagate(&err, err_end);
> -out_clean:
> - opts_visitor_cleanup(ov);
> -
> - QDECREF(pdict);
> - qemu_opts_del(opts);
> - g_free(id);
> - g_free(type);
> -
> -out:
> - hmp_handle_error(mon, &err);
> }
>
> void hmp_getfd(Monitor *mon, const QDict *qdict)
> @@ -1934,7 +1904,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
> const char *id = qdict_get_str(qdict, "id");
> Error *err = NULL;
>
> - qmp_object_del(id, &err);
> + user_creatable_del(id, &err);
> hmp_handle_error(mon, &err);
> }
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 91b95ae..aa0f373 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -43,9 +43,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
> int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> void *opaque);
>
> -void object_add(const char *type, const char *id, const QDict *qdict,
> - Visitor *v, Error **errp);
> -
> AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> bool has_opaque, const char *opaque,
> Error **errp);
> diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> index 283ae0d..d579746 100644
> --- a/include/qom/object_interfaces.h
> +++ b/include/qom/object_interfaces.h
> @@ -2,6 +2,8 @@
> #define OBJECT_INTERFACES_H
>
> #include "qom/object.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/visitor.h"
>
> #define TYPE_USER_CREATABLE "user-creatable"
>
> @@ -72,4 +74,94 @@ void user_creatable_complete(Object *obj, Error **errp);
> * from implements USER_CREATABLE interface.
> */
> bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
> +
> +/**
> + * user_creatable_add:
> + * @qdict: the object definition
> + * @v: the visitor
> + * @errp: if an error occurs, a pointer to an area to store the error
> + *
> + * Create an instance of the user creatable object whose type
> + * is defined in @qdict by the 'qom-type' field, placing it
> + * in the object composition tree with name provided by the
> + * 'id' field. The remaining fields in @qdict are used to
> + * initialize the object properties.
> + *
> + * Returns: the newly created object or NULL on error
> + */
> +Object *user_creatable_add(const QDict *qdict,
> + Visitor *v, Error **errp);
> +
> +/**
> + * user_creatable_add_type:
> + * @type: the object type name
> + * @id: the unique ID for the object
> + * @qdict: the object properties
> + * @v: the visitor
> + * @errp: if an error occurs, a pointer to an area to store the error
> + *
> + * Create an instance of the user creatable object @type, placing
> + * it in the object composition tree with name @id, initializing
> + * it with properties from @qdict
> + *
> + * Returns: the newly created object or NULL on error
> + */
> +Object *user_creatable_add_type(const char *type, const char *id,
> + const QDict *qdict,
> + Visitor *v, Error **errp);
> +
> +/**
> + * user_creatable_add_opts:
> + * @opts: the object definition
> + * @errp: if an error occurs, a pointer to an area to store the error
> + *
> + * Create an instance of the user creatable object whose type
> + * is defined in @opts by the 'qom-type' option, placing it
> + * in the object composition tree with name provided by the
> + * 'id' field. The remaining options in @opts are used to
> + * initialize the object properties.
> + *
> + * Returns: the newly created object or NULL on error
> + */
> +Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
> +
> +
> +/**
> + * user_creatable_add_opts_predicate:
> + * @type: the QOM type to be added
> + *
> + * A callback function to determine whether an object
> + * of type @type should be created. Instances of this
> + * callback should be passed to user_creatable_add_opts_foreach
> + */
> +typedef bool (*user_creatable_add_opts_predicate)(const char *type);
> +
> +/**
> + * user_creatable_add_opts_foreach:
> + * @opaque: a user_creatable_add_opts_predicate callback or NULL
> + * @opts: options to create
> + * @errp: if an error occurs, a pointer to an area to store the error
> + *
> + * An iterator callback to be used in conjunction with
> + * the qemu_opts_foreach() method for creating a list of
> + * objects from a set of QemuOpts
> + *
> + * The @opaque parameter can be passed a user_creatable_add_opts_predicate
> + * callback to filter which types of object are created during iteration.
> + *
> + * Returns: 0 on success, -1 on error
> + */
> +int user_creatable_add_opts_foreach(void *opaque,
> + QemuOpts *opts, Error **errp);
> +
> +/**
> + * user_creatable_del:
> + * @id: the unique ID for the object
> + * @errp: if an error occurs, a pointer to an area to store the error
> + *
> + * Delete an instance of the user creatable object identified
> + * by @id.
> + */
> +void user_creatable_del(const char *id, Error **errp);
> +
> #endif
> diff --git a/qmp.c b/qmp.c
> index 6ae7230..9a93d5e 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -633,65 +633,13 @@ void qmp_add_client(const char *protocol, const char *fdname,
> close(fd);
> }
>
> -void object_add(const char *type, const char *id, const QDict *qdict,
> - Visitor *v, Error **errp)
> -{
> - Object *obj;
> - ObjectClass *klass;
> - const QDictEntry *e;
> - Error *local_err = NULL;
> -
> - klass = object_class_by_name(type);
> - if (!klass) {
> - error_setg(errp, "invalid object type: %s", type);
> - return;
> - }
> -
> - if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
> - error_setg(errp, "object type '%s' isn't supported by object-add",
> - type);
> - return;
> - }
> -
> - if (object_class_is_abstract(klass)) {
> - error_setg(errp, "object type '%s' is abstract", type);
> - return;
> - }
> -
> - obj = object_new(type);
> - if (qdict) {
> - for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> - object_property_set(obj, v, e->key, &local_err);
> - if (local_err) {
> - goto out;
> - }
> - }
> - }
> -
> - object_property_add_child(object_get_objects_root(),
> - id, obj, &local_err);
> - if (local_err) {
> - goto out;
> - }
> -
> - user_creatable_complete(obj, &local_err);
> - if (local_err) {
> - object_property_del(object_get_objects_root(),
> - id, &error_abort);
> - goto out;
> - }
> -out:
> - if (local_err) {
> - error_propagate(errp, local_err);
> - }
> - object_unref(obj);
> -}
>
> void qmp_object_add(const char *type, const char *id,
> bool has_props, QObject *props, Error **errp)
> {
> const QDict *pdict = NULL;
> QmpInputVisitor *qiv;
> + Object *obj;
>
> if (props) {
> pdict = qobject_to_qdict(props);
> @@ -702,27 +650,17 @@ void qmp_object_add(const char *type, const char *id,
> }
>
> qiv = qmp_input_visitor_new(props);
> - object_add(type, id, pdict, qmp_input_get_visitor(qiv), errp);
> + obj = user_creatable_add_type(type, id, pdict,
> + qmp_input_get_visitor(qiv), errp);
> qmp_input_visitor_cleanup(qiv);
> + if (obj) {
> + object_unref(obj);
> + }
> }
>
> void qmp_object_del(const char *id, Error **errp)
> {
> - Object *container;
> - Object *obj;
> -
> - container = object_get_objects_root();
> - obj = object_resolve_path_component(container, id);
> - if (!obj) {
> - error_setg(errp, "object id not found");
> - return;
> - }
> -
> - if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
> - error_setg(errp, "%s is in use, can not be deleted", id);
> - return;
> - }
> - object_unparent(obj);
> + user_creatable_del(id, errp);
> }
>
> MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index f1218f0..c2f6e29 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -1,6 +1,9 @@
> #include "qemu/osdep.h"
> #include "qom/object_interfaces.h"
> #include "qemu/module.h"
> +#include "qapi-visit.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi/opts-visitor.h"
>
> void user_creatable_complete(Object *obj, Error **errp)
> {
> @@ -31,6 +34,177 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp)
> }
> }
>
> +
> +Object *user_creatable_add(const QDict *qdict,
> + Visitor *v, Error **errp)
> +{
> + char *type = NULL;
> + char *id = NULL;
> + Object *obj = NULL;
> + Error *local_err = NULL, *end_err = NULL;
> + QDict *pdict;
> +
> + pdict = qdict_clone_shallow(qdict);
> +
> + visit_start_struct(v, NULL, NULL, 0, &local_err);
> + if (local_err) {
> + goto out;
> + }
> +
> + qdict_del(pdict, "qom-type");
> + visit_type_str(v, "qom-type", &type, &local_err);
> + if (local_err) {
> + goto out_visit;
> + }
> +
> + qdict_del(pdict, "id");
> + visit_type_str(v, "id", &id, &local_err);
> + if (local_err) {
> + goto out_visit;
> + }
> +
> + obj = user_creatable_add_type(type, id, pdict, v, &local_err);
> + if (local_err) {
> + goto out_visit;
> + }
> +
> + out_visit:
> + visit_end_struct(v, &end_err);
> + if (end_err) {
> + error_propagate(&local_err, end_err);
> + if (obj) {
> + user_creatable_del(id, NULL);
> + }
> + goto out;
> + }
> +
> +out:
> + QDECREF(pdict);
> + g_free(id);
> + g_free(type);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + object_unref(obj);
> + return NULL;
> + }
> + return obj;
> +}
> +
> +
> +Object *user_creatable_add_type(const char *type, const char *id,
> + const QDict *qdict,
> + Visitor *v, Error **errp)
> +{
> + Object *obj;
> + ObjectClass *klass;
> + const QDictEntry *e;
> + Error *local_err = NULL;
> +
> + klass = object_class_by_name(type);
> + if (!klass) {
> + error_setg(errp, "invalid object type: %s", type);
> + return NULL;
> + }
> +
> + if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
> + error_setg(errp, "object type '%s' isn't supported by object-add",
> + type);
> + return NULL;
> + }
> +
> + if (object_class_is_abstract(klass)) {
> + error_setg(errp, "object type '%s' is abstract", type);
> + return NULL;
> + }
> +
> + obj = object_new(type);
> + if (qdict) {
> + for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> + object_property_set(obj, v, e->key, &local_err);
> + if (local_err) {
> + goto out;
> + }
> + }
> + }
> +
> + object_property_add_child(object_get_objects_root(),
> + id, obj, &local_err);
> + if (local_err) {
> + goto out;
> + }
> +
> + user_creatable_complete(obj, &local_err);
> + if (local_err) {
> + object_property_del(object_get_objects_root(),
> + id, &error_abort);
> + goto out;
> + }
> +out:
> + if (local_err) {
> + error_propagate(errp, local_err);
> + object_unref(obj);
> + return NULL;
> + }
> + return obj;
> +}
> +
> +
> +Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
> +{
> + OptsVisitor *ov;
> + QDict *pdict;
> + Object *obj = NULL;
> +
> + ov = opts_visitor_new(opts);
> + pdict = qemu_opts_to_qdict(opts, NULL);
> +
> + obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
> + opts_visitor_cleanup(ov);
> + QDECREF(pdict);
> + return obj;
> +}
> +
> +
> +int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
> +{
> + bool (*type_predicate)(const char *) = opaque;
> + Object *obj = NULL;
> + const char *type;
> +
> + type = qemu_opt_get(opts, "qom-type");
> + if (type && type_predicate &&
> + !type_predicate(type)) {
> + return 0;
> + }
> +
> + obj = user_creatable_add_opts(opts, errp);
> + if (!obj) {
> + return -1;
> + }
> + object_unref(obj);
> + return 0;
> +}
> +
> +
> +void user_creatable_del(const char *id, Error **errp)
> +{
> + Object *container;
> + Object *obj;
> +
> + container = object_get_objects_root();
> + obj = object_resolve_path_component(container, id);
> + if (!obj) {
> + error_setg(errp, "object '%s' not found", id);
> + return;
> + }
> +
> + if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
> + error_setg(errp, "object '%s' is in use, can not be deleted", id);
> + return;
> + }
> + object_unparent(obj);
> +}
> +
> static void register_types(void)
> {
> static const TypeInfo uc_interface_info = {
> diff --git a/vl.c b/vl.c
> index 175ebcc..9bd881e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2816,64 +2816,6 @@ static bool object_create_delayed(const char *type)
> }
>
>
> -static int object_create(void *opaque, QemuOpts *opts, Error **errp)
> -{
> - Error *err = NULL;
> - Error *err_end = NULL;
> - char *type = NULL;
> - char *id = NULL;
> - OptsVisitor *ov;
> - QDict *pdict;
> - bool (*type_predicate)(const char *) = opaque;
> - Visitor *v;
> -
> - ov = opts_visitor_new(opts);
> - pdict = qemu_opts_to_qdict(opts, NULL);
> - v = opts_get_visitor(ov);
> -
> - visit_start_struct(v, NULL, NULL, 0, &err);
> - if (err) {
> - goto out;
> - }
> -
> - qdict_del(pdict, "qom-type");
> - visit_type_str(v, "qom-type", &type, &err);
> - if (err) {
> - goto out;
> - }
> - if (!type_predicate(type)) {
> - visit_end_struct(v, NULL);
> - goto out;
> - }
> -
> - qdict_del(pdict, "id");
> - visit_type_str(v, "id", &id, &err);
> - if (err) {
> - goto out_end;
> - }
> -
> - object_add(type, id, pdict, v, &err);
> -
> -out_end:
> - visit_end_struct(v, &err_end);
> - if (!err && err_end) {
> - qmp_object_del(id, NULL);
> - }
> - error_propagate(&err, err_end);
> -
> -out:
> - opts_visitor_cleanup(ov);
> -
> - QDECREF(pdict);
> - g_free(id);
> - g_free(type);
> - if (err) {
> - error_report_err(err);
> - return -1;
> - }
> - return 0;
> -}
> -
> static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
> MachineClass *mc)
> {
> @@ -4299,8 +4241,9 @@ int main(int argc, char **argv, char **envp)
> socket_init();
>
> if (qemu_opts_foreach(qemu_find_opts("object"),
> - object_create,
> - object_create_initial, NULL)) {
> + user_creatable_add_opts_foreach,
> + object_create_initial, &err)) {
> + error_report_err(err);
> exit(1);
> }
>
> @@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
> }
>
> if (qemu_opts_foreach(qemu_find_opts("object"),
> - object_create,
> - object_create_delayed, NULL)) {
> + user_creatable_add_opts_foreach,
> + object_create_delayed, &err)) {
> + error_report_err(err);
> exit(1);
> }
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types
2016-04-27 9:26 ` Markus Armbruster
@ 2016-04-27 9:58 ` Daniel P. Berrange
2016-04-27 12:43 ` Eric Blake
2016-04-27 13:37 ` Markus Armbruster
1 sibling, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2016-04-27 9:58 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Kevin Wolf, Paolo Bonzini, qemu-block, Eric Blake
On Wed, Apr 27, 2016 at 11:26:23AM +0200, Markus Armbruster wrote:
> This commit regresses error message quality from
>
> $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
> qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
>
> to just
>
> qemu-system-x86_64: Property '.foo' not found
I'm not seeing that behaviour myself in current git master, nor
immediately before or after 90998d58964cd17f8b0b03800b0a4508f8b543da
is applied. I always just get
$ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
So it all appears to be working correctly. How reliably reproducable
is it for you ? I'm testing on Fedora 23 x86_64 host and can't
see the failure despite many invokations.
> Clue: cur_loc points to garbage.
>
> (gdb) p cur_loc
> $1 = (Location *) 0x7fffffffdc10
> (gdb) p *cur_loc
> $2 = {kind = (unknown: 4294958128), num = 32767,
> ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}
>
> Looks like cur_loc is dangling. Happens when you forget to loc_pop() a
> Location before it dies. This one is on the stack.
>
> *Might* be release critical.
This patch doesn't even touch any code which calls loc_push/loc_pop
so I'm kind of surprised if this patch breaks it. Given that it looks
like stack corruption though, I wonder if this commit has just exposed
an already latent non-deterministic bug for you ? IOW root cause could
be an earlier patch ?
>
> For comparison, this is how it looks before the patch:
>
> (gdb) p cur_loc
> $1 = (Location *) 0x7fffffffdc10
> (gdb) p *cur_loc
> $2 = {kind = LOC_CMDLINE, num = 2, ptr = 0x7fffffffe018, prev =
> 0x5555565d2770 <std_loc>}
>
> Reported-by: Eric Blake <eblake@redhat.com>
>
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > The QMP monitor code has two helper methods object_add
> > and qmp_object_del that are called from several places
> > in the code (QMP, HMP and main emulator startup).
> >
> > The HMP and main emulator startup code also share
> > further logic that extracts the qom-type & id
> > values from a qdict.
> >
> > We soon need to use this logic from qemu-img, qemu-io
> > and qemu-nbd too, but don't want those to depend on
> > the monitor, nor do we want to duplicate the code.
> >
> > To avoid this, move some code out of qmp.c and hmp.c
> > adding new methods to qom/object_interfaces.c
> >
> > - user_creatable_add - takes a QDict holding a full
> > object definition & instantiates it
> > - user_creatable_add_type - takes an ID, type name,
> > and QDict holding object properties & instantiates
> > it
> > - user_creatable_add_opts - takes a QemuOpts holding
> > a full object definition & instantiates it
> > - user_creatable_add_opts_foreach - variant on
> > user_creatable_add_opts which can be directly used
> > in conjunction with qemu_opts_foreach.
> > - user_creatable_del - takes an ID and deletes the
> > corresponding object
> >
> > The existing code is updated to use these new methods.
> >
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > hmp.c | 52 +++---------
> > include/monitor/monitor.h | 3 -
> > include/qom/object_interfaces.h | 92 +++++++++++++++++++++
> > qmp.c | 76 ++----------------
> > qom/object_interfaces.c | 174 ++++++++++++++++++++++++++++++++++++++++
> > vl.c | 68 ++--------------
> > 6 files changed, 290 insertions(+), 175 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index c6419da..41fb9ca 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -30,6 +30,7 @@
> > #include "qapi/string-output-visitor.h"
> > #include "qapi/util.h"
> > #include "qapi-visit.h"
> > +#include "qom/object_interfaces.h"
> > #include "ui/console.h"
> > #include "block/qapi.h"
> > #include "qemu-io.h"
> > @@ -1655,58 +1656,27 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> > void hmp_object_add(Monitor *mon, const QDict *qdict)
> > {
> > Error *err = NULL;
> > - Error *err_end = NULL;
> > QemuOpts *opts;
> > - char *type = NULL;
> > - char *id = NULL;
> > OptsVisitor *ov;
> > - QDict *pdict;
> > - Visitor *v;
> > + Object *obj = NULL;
> >
> > opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
> > if (err) {
> > - goto out;
> > + hmp_handle_error(mon, &err);
> > + return;
> > }
> >
> > ov = opts_visitor_new(opts);
> > - pdict = qdict_clone_shallow(qdict);
> > - v = opts_get_visitor(ov);
> > -
> > - visit_start_struct(v, NULL, NULL, 0, &err);
> > - if (err) {
> > - goto out_clean;
> > - }
> > -
> > - qdict_del(pdict, "qom-type");
> > - visit_type_str(v, "qom-type", &type, &err);
> > - if (err) {
> > - goto out_end;
> > - }
> > + obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
> > + opts_visitor_cleanup(ov);
> > + qemu_opts_del(opts);
> >
> > - qdict_del(pdict, "id");
> > - visit_type_str(v, "id", &id, &err);
> > if (err) {
> > - goto out_end;
> > + hmp_handle_error(mon, &err);
> > }
> > -
> > - object_add(type, id, pdict, v, &err);
> > -
> > -out_end:
> > - visit_end_struct(v, &err_end);
> > - if (!err && err_end) {
> > - qmp_object_del(id, NULL);
> > + if (obj) {
> > + object_unref(obj);
> > }
> > - error_propagate(&err, err_end);
> > -out_clean:
> > - opts_visitor_cleanup(ov);
> > -
> > - QDECREF(pdict);
> > - qemu_opts_del(opts);
> > - g_free(id);
> > - g_free(type);
> > -
> > -out:
> > - hmp_handle_error(mon, &err);
> > }
> >
> > void hmp_getfd(Monitor *mon, const QDict *qdict)
> > @@ -1934,7 +1904,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
> > const char *id = qdict_get_str(qdict, "id");
> > Error *err = NULL;
> >
> > - qmp_object_del(id, &err);
> > + user_creatable_del(id, &err);
> > hmp_handle_error(mon, &err);
> > }
> >
> > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> > index 91b95ae..aa0f373 100644
> > --- a/include/monitor/monitor.h
> > +++ b/include/monitor/monitor.h
> > @@ -43,9 +43,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
> > int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> > void *opaque);
> >
> > -void object_add(const char *type, const char *id, const QDict *qdict,
> > - Visitor *v, Error **errp);
> > -
> > AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> > bool has_opaque, const char *opaque,
> > Error **errp);
> > diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> > index 283ae0d..d579746 100644
> > --- a/include/qom/object_interfaces.h
> > +++ b/include/qom/object_interfaces.h
> > @@ -2,6 +2,8 @@
> > #define OBJECT_INTERFACES_H
> >
> > #include "qom/object.h"
> > +#include "qapi/qmp/qdict.h"
> > +#include "qapi/visitor.h"
> >
> > #define TYPE_USER_CREATABLE "user-creatable"
> >
> > @@ -72,4 +74,94 @@ void user_creatable_complete(Object *obj, Error **errp);
> > * from implements USER_CREATABLE interface.
> > */
> > bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
> > +
> > +/**
> > + * user_creatable_add:
> > + * @qdict: the object definition
> > + * @v: the visitor
> > + * @errp: if an error occurs, a pointer to an area to store the error
> > + *
> > + * Create an instance of the user creatable object whose type
> > + * is defined in @qdict by the 'qom-type' field, placing it
> > + * in the object composition tree with name provided by the
> > + * 'id' field. The remaining fields in @qdict are used to
> > + * initialize the object properties.
> > + *
> > + * Returns: the newly created object or NULL on error
> > + */
> > +Object *user_creatable_add(const QDict *qdict,
> > + Visitor *v, Error **errp);
> > +
> > +/**
> > + * user_creatable_add_type:
> > + * @type: the object type name
> > + * @id: the unique ID for the object
> > + * @qdict: the object properties
> > + * @v: the visitor
> > + * @errp: if an error occurs, a pointer to an area to store the error
> > + *
> > + * Create an instance of the user creatable object @type, placing
> > + * it in the object composition tree with name @id, initializing
> > + * it with properties from @qdict
> > + *
> > + * Returns: the newly created object or NULL on error
> > + */
> > +Object *user_creatable_add_type(const char *type, const char *id,
> > + const QDict *qdict,
> > + Visitor *v, Error **errp);
> > +
> > +/**
> > + * user_creatable_add_opts:
> > + * @opts: the object definition
> > + * @errp: if an error occurs, a pointer to an area to store the error
> > + *
> > + * Create an instance of the user creatable object whose type
> > + * is defined in @opts by the 'qom-type' option, placing it
> > + * in the object composition tree with name provided by the
> > + * 'id' field. The remaining options in @opts are used to
> > + * initialize the object properties.
> > + *
> > + * Returns: the newly created object or NULL on error
> > + */
> > +Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
> > +
> > +
> > +/**
> > + * user_creatable_add_opts_predicate:
> > + * @type: the QOM type to be added
> > + *
> > + * A callback function to determine whether an object
> > + * of type @type should be created. Instances of this
> > + * callback should be passed to user_creatable_add_opts_foreach
> > + */
> > +typedef bool (*user_creatable_add_opts_predicate)(const char *type);
> > +
> > +/**
> > + * user_creatable_add_opts_foreach:
> > + * @opaque: a user_creatable_add_opts_predicate callback or NULL
> > + * @opts: options to create
> > + * @errp: if an error occurs, a pointer to an area to store the error
> > + *
> > + * An iterator callback to be used in conjunction with
> > + * the qemu_opts_foreach() method for creating a list of
> > + * objects from a set of QemuOpts
> > + *
> > + * The @opaque parameter can be passed a user_creatable_add_opts_predicate
> > + * callback to filter which types of object are created during iteration.
> > + *
> > + * Returns: 0 on success, -1 on error
> > + */
> > +int user_creatable_add_opts_foreach(void *opaque,
> > + QemuOpts *opts, Error **errp);
> > +
> > +/**
> > + * user_creatable_del:
> > + * @id: the unique ID for the object
> > + * @errp: if an error occurs, a pointer to an area to store the error
> > + *
> > + * Delete an instance of the user creatable object identified
> > + * by @id.
> > + */
> > +void user_creatable_del(const char *id, Error **errp);
> > +
> > #endif
> > diff --git a/qmp.c b/qmp.c
> > index 6ae7230..9a93d5e 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -633,65 +633,13 @@ void qmp_add_client(const char *protocol, const char *fdname,
> > close(fd);
> > }
> >
> > -void object_add(const char *type, const char *id, const QDict *qdict,
> > - Visitor *v, Error **errp)
> > -{
> > - Object *obj;
> > - ObjectClass *klass;
> > - const QDictEntry *e;
> > - Error *local_err = NULL;
> > -
> > - klass = object_class_by_name(type);
> > - if (!klass) {
> > - error_setg(errp, "invalid object type: %s", type);
> > - return;
> > - }
> > -
> > - if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
> > - error_setg(errp, "object type '%s' isn't supported by object-add",
> > - type);
> > - return;
> > - }
> > -
> > - if (object_class_is_abstract(klass)) {
> > - error_setg(errp, "object type '%s' is abstract", type);
> > - return;
> > - }
> > -
> > - obj = object_new(type);
> > - if (qdict) {
> > - for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> > - object_property_set(obj, v, e->key, &local_err);
> > - if (local_err) {
> > - goto out;
> > - }
> > - }
> > - }
> > -
> > - object_property_add_child(object_get_objects_root(),
> > - id, obj, &local_err);
> > - if (local_err) {
> > - goto out;
> > - }
> > -
> > - user_creatable_complete(obj, &local_err);
> > - if (local_err) {
> > - object_property_del(object_get_objects_root(),
> > - id, &error_abort);
> > - goto out;
> > - }
> > -out:
> > - if (local_err) {
> > - error_propagate(errp, local_err);
> > - }
> > - object_unref(obj);
> > -}
> >
> > void qmp_object_add(const char *type, const char *id,
> > bool has_props, QObject *props, Error **errp)
> > {
> > const QDict *pdict = NULL;
> > QmpInputVisitor *qiv;
> > + Object *obj;
> >
> > if (props) {
> > pdict = qobject_to_qdict(props);
> > @@ -702,27 +650,17 @@ void qmp_object_add(const char *type, const char *id,
> > }
> >
> > qiv = qmp_input_visitor_new(props);
> > - object_add(type, id, pdict, qmp_input_get_visitor(qiv), errp);
> > + obj = user_creatable_add_type(type, id, pdict,
> > + qmp_input_get_visitor(qiv), errp);
> > qmp_input_visitor_cleanup(qiv);
> > + if (obj) {
> > + object_unref(obj);
> > + }
> > }
> >
> > void qmp_object_del(const char *id, Error **errp)
> > {
> > - Object *container;
> > - Object *obj;
> > -
> > - container = object_get_objects_root();
> > - obj = object_resolve_path_component(container, id);
> > - if (!obj) {
> > - error_setg(errp, "object id not found");
> > - return;
> > - }
> > -
> > - if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
> > - error_setg(errp, "%s is in use, can not be deleted", id);
> > - return;
> > - }
> > - object_unparent(obj);
> > + user_creatable_del(id, errp);
> > }
> >
> > MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
> > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> > index f1218f0..c2f6e29 100644
> > --- a/qom/object_interfaces.c
> > +++ b/qom/object_interfaces.c
> > @@ -1,6 +1,9 @@
> > #include "qemu/osdep.h"
> > #include "qom/object_interfaces.h"
> > #include "qemu/module.h"
> > +#include "qapi-visit.h"
> > +#include "qapi/qmp-output-visitor.h"
> > +#include "qapi/opts-visitor.h"
> >
> > void user_creatable_complete(Object *obj, Error **errp)
> > {
> > @@ -31,6 +34,177 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp)
> > }
> > }
> >
> > +
> > +Object *user_creatable_add(const QDict *qdict,
> > + Visitor *v, Error **errp)
> > +{
> > + char *type = NULL;
> > + char *id = NULL;
> > + Object *obj = NULL;
> > + Error *local_err = NULL, *end_err = NULL;
> > + QDict *pdict;
> > +
> > + pdict = qdict_clone_shallow(qdict);
> > +
> > + visit_start_struct(v, NULL, NULL, 0, &local_err);
> > + if (local_err) {
> > + goto out;
> > + }
> > +
> > + qdict_del(pdict, "qom-type");
> > + visit_type_str(v, "qom-type", &type, &local_err);
> > + if (local_err) {
> > + goto out_visit;
> > + }
> > +
> > + qdict_del(pdict, "id");
> > + visit_type_str(v, "id", &id, &local_err);
> > + if (local_err) {
> > + goto out_visit;
> > + }
> > +
> > + obj = user_creatable_add_type(type, id, pdict, v, &local_err);
> > + if (local_err) {
> > + goto out_visit;
> > + }
> > +
> > + out_visit:
> > + visit_end_struct(v, &end_err);
> > + if (end_err) {
> > + error_propagate(&local_err, end_err);
> > + if (obj) {
> > + user_creatable_del(id, NULL);
> > + }
> > + goto out;
> > + }
> > +
> > +out:
> > + QDECREF(pdict);
> > + g_free(id);
> > + g_free(type);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + object_unref(obj);
> > + return NULL;
> > + }
> > + return obj;
> > +}
> > +
> > +
> > +Object *user_creatable_add_type(const char *type, const char *id,
> > + const QDict *qdict,
> > + Visitor *v, Error **errp)
> > +{
> > + Object *obj;
> > + ObjectClass *klass;
> > + const QDictEntry *e;
> > + Error *local_err = NULL;
> > +
> > + klass = object_class_by_name(type);
> > + if (!klass) {
> > + error_setg(errp, "invalid object type: %s", type);
> > + return NULL;
> > + }
> > +
> > + if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
> > + error_setg(errp, "object type '%s' isn't supported by object-add",
> > + type);
> > + return NULL;
> > + }
> > +
> > + if (object_class_is_abstract(klass)) {
> > + error_setg(errp, "object type '%s' is abstract", type);
> > + return NULL;
> > + }
> > +
> > + obj = object_new(type);
> > + if (qdict) {
> > + for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> > + object_property_set(obj, v, e->key, &local_err);
> > + if (local_err) {
> > + goto out;
> > + }
> > + }
> > + }
> > +
> > + object_property_add_child(object_get_objects_root(),
> > + id, obj, &local_err);
> > + if (local_err) {
> > + goto out;
> > + }
> > +
> > + user_creatable_complete(obj, &local_err);
> > + if (local_err) {
> > + object_property_del(object_get_objects_root(),
> > + id, &error_abort);
> > + goto out;
> > + }
> > +out:
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + object_unref(obj);
> > + return NULL;
> > + }
> > + return obj;
> > +}
> > +
> > +
> > +Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
> > +{
> > + OptsVisitor *ov;
> > + QDict *pdict;
> > + Object *obj = NULL;
> > +
> > + ov = opts_visitor_new(opts);
> > + pdict = qemu_opts_to_qdict(opts, NULL);
> > +
> > + obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
> > + opts_visitor_cleanup(ov);
> > + QDECREF(pdict);
> > + return obj;
> > +}
> > +
> > +
> > +int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
> > +{
> > + bool (*type_predicate)(const char *) = opaque;
> > + Object *obj = NULL;
> > + const char *type;
> > +
> > + type = qemu_opt_get(opts, "qom-type");
> > + if (type && type_predicate &&
> > + !type_predicate(type)) {
> > + return 0;
> > + }
> > +
> > + obj = user_creatable_add_opts(opts, errp);
> > + if (!obj) {
> > + return -1;
> > + }
> > + object_unref(obj);
> > + return 0;
> > +}
> > +
> > +
> > +void user_creatable_del(const char *id, Error **errp)
> > +{
> > + Object *container;
> > + Object *obj;
> > +
> > + container = object_get_objects_root();
> > + obj = object_resolve_path_component(container, id);
> > + if (!obj) {
> > + error_setg(errp, "object '%s' not found", id);
> > + return;
> > + }
> > +
> > + if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
> > + error_setg(errp, "object '%s' is in use, can not be deleted", id);
> > + return;
> > + }
> > + object_unparent(obj);
> > +}
> > +
> > static void register_types(void)
> > {
> > static const TypeInfo uc_interface_info = {
> > diff --git a/vl.c b/vl.c
> > index 175ebcc..9bd881e 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2816,64 +2816,6 @@ static bool object_create_delayed(const char *type)
> > }
> >
> >
> > -static int object_create(void *opaque, QemuOpts *opts, Error **errp)
> > -{
> > - Error *err = NULL;
> > - Error *err_end = NULL;
> > - char *type = NULL;
> > - char *id = NULL;
> > - OptsVisitor *ov;
> > - QDict *pdict;
> > - bool (*type_predicate)(const char *) = opaque;
> > - Visitor *v;
> > -
> > - ov = opts_visitor_new(opts);
> > - pdict = qemu_opts_to_qdict(opts, NULL);
> > - v = opts_get_visitor(ov);
> > -
> > - visit_start_struct(v, NULL, NULL, 0, &err);
> > - if (err) {
> > - goto out;
> > - }
> > -
> > - qdict_del(pdict, "qom-type");
> > - visit_type_str(v, "qom-type", &type, &err);
> > - if (err) {
> > - goto out;
> > - }
> > - if (!type_predicate(type)) {
> > - visit_end_struct(v, NULL);
> > - goto out;
> > - }
> > -
> > - qdict_del(pdict, "id");
> > - visit_type_str(v, "id", &id, &err);
> > - if (err) {
> > - goto out_end;
> > - }
> > -
> > - object_add(type, id, pdict, v, &err);
> > -
> > -out_end:
> > - visit_end_struct(v, &err_end);
> > - if (!err && err_end) {
> > - qmp_object_del(id, NULL);
> > - }
> > - error_propagate(&err, err_end);
> > -
> > -out:
> > - opts_visitor_cleanup(ov);
> > -
> > - QDECREF(pdict);
> > - g_free(id);
> > - g_free(type);
> > - if (err) {
> > - error_report_err(err);
> > - return -1;
> > - }
> > - return 0;
> > -}
> > -
> > static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
> > MachineClass *mc)
> > {
> > @@ -4299,8 +4241,9 @@ int main(int argc, char **argv, char **envp)
> > socket_init();
> >
> > if (qemu_opts_foreach(qemu_find_opts("object"),
> > - object_create,
> > - object_create_initial, NULL)) {
> > + user_creatable_add_opts_foreach,
> > + object_create_initial, &err)) {
> > + error_report_err(err);
> > exit(1);
> > }
> >
> > @@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
> > }
> >
> > if (qemu_opts_foreach(qemu_find_opts("object"),
> > - object_create,
> > - object_create_delayed, NULL)) {
> > + user_creatable_add_opts_foreach,
> > + object_create_delayed, &err)) {
> > + error_report_err(err);
> > exit(1);
> > }
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types
2016-04-27 9:58 ` Daniel P. Berrange
@ 2016-04-27 12:43 ` Eric Blake
2016-04-27 14:55 ` Daniel P. Berrange
0 siblings, 1 reply; 20+ messages in thread
From: Eric Blake @ 2016-04-27 12:43 UTC (permalink / raw)
To: Daniel P. Berrange, Markus Armbruster
Cc: qemu-devel, Kevin Wolf, Paolo Bonzini, qemu-block
[-- Attachment #1: Type: text/plain, Size: 3096 bytes --]
On 04/27/2016 03:58 AM, Daniel P. Berrange wrote:
> On Wed, Apr 27, 2016 at 11:26:23AM +0200, Markus Armbruster wrote:
>> This commit regresses error message quality from
>>
>> $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
>> qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
>>
>> to just
>>
>> qemu-system-x86_64: Property '.foo' not found
>
> I'm not seeing that behaviour myself in current git master, nor
> immediately before or after 90998d58964cd17f8b0b03800b0a4508f8b543da
> is applied. I always just get
>
> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
> qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
>
> So it all appears to be working correctly. How reliably reproducable
> is it for you ? I'm testing on Fedora 23 x86_64 host and can't
> see the failure despite many invokations.
I'm reproducing it on my F23 machine, where 90998d58 indeed flips the
behavior I'm seeing. Maybe it's a factor of which malloc engine is in
use, or level of compiler optimization?
My config.status states:
exec '/home/eblake/qemu/configure' '--enable-kvm' '--enable-system'
'--disable-user' '--target-list=x86_64-softmmu,ppc64-softmmu'
'--enable-debug'
>
>> Clue: cur_loc points to garbage.
>>
>> (gdb) p cur_loc
>> $1 = (Location *) 0x7fffffffdc10
>> (gdb) p *cur_loc
>> $2 = {kind = (unknown: 4294958128), num = 32767,
>> ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}
>>
>> Looks like cur_loc is dangling. Happens when you forget to loc_pop() a
>> Location before it dies. This one is on the stack.
>>
>> *Might* be release critical.
>
> This patch doesn't even touch any code which calls loc_push/loc_pop
> so I'm kind of surprised if this patch breaks it. Given that it looks
> like stack corruption though, I wonder if this commit has just exposed
> an already latent non-deterministic bug for you ? IOW root cause could
> be an earlier patch ?
Could it be a latent bug in qemu_opts_foreach()? Your patch changes a
call from qemu_opts_foreach(object_create) to
qemu_opts_foreach(user_creatable_add_opts_foreach), where the new
callback may expose different behavior to the stack and thus expose the
latent problem.
>>> @@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
>>> }
>>>
>>> if (qemu_opts_foreach(qemu_find_opts("object"),
>>> - object_create,
>>> - object_create_delayed, NULL)) {
>>> + user_creatable_add_opts_foreach,
>>> + object_create_delayed, &err)) {
>>> + error_report_err(err);
>>> exit(1);
>>> }
>
> Regards,
> Daniel
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types
2016-04-27 9:26 ` Markus Armbruster
2016-04-27 9:58 ` Daniel P. Berrange
@ 2016-04-27 13:37 ` Markus Armbruster
1 sibling, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2016-04-27 13:37 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Kevin Wolf, Paolo Bonzini, qemu-block, Eric Blake
Markus Armbruster <armbru@redhat.com> writes:
> This commit regresses error message quality from
>
> $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
> qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
>
> to just
>
> qemu-system-x86_64: Property '.foo' not found
>
> Clue: cur_loc points to garbage.
>
> (gdb) p cur_loc
> $1 = (Location *) 0x7fffffffdc10
> (gdb) p *cur_loc
> $2 = {kind = (unknown: 4294958128), num = 32767,
> ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}
>
> Looks like cur_loc is dangling. Happens when you forget to loc_pop() a
> Location before it dies. This one is on the stack.
>
> *Might* be release critical.
>
> For comparison, this is how it looks before the patch:
>
> (gdb) p cur_loc
> $1 = (Location *) 0x7fffffffdc10
> (gdb) p *cur_loc
> $2 = {kind = LOC_CMDLINE, num = 2, ptr = 0x7fffffffe018, prev =
> 0x5555565d2770 <std_loc>}
>
> Reported-by: Eric Blake <eblake@redhat.com>
I think I nailed it. Preparing patches...
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types
2016-04-27 12:43 ` Eric Blake
@ 2016-04-27 14:55 ` Daniel P. Berrange
0 siblings, 0 replies; 20+ messages in thread
From: Daniel P. Berrange @ 2016-04-27 14:55 UTC (permalink / raw)
To: Eric Blake
Cc: Markus Armbruster, qemu-devel, Kevin Wolf, Paolo Bonzini, qemu-block
On Wed, Apr 27, 2016 at 06:43:43AM -0600, Eric Blake wrote:
> On 04/27/2016 03:58 AM, Daniel P. Berrange wrote:
> > On Wed, Apr 27, 2016 at 11:26:23AM +0200, Markus Armbruster wrote:
> >> This commit regresses error message quality from
> >>
> >> $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
> >> qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
> >>
> >> to just
> >>
> >> qemu-system-x86_64: Property '.foo' not found
> >
> > I'm not seeing that behaviour myself in current git master, nor
> > immediately before or after 90998d58964cd17f8b0b03800b0a4508f8b543da
> > is applied. I always just get
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
> > qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
> >
> > So it all appears to be working correctly. How reliably reproducable
> > is it for you ? I'm testing on Fedora 23 x86_64 host and can't
> > see the failure despite many invokations.
>
> I'm reproducing it on my F23 machine, where 90998d58 indeed flips the
> behavior I'm seeing. Maybe it's a factor of which malloc engine is in
> use, or level of compiler optimization?
>
> My config.status states:
> exec '/home/eblake/qemu/configure' '--enable-kvm' '--enable-system'
> '--disable-user' '--target-list=x86_64-softmmu,ppc64-softmmu'
> '--enable-debug'
For the record, the --enable-debug flag was the key to making this
bug reproducable
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-04-27 14:55 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-15 14:33 [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
2016-04-27 9:26 ` Markus Armbruster
2016-04-27 9:58 ` Daniel P. Berrange
2016-04-27 12:43 ` Eric Blake
2016-04-27 14:55 ` Daniel P. Berrange
2016-04-27 13:37 ` Markus Armbruster
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 02/10] qemu-io: add support for --object command line arg Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 03/10] qemu-nbd: " Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 04/10] qemu-img: " Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 06/10] qemu-nbd: " Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 07/10] qemu-img: " Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 08/10] qemu-nbd: don't overlap long option values with short options Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 09/10] qemu-nbd: use no_argument/required_argument constants Daniel P. Berrange
2016-02-15 14:33 ` [Qemu-devel] [PATCH v6 10/10] qemu-io: " Daniel P. Berrange
2016-02-15 15:53 ` [Qemu-devel] [PATCH v6 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
2016-02-17 10:10 ` Daniel P. Berrange
2016-02-17 10:37 ` Kevin Wolf
2016-02-17 10:41 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).