* [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
@ 2016-02-02 12:57 Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
` (10 more replies)
0 siblings, 11 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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
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
The first 4 patches in this series are a pre-requisite for
3 other series
- Support for TLS in NBD
- Support for secrets for passwd auth in curl, rbd, iscsi
(fixes a CVE issue in libvirt)
- Support for LUKS encryption passwords
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-img: add support for --object command line arg
qemu-nbd: add support for --object command line arg
qemu-io: 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 | 433 +++++++++++++++++++++++++++++++++++++---
qemu-img.texi | 14 ++
qemu-io.c | 92 +++++++--
qemu-nbd.c | 127 +++++++++---
qemu-nbd.texi | 6 +
qmp.c | 76 +------
qom/object_interfaces.c | 180 +++++++++++++++++
vl.c | 66 +-----
12 files changed, 916 insertions(+), 269 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-02 14:47 ` Andreas Färber
2016-02-02 23:38 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg Daniel P. Berrange
` (9 subsequent siblings)
10 siblings, 2 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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.
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 | 180 ++++++++++++++++++++++++++++++++++++++++
vl.c | 66 ++-------------
6 files changed, 296 insertions(+), 173 deletions(-)
diff --git a/hmp.c b/hmp.c
index 54f2620..95930b0 100644
--- a/hmp.c
+++ b/hmp.c
@@ -29,6 +29,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"
@@ -1652,58 +1653,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;
- void *dummy = NULL;
OptsVisitor *ov;
- QDict *pdict;
+ 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);
-
- visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
- if (err) {
- goto out_clean;
- }
-
- qdict_del(pdict, "qom-type");
- visit_type_str(opts_get_visitor(ov), &type, "qom-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(opts_get_visitor(ov), &id, "id", &err);
if (err) {
- goto out_end;
+ hmp_handle_error(mon, &err);
}
-
- object_add(type, id, pdict, opts_get_visitor(ov), &err);
-
-out_end:
- visit_end_struct(opts_get_visitor(ov), &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);
- g_free(dummy);
-
-out:
- hmp_handle_error(mon, &err);
}
void hmp_getfd(Monitor *mon, const QDict *qdict)
@@ -1933,7 +1903,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..f50e404 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 53affe2..f4c12b3 100644
--- a/qmp.c
+++ b/qmp.c
@@ -632,65 +632,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);
@@ -701,27 +649,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 a66cd60..7951f97 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -1,5 +1,8 @@
#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)
{
@@ -30,6 +33,183 @@ 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, NULL, 0, &local_err);
+ if (local_err) {
+ goto out;
+ }
+
+ qdict_del(pdict, "qom-type");
+ visit_type_str(v, &type, "qom-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) {
+ if (!local_err) {
+ error_propagate(&local_err, end_err);
+ } else {
+ error_free(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);
+ if (obj) {
+ 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 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);
+}
+
static void register_types(void)
{
static const TypeInfo uc_interface_info = {
diff --git a/vl.c b/vl.c
index f043009..a6ed2f0 100644
--- a/vl.c
+++ b/vl.c
@@ -2819,62 +2819,6 @@ static bool object_create_delayed(const char *type)
}
-static int object_create(void *opaque, QemuOpts *opts, Error **errp)
-{
- Error *err = NULL;
- char *type = NULL;
- char *id = NULL;
- void *dummy = NULL;
- OptsVisitor *ov;
- QDict *pdict;
- bool (*type_predicate)(const char *) = opaque;
-
- ov = opts_visitor_new(opts);
- pdict = qemu_opts_to_qdict(opts, NULL);
-
- visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
- if (err) {
- goto out;
- }
-
- qdict_del(pdict, "qom-type");
- visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
- if (err) {
- goto out;
- }
- if (!type_predicate(type)) {
- goto out;
- }
-
- qdict_del(pdict, "id");
- visit_type_str(opts_get_visitor(ov), &id, "id", &err);
- if (err) {
- goto out;
- }
-
- object_add(type, id, pdict, opts_get_visitor(ov), &err);
- if (err) {
- goto out;
- }
- visit_end_struct(opts_get_visitor(ov), &err);
- if (err) {
- qmp_object_del(id, NULL);
- }
-
-out:
- opts_visitor_cleanup(ov);
-
- QDECREF(pdict);
- g_free(id);
- g_free(type);
- g_free(dummy);
- 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)
{
@@ -4286,8 +4230,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);
}
@@ -4404,8 +4349,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] 30+ messages in thread
* [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-03 0:24 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 03/10] qemu-nbd: " Daniel P. Berrange
` (8 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-img-cmds.hx | 44 ++++-----
qemu-img.c | 269 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
qemu-img.texi | 8 ++
3 files changed, 291 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 33e451c..524b64f 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 @code{qemu(1)}\n"
+ " manual page for a description of the object properties. The common object\n"
+ " type that it makes sense to define is 'secret' object, which is used to\n"
+ " supply passwords and/or 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,14 @@ static int img_create(int argc, char **argv)
bool quiet = false;
for(;;) {
- c = getopt(argc, argv, "F:b:f:he6o:q");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -317,6 +340,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 +363,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 +527,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 +539,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 +576,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 +600,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 +730,14 @@ static int img_commit(int argc, char **argv)
cache = BDRV_DEFAULT_CACHE;
base = NULL;
for(;;) {
- c = getopt(argc, argv, "f:ht:b:dpq");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -704,6 +766,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 +787,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 +1050,18 @@ 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");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -1003,6 +1088,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 +1112,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);
@@ -1223,6 +1325,7 @@ out2:
blk_unref(blk1);
out3:
qemu_progress_end();
+out4:
return ret;
}
@@ -1552,7 +1655,14 @@ 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");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -1643,9 +1753,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;
@@ -2075,6 +2199,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;
@@ -2085,6 +2210,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",
@@ -2106,6 +2232,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) {
@@ -2122,6 +2256,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;
@@ -2245,6 +2386,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;
@@ -2254,6 +2396,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",
@@ -2272,6 +2415,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) {
@@ -2288,6 +2439,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;
@@ -2357,7 +2515,14 @@ 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");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -2401,6 +2566,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;
}
}
@@ -2409,6 +2582,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) {
@@ -2482,7 +2662,14 @@ 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");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -2515,6 +2702,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;
}
}
@@ -2530,6 +2725,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);
@@ -2790,6 +2992,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),
@@ -2816,7 +3020,14 @@ static int img_resize(int argc, char **argv)
/* Parse getopt arguments */
fmt = NULL;
for(;;) {
- c = getopt(argc, argv, "f:hq");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -2831,6 +3042,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) {
@@ -2838,6 +3057,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 '+':
@@ -2925,10 +3151,18 @@ 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");
+ int option_index = 0;
+ 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, &option_index);
if (c == -1) {
break;
}
@@ -2964,6 +3198,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;
}
}
@@ -2971,6 +3213,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;
}
@@ -3093,6 +3343,9 @@ int main(int argc, char **argv)
}
cmdname = argv[1];
+ module_call_init(MODULE_INIT_QOM);
+ 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..612f15a 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 only object type that
+it makes sense to define is the @code{secret} object, 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] 30+ messages in thread
* [Qemu-devel] [PATCH v5 03/10] qemu-nbd: add support for --object command line arg
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-03 2:33 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 04/10] qemu-io: " Daniel P. Berrange
` (7 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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...
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 ede4a54..0e019c1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -23,9 +23,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 <stdarg.h>
#include <stdio.h>
@@ -44,6 +46,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;
@@ -77,6 +80,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"
@@ -374,6 +380,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;
@@ -411,6 +427,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;
@@ -436,6 +453,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) {
@@ -591,6 +610,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;
}
}
@@ -600,6 +627,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 46fd483..9f9daca 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -14,6 +14,12 @@ Export QEMU disk image using NBD protocol.
@table @option
@item @var{filename}
is a disk image filename
+@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}
port to listen on (default @samp{10809})
@item -o, --offset=@var{offset}
--
2.5.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v5 04/10] qemu-io: add support for --object command line arg
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (2 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 03/10] qemu-nbd: " Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-03 2:42 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
` (6 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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...
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 d593f19..65a28e4 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] 30+ messages in thread
* [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (3 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 04/10] qemu-io: " Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-03 15:37 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 06/10] qemu-nbd: " Daniel P. Berrange
` (5 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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
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=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-io.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/qemu-io.c b/qemu-io.c
index 65a28e4..3544fd7 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -366,6 +366,7 @@ static void reenable_tty_echo(void)
enum {
OPTION_OBJECT = 256,
+ OPTION_IMAGE_OPTS = 257,
};
static QemuOptsList qemu_object_opts = {
@@ -378,6 +379,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 +408,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 +416,7 @@ int main(int argc, char **argv)
int flags = BDRV_O_UNMAP;
Error *local_error = NULL;
QDict *opts = NULL;
+ bool imageOpts = false;
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -473,6 +486,9 @@ int main(int argc, char **argv)
exit(1);
}
} break;
+ case OPTION_IMAGE_OPTS:
+ imageOpts = true;
+ break;
default:
usage(progname);
exit(1);
@@ -515,7 +531,20 @@ int main(int argc, char **argv)
flags |= BDRV_O_RDWR;
}
- if ((argc - optind) == 1) {
+ if (imageOpts) {
+ QemuOpts *qopts;
+ qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
+ if (!qopts) {
+ exit(1);
+ }
+ if (opts) {
+ error_report("--image-opts and -f are mutually exclusive");
+ exit(1);
+ }
+ opts = qemu_opts_to_qdict(qopts, NULL);
+ qemu_opts_reset(&file_opts);
+ openfile(NULL, flags, opts);
+ } else if ((argc - optind) == 1) {
openfile(argv[optind], flags, opts);
}
command_loop();
--
2.5.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v5 06/10] qemu-nbd: allow specifying image as a set of options args
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (4 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-03 15:47 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 07/10] qemu-img: " Daniel P. Berrange
` (4 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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 | 42 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 5 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 0e019c1..ee91e47 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -47,6 +47,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 6
static NBDExport *exp;
static int verbose;
@@ -380,6 +381,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",
@@ -428,6 +439,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;
@@ -445,6 +457,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.
@@ -618,6 +631,9 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
} break;
+ case QEMU_NBD_OPT_IMAGE_OPTS:
+ imageOpts = true;
+ break;
}
}
@@ -724,13 +740,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]);
--
2.5.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (5 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 06/10] qemu-nbd: " Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-04 15:42 ` Kevin Wolf
2016-02-04 15:59 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 08/10] qemu-nbd: don't overlap long option values with short options Daniel P. Berrange
` (3 subsequent siblings)
10 siblings, 2 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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 | 164 +++++++++++++++++++++++++++++++++++++++++++++++--------
qemu-img.texi | 6 ++
3 files changed, 170 insertions(+), 44 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 524b64f..0c54fe6 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,9 +222,31 @@ 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 BlockBackend *img_open_opts(const char *id,
+ QemuOpts *opts, int flags)
+{
+ QDict *options;
+ Error *local_err = NULL;
+ char *file = NULL;
+ BlockBackend *blk;
+ file = g_strdup(qemu_opt_get(opts, "file"));
+ qemu_opt_unset(opts, "file");
+ options = qemu_opts_to_qdict(opts, NULL);
+ blk = blk_new_open(id, file, NULL, options, flags, &local_err);
+ if (!blk) {
+ error_report("Could not open '%s': %s", file ? file : "",
+ error_get_pretty(local_err));
+ g_free(file);
+ error_free(local_err);
+ return NULL;
+ }
+ g_free(file);
+ 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;
BlockDriverState *bs;
@@ -251,6 +283,33 @@ fail:
return NULL;
}
+
+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("image", opts, flags);
+ } else {
+ blk = img_open_file("image", filename, fmt, flags, true, quiet);
+ }
+ return blk;
+}
+
+
static int add_old_style_options(const char *fmt, QemuOpts *opts,
const char *base_filename,
const char *base_fmt)
@@ -528,6 +587,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;
@@ -540,6 +600,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",
@@ -584,6 +645,9 @@ static int img_check(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -613,7 +677,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;
}
@@ -725,6 +789,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;
@@ -734,6 +799,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",
@@ -774,6 +840,9 @@ static int img_commit(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -801,7 +870,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;
}
@@ -1051,6 +1120,7 @@ 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 (;;) {
@@ -1058,6 +1128,7 @@ static int img_compare(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:T:pqs",
@@ -1097,6 +1168,9 @@ static int img_compare(int argc, char **argv)
goto out4;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -1131,18 +1205,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 +1720,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";
@@ -1659,6 +1734,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",
@@ -1760,6 +1836,9 @@ static int img_convert(int argc, char **argv)
goto fail_getopt;
}
break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -1776,7 +1855,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;
@@ -1814,8 +1892,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;
@@ -1956,7 +2034,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;
@@ -2123,7 +2207,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)
{
@@ -2147,8 +2232,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;
}
@@ -2200,6 +2286,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;
@@ -2211,6 +2298,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",
@@ -2240,6 +2328,9 @@ static int img_info(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -2263,7 +2354,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;
}
@@ -2387,6 +2478,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;
@@ -2397,6 +2489,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",
@@ -2423,6 +2516,9 @@ static int img_map(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -2446,7 +2542,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;
}
@@ -2511,6 +2608,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 */
@@ -2519,6 +2617,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",
@@ -2574,6 +2673,9 @@ static int img_snapshot(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -2590,7 +2692,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;
}
@@ -2654,6 +2757,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;
@@ -2666,6 +2770,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",
@@ -2710,6 +2815,9 @@ static int img_rebase(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -2720,6 +2828,7 @@ static int img_rebase(int argc, char **argv)
if (optind != argc - 1) {
error_exit("Expecting one image file name");
}
+
if (!unsafe && !out_baseimg) {
error_exit("Must specify backing file (-b) or use unsafe mode (-u)");
}
@@ -2755,7 +2864,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;
@@ -3007,6 +3116,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. */
@@ -3024,6 +3134,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",
@@ -3050,6 +3161,9 @@ static int img_resize(int argc, char **argv)
return 1;
}
} break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
if (optind != argc - 1) {
@@ -3091,8 +3205,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;
@@ -3152,6 +3266,7 @@ 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 (;;) {
@@ -3159,6 +3274,7 @@ static int img_amend(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, "ho:f:t:pq",
@@ -3206,6 +3322,9 @@ static int img_amend(int argc, char **argv)
goto out_no_progress;
}
break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
}
}
@@ -3247,7 +3366,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;
@@ -3345,6 +3464,7 @@ int main(int argc, char **argv)
module_call_init(MODULE_INIT_QOM);
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 612f15a..da599c8 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -32,6 +32,12 @@ page for a description of the object properties. The only object type that
it makes sense to define is the @code{secret} object, 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] 30+ messages in thread
* [Qemu-devel] [PATCH v5 08/10] qemu-nbd: don't overlap long option values with short options
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (6 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 07/10] qemu-img: " Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 09/10] qemu-nbd: use no_argument/required_argument constants Daniel P. Berrange
` (2 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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 ee91e47..026cf86 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -42,12 +42,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 6
+#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] 30+ messages in thread
* [Qemu-devel] [PATCH v5 09/10] qemu-nbd: use no_argument/required_argument constants
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (7 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 08/10] qemu-nbd: don't overlap long option values with short options Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 10/10] qemu-io: " Daniel P. Berrange
2016-02-04 15:44 ` [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
10 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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 026cf86..54d579c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -417,29 +417,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] 30+ messages in thread
* [Qemu-devel] [PATCH v5 10/10] qemu-io: use no_argument/required_argument constants
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (8 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 09/10] qemu-nbd: use no_argument/required_argument constants Daniel P. Berrange
@ 2016-02-02 12:57 ` Daniel P. Berrange
2016-02-04 15:44 ` [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
10 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Paolo Bonzini,
Andreas Färber
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 3544fd7..d94dad1 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -394,21 +394,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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
@ 2016-02-02 14:47 ` Andreas Färber
2016-02-02 23:38 ` Eric Blake
1 sibling, 0 replies; 30+ messages in thread
From: Andreas Färber @ 2016-02-02 14:47 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel, Paolo Bonzini, Stefan Hajnoczi
Cc: Kevin Wolf, Markus Armbruster, qemu-block
Am 02.02.2016 um 13:57 schrieb Daniel P. Berrange:
> 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.
>
> 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 | 180 ++++++++++++++++++++++++++++++++++++++++
> vl.c | 66 ++-------------
> 6 files changed, 296 insertions(+), 173 deletions(-)
No objections from my side, looks sane, but I'd appreciate Paolo/Stefan
to ack since I wasn't really involved in that code.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
2016-02-02 14:47 ` Andreas Färber
@ 2016-02-02 23:38 ` Eric Blake
2016-02-02 23:41 ` Andreas Färber
1 sibling, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-02-02 23:38 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 7965 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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.
Yay - merge conflicts with my work pending on Markus' qapi-next branch:
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00254.html
>
> 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.
>
> 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 | 180 ++++++++++++++++++++++++++++++++++++++++
> vl.c | 66 ++-------------
> 6 files changed, 296 insertions(+), 173 deletions(-)
diffstat is misleading, overall I think this is a nice cleanup.
>
> diff --git a/hmp.c b/hmp.c
> index 54f2620..95930b0 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -29,6 +29,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"
Any reason to name the file with _ instead of -? It looks funny in
relation to the other three files in just this hunk...
> +++ 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"
...Oh - the file is pre-existing. If we want to rename it now, it
should be an independent patch.
> +/**
> + * 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,
s/type,/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
Not the first time that I've noticed that you're not a fan of trailing
'.' in paragraphs :) I won't hold it against you
> + *
> + * 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,
s/type,/type/
> @@ -701,27 +649,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);
> + }
'if' is redundant; object_unref(NULL) is safe.
> +++ b/qom/object_interfaces.c
> @@ -1,5 +1,8 @@
> #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)
> {
> @@ -30,6 +33,183 @@ 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, NULL, 0, &local_err);
Yep, conflicts with my pending patches. One less NULL needed if mine
land first.
> + if (local_err) {
> + goto out;
> + }
> +
> + qdict_del(pdict, "qom-type");
> + visit_type_str(v, &type, "qom-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) {
> + if (!local_err) {
> + error_propagate(&local_err, end_err);
> + } else {
> + error_free(end_err);
> + }
This 'if/else' can be simplified to:
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);
> + if (obj) {
> + 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;
> +}
Nice way to share code.
> +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 id not found");
Might be worth a "%s",id in there somewhere.
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types
2016-02-02 23:38 ` Eric Blake
@ 2016-02-02 23:41 ` Andreas Färber
2016-02-03 0:15 ` Eric Blake
0 siblings, 1 reply; 30+ messages in thread
From: Andreas Färber @ 2016-02-02 23:41 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
Am 03.02.2016 um 00:38 schrieb Eric Blake:
> On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
>> 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.
>
> Yay - merge conflicts with my work pending on Markus' qapi-next branch:
> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00254.html
Did I review the wrong patch then? I assumed this was the one I was
asked to look at just before FOSDEM...
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types
2016-02-02 23:41 ` Andreas Färber
@ 2016-02-03 0:15 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-02-03 0:15 UTC (permalink / raw)
To: Andreas Färber
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]
On 02/02/2016 04:41 PM, Andreas Färber wrote:
> Am 03.02.2016 um 00:38 schrieb Eric Blake:
>> On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
>>> 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.
>>
>> Yay - merge conflicts with my work pending on Markus' qapi-next branch:
>> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00254.html
>
> Did I review the wrong patch then? I assumed this was the one I was
> asked to look at just before FOSDEM...
No, you got the right patch; it's just that I've been independently
improving the code in hmp.c and vl.c that Dan is now refactoring, so at
least one of us will have to rebase.
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg Daniel P. Berrange
@ 2016-02-03 0:24 ` Eric Blake
2016-02-03 10:09 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-02-03 0:24 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 3430 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qemu-img-cmds.hx | 44 ++++-----
> qemu-img.c | 269 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> qemu-img.texi | 8 ++
> 3 files changed, 291 insertions(+), 30 deletions(-)
>
> +++ b/qemu-img.c
> @@ -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 @code{qemu(1)}\n"
Drop @code; this is the --help text.
> + " manual page for a description of the object properties. The common object\n"
> + " type that it makes sense to define is 'secret' object, which is used to\n"
s/is/is a/
or maybe go for something shorter:
The most common object type is a 'secret', which is used...
or match the text you put in the info:
The only object type that it makes sense to define is the 'secret'
object, which is used...
> @@ -275,7 +291,14 @@ static int img_create(int argc, char **argv)
> bool quiet = false;
>
> for(;;) {
> - c = getopt(argc, argv, "F:b:f:he6o:q");
> + int option_index = 0;
> + 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, &option_index);
Can't you pass NULL for the last parameter, if you aren't going to use
option_index in your error reporting?
> @@ -675,7 +730,14 @@ static int img_commit(int argc, char **argv)
> cache = BDRV_DEFAULT_CACHE;
> base = NULL;
> for(;;) {
> - c = getopt(argc, argv, "f:ht:b:dpq");
> + int option_index = 0;
> + 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, &option_index);
more than once. I'll quit pointing it out. Doesn't affect behavior
either way.
> +++ 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 only object type that
> +it makes sense to define is the @code{secret} object, which is used to
> +supply passwords and/or encryption keys.
With the help text fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 03/10] qemu-nbd: add support for --object command line arg
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 03/10] qemu-nbd: " Daniel P. Berrange
@ 2016-02-03 2:33 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-02-03 2:33 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 790 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qemu-nbd.c | 34 ++++++++++++++++++++++++++++++++++
> qemu-nbd.texi | 6 ++++++
> 2 files changed, 40 insertions(+)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 04/10] qemu-io: add support for --object command line arg
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 04/10] qemu-io: " Daniel P. Berrange
@ 2016-02-03 2:42 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-02-03 2:42 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 750 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qemu-io.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg
2016-02-03 0:24 ` Eric Blake
@ 2016-02-03 10:09 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-03 10:09 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster,
Paolo Bonzini, Andreas Färber
On Tue, Feb 02, 2016 at 05:24:32PM -0700, Eric Blake wrote:
> On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> > 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...
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > qemu-img-cmds.hx | 44 ++++-----
> > qemu-img.c | 269 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > qemu-img.texi | 8 ++
> > 3 files changed, 291 insertions(+), 30 deletions(-)
> >
>
> > +++ b/qemu-img.c
> > @@ -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 @code{qemu(1)}\n"
>
> Drop @code; this is the --help text.
>
> > + " manual page for a description of the object properties. The common object\n"
> > + " type that it makes sense to define is 'secret' object, which is used to\n"
>
> s/is/is a/
>
> or maybe go for something shorter:
>
> The most common object type is a 'secret', which is used...
>
> or match the text you put in the info:
>
> The only object type that it makes sense to define is the 'secret'
> object, which is used...
>
> > @@ -275,7 +291,14 @@ static int img_create(int argc, char **argv)
> > bool quiet = false;
> >
> > for(;;) {
> > - c = getopt(argc, argv, "F:b:f:he6o:q");
> > + int option_index = 0;
> > + 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, &option_index);
>
> Can't you pass NULL for the last parameter, if you aren't going to use
> option_index in your error reporting?
Oh, I didn't realize that it allowed NULL for that parameter.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
@ 2016-02-03 15:37 ` Eric Blake
2016-02-03 17:13 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-02-03 15:37 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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
>
> 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=file,filename=/home/berrange/demo.qcow2
>
> This flag is mutually exclusive with the '-f' flag.
I guess it's easier to enforce the mutual exclusion, than it is to try
and figure out how to make -f work with the --image-opts filename as
long as the two aren't specifying conflicting formats. Seems okay as
long as it is documented well.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qemu-io.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> @@ -515,7 +531,20 @@ int main(int argc, char **argv)
> flags |= BDRV_O_RDWR;
> }
>
> - if ((argc - optind) == 1) {
> + if (imageOpts) {
> + QemuOpts *qopts;
> + qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
Ouch. If argc == optind (possible if I type 'qemu-io --image-opts'
without a filename), then argv[optind] == NULL, and you end up calling
strncmp(NULL, "id=", 3) inside opts_parse().
Also, I noticed that running 'qemu-io' without arguments puts you into a
shell mode, where you can then open files after the fact via the
open_f() callback function (the 'open' command) - either that function
should that function be given --image-opts support, or use of
--image-opts from the command line should globally affect all subsequent
use of open_f().
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 06/10] qemu-nbd: allow specifying image as a set of options args
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 06/10] qemu-nbd: " Daniel P. Berrange
@ 2016-02-03 15:47 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-02-03 15:47 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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 | 42 +++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 37 insertions(+), 5 deletions(-)
Where is this new option documented? At a minimum, 'qemu-nbd --help'
should mention it. If later in the series, mention that in the commit
message.
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 0e019c1..ee91e47 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -47,6 +47,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 6
Churn here where 8/10 has to touch the same line; but I'm not sure
rearranging the series is worth the effort, so I don't mind it.
> @@ -724,13 +740,29 @@ int main(int argc, char **argv)
> bdrv_init();
> atexit(bdrv_close_all);
There's an earlier use of argv[optind] for the --disconnect option;
should that code be tweaked at all, or is it always safe for that path
to blindly open(name) without trying to parse options?
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args
2016-02-03 15:37 ` Eric Blake
@ 2016-02-03 17:13 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-03 17:13 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster,
Paolo Bonzini, Andreas Färber
On Wed, Feb 03, 2016 at 08:37:15AM -0700, Eric Blake wrote:
> On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> > 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
> >
> > 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=file,filename=/home/berrange/demo.qcow2
> >
> > This flag is mutually exclusive with the '-f' flag.
>
> I guess it's easier to enforce the mutual exclusion, than it is to try
> and figure out how to make -f work with the --image-opts filename as
> long as the two aren't specifying conflicting formats. Seems okay as
> long as it is documented well.
>
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > qemu-io.c | 31 ++++++++++++++++++++++++++++++-
> > 1 file changed, 30 insertions(+), 1 deletion(-)
> >
>
> > @@ -515,7 +531,20 @@ int main(int argc, char **argv)
> > flags |= BDRV_O_RDWR;
> > }
> >
> > - if ((argc - optind) == 1) {
> > + if (imageOpts) {
> > + QemuOpts *qopts;
> > + qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
>
> Ouch. If argc == optind (possible if I type 'qemu-io --image-opts'
> without a filename), then argv[optind] == NULL, and you end up calling
> strncmp(NULL, "id=", 3) inside opts_parse().
Yeah, I should not have removed the ((argc - optind) ==1) check here - it
should be the first thing checked, and imageOpts the second.
> Also, I noticed that running 'qemu-io' without arguments puts you into a
> shell mode, where you can then open files after the fact via the
> open_f() callback function (the 'open' command) - either that function
> should that function be given --image-opts support, or use of
> --image-opts from the command line should globally affect all subsequent
> use of open_f().
That function already has a --option / -o argument that has a similar
result, but I agree that if --image-opts is given on the cli we should
probably use that exclusively.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 07/10] qemu-img: " Daniel P. Berrange
@ 2016-02-04 15:42 ` Kevin Wolf
2016-02-04 15:47 ` Daniel P. Berrange
2016-02-04 15:59 ` Eric Blake
1 sibling, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-02-04 15:42 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini,
Andreas Färber
Am 02.02.2016 um 13:57 hat Daniel P. Berrange geschrieben:
> 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>
> @@ -212,9 +222,31 @@ 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 BlockBackend *img_open_opts(const char *id,
> + QemuOpts *opts, int flags)
> +{
> + QDict *options;
> + Error *local_err = NULL;
> + char *file = NULL;
> + BlockBackend *blk;
> + file = g_strdup(qemu_opt_get(opts, "file"));
> + qemu_opt_unset(opts, "file");
Didn't we decide that we don't want to special-case "file"?
> + options = qemu_opts_to_qdict(opts, NULL);
> + blk = blk_new_open(id, file, NULL, options, flags, &local_err);
> + if (!blk) {
> + error_report("Could not open '%s': %s", file ? file : "",
> + error_get_pretty(local_err));
> + g_free(file);
> + error_free(local_err);
> + return NULL;
> + }
> + g_free(file);
> + 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;
> BlockDriverState *bs;
> @@ -251,6 +283,33 @@ fail:
> return NULL;
> }
>
> +
> +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("image", opts, flags);
> + } else {
> + blk = img_open_file("image", filename, fmt, flags, true, quiet);
> + }
> + return blk;
> +}
I think id should be passed on instead of being replaced by "image".
> @@ -1956,7 +2034,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);
So is the plan to add another option (like --target-image-opts) when
this call is converted?
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
` (9 preceding siblings ...)
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 10/10] qemu-io: " Daniel P. Berrange
@ 2016-02-04 15:44 ` Kevin Wolf
10 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2016-02-04 15:44 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini,
Andreas Färber
Am 02.02.2016 um 13:57 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
>
> 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
>
> The first 4 patches in this series are a pre-requisite for
> 3 other series
>
> - Support for TLS in NBD
> - Support for secrets for passwd auth in curl, rbd, iscsi
> (fixes a CVE issue in libvirt)
> - Support for LUKS encryption passwords
Apart from the few comments that were made (including my own that I just
sent), this looks good to me.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-04 15:42 ` Kevin Wolf
@ 2016-02-04 15:47 ` Daniel P. Berrange
2016-02-04 16:06 ` Kevin Wolf
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-04 15:47 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini,
Andreas Färber
On Thu, Feb 04, 2016 at 04:42:06PM +0100, Kevin Wolf wrote:
> Am 02.02.2016 um 13:57 hat Daniel P. Berrange geschrieben:
> > 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>
>
> > @@ -212,9 +222,31 @@ 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 BlockBackend *img_open_opts(const char *id,
> > + QemuOpts *opts, int flags)
> > +{
> > + QDict *options;
> > + Error *local_err = NULL;
> > + char *file = NULL;
> > + BlockBackend *blk;
> > + file = g_strdup(qemu_opt_get(opts, "file"));
> > + qemu_opt_unset(opts, "file");
>
> Didn't we decide that we don't want to special-case "file"?
Yes, sorry I screwed this up when I refactored it.
> > +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("image", opts, flags);
> > + } else {
> > + blk = img_open_file("image", filename, fmt, flags, true, quiet);
> > + }
> > + return blk;
> > +}
>
> I think id should be passed on instead of being replaced by "image".
ok
> > @@ -1956,7 +2034,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);
>
> So is the plan to add another option (like --target-image-opts) when
> this call is converted?
Well I was hoping --image-opts would affect both source and target,
but i guess if we ship it only affecting source, we can't extend
it to also affect target without back compat issues, so that might
force adding a --target-image-opts
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 07/10] qemu-img: " Daniel P. Berrange
2016-02-04 15:42 ` Kevin Wolf
@ 2016-02-04 15:59 ` Eric Blake
2016-02-04 16:03 ` Daniel P. Berrange
1 sibling, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-02-04 15:59 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Markus Armbruster, qemu-block,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 2030 bytes --]
On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> 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 | 164 +++++++++++++++++++++++++++++++++++++++++++++++--------
> qemu-img.texi | 6 ++
> 3 files changed, 170 insertions(+), 44 deletions(-)
>
> +static BlockBackend *img_open_opts(const char *id,
> + QemuOpts *opts, int flags)
> +{
> + QDict *options;
> + Error *local_err = NULL;
> + char *file = NULL;
> + BlockBackend *blk;
> + file = g_strdup(qemu_opt_get(opts, "file"));
> + qemu_opt_unset(opts, "file");
> + options = qemu_opts_to_qdict(opts, NULL);
> + blk = blk_new_open(id, file, NULL, options, flags, &local_err);
> + if (!blk) {
> + error_report("Could not open '%s': %s", file ? file : "",
> + error_get_pretty(local_err));
Markus' code has landed; this would be cleaner with error_reportf_err()
from commit 8277d2aa.
> @@ -2720,6 +2828,7 @@ static int img_rebase(int argc, char **argv)
> if (optind != argc - 1) {
> error_exit("Expecting one image file name");
> }
> +
> if (!unsafe && !out_baseimg) {
> error_exit("Must specify backing file (-b) or use unsafe mode (-u)");
> }
Spurious hunk?
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-04 15:59 ` Eric Blake
@ 2016-02-04 16:03 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-04 16:03 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster,
Paolo Bonzini, Andreas Färber
On Thu, Feb 04, 2016 at 08:59:56AM -0700, Eric Blake wrote:
> On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> > 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 | 164 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > qemu-img.texi | 6 ++
> > 3 files changed, 170 insertions(+), 44 deletions(-)
> >
>
> > +static BlockBackend *img_open_opts(const char *id,
> > + QemuOpts *opts, int flags)
> > +{
> > + QDict *options;
> > + Error *local_err = NULL;
> > + char *file = NULL;
> > + BlockBackend *blk;
> > + file = g_strdup(qemu_opt_get(opts, "file"));
> > + qemu_opt_unset(opts, "file");
> > + options = qemu_opts_to_qdict(opts, NULL);
> > + blk = blk_new_open(id, file, NULL, options, flags, &local_err);
> > + if (!blk) {
> > + error_report("Could not open '%s': %s", file ? file : "",
> > + error_get_pretty(local_err));
>
> Markus' code has landed; this would be cleaner with error_reportf_err()
> from commit 8277d2aa.
Ok will change.
>
> > @@ -2720,6 +2828,7 @@ static int img_rebase(int argc, char **argv)
> > if (optind != argc - 1) {
> > error_exit("Expecting one image file name");
> > }
> > +
> > if (!unsafe && !out_baseimg) {
> > error_exit("Must specify backing file (-b) or use unsafe mode (-u)");
> > }
>
> Spurious hunk?
Yes, left over from earlier versions
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-04 15:47 ` Daniel P. Berrange
@ 2016-02-04 16:06 ` Kevin Wolf
2016-02-04 16:35 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-02-04 16:06 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini,
Andreas Färber
Am 04.02.2016 um 16:47 hat Daniel P. Berrange geschrieben:
> On Thu, Feb 04, 2016 at 04:42:06PM +0100, Kevin Wolf wrote:
> > Am 02.02.2016 um 13:57 hat Daniel P. Berrange geschrieben:
> > > @@ -1956,7 +2034,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);
> >
> > So is the plan to add another option (like --target-image-opts) when
> > this call is converted?
>
> Well I was hoping --image-opts would affect both source and target,
> but i guess if we ship it only affecting source, we can't extend
> it to also affect target without back compat issues, so that might
> force adding a --target-image-opts
Yes, that's exactly why I'm asking. We need to decide now whether this
would be an acceptable outcome or whether we shouldn't have --image-opts
in this command for now at all.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-04 16:06 ` Kevin Wolf
@ 2016-02-04 16:35 ` Daniel P. Berrange
2016-02-05 15:52 ` Kevin Wolf
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-04 16:35 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini,
Andreas Färber
On Thu, Feb 04, 2016 at 05:06:06PM +0100, Kevin Wolf wrote:
> Am 04.02.2016 um 16:47 hat Daniel P. Berrange geschrieben:
> > On Thu, Feb 04, 2016 at 04:42:06PM +0100, Kevin Wolf wrote:
> > > Am 02.02.2016 um 13:57 hat Daniel P. Berrange geschrieben:
> > > > @@ -1956,7 +2034,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);
> > >
> > > So is the plan to add another option (like --target-image-opts) when
> > > this call is converted?
> >
> > Well I was hoping --image-opts would affect both source and target,
> > but i guess if we ship it only affecting source, we can't extend
> > it to also affect target without back compat issues, so that might
> > force adding a --target-image-opts
>
> Yes, that's exactly why I'm asking. We need to decide now whether this
> would be an acceptable outcome or whether we shouldn't have --image-opts
> in this command for now at all.
I thinking having '--target-image-opts' wouldn't be the end of the world.
I also get the feeling that solving the target problem may well require
new cli args regardless, since there's multiple sets of options we care
about - the image creation options are separate from the image runtime
options in the block layer, and so given the image creation options theres
no obvious way to extrapolate the subset which are also valid runtime
options. Also AFAICT, we can't actually specify options at all for the
layer below - it just assumes a plain file backend, so there's no way
to pass options to express creation of qcow2-on-nbd for example, or
formatting of luks-on-rbd, etc - only qcow2-on-file or luks-on-file.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/10] qemu-img: allow specifying image as a set of options args
2016-02-04 16:35 ` Daniel P. Berrange
@ 2016-02-05 15:52 ` Kevin Wolf
0 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2016-02-05 15:52 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-block, qemu-devel, Markus Armbruster, Paolo Bonzini,
Andreas Färber
Am 04.02.2016 um 17:35 hat Daniel P. Berrange geschrieben:
> On Thu, Feb 04, 2016 at 05:06:06PM +0100, Kevin Wolf wrote:
> > Am 04.02.2016 um 16:47 hat Daniel P. Berrange geschrieben:
> > > On Thu, Feb 04, 2016 at 04:42:06PM +0100, Kevin Wolf wrote:
> > > > Am 02.02.2016 um 13:57 hat Daniel P. Berrange geschrieben:
> > > > > @@ -1956,7 +2034,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);
> > > >
> > > > So is the plan to add another option (like --target-image-opts) when
> > > > this call is converted?
> > >
> > > Well I was hoping --image-opts would affect both source and target,
> > > but i guess if we ship it only affecting source, we can't extend
> > > it to also affect target without back compat issues, so that might
> > > force adding a --target-image-opts
> >
> > Yes, that's exactly why I'm asking. We need to decide now whether this
> > would be an acceptable outcome or whether we shouldn't have --image-opts
> > in this command for now at all.
>
> I thinking having '--target-image-opts' wouldn't be the end of the world.
> I also get the feeling that solving the target problem may well require
> new cli args regardless, since there's multiple sets of options we care
> about - the image creation options are separate from the image runtime
> options in the block layer, and so given the image creation options theres
> no obvious way to extrapolate the subset which are also valid runtime
> options. Also AFAICT, we can't actually specify options at all for the
> layer below - it just assumes a plain file backend, so there's no way
> to pass options to express creation of qcow2-on-nbd for example, or
> formatting of luks-on-rbd, etc - only qcow2-on-file or luks-on-file.
Eventually I'd like to restructure bdrv_create() option handling similar
to bdrv_open() so that you can pass options explicitly to the protocol
layer, but for the time being, that's right, of course.
If having the options for the source is useful even without having them
for the target, I'm okay with having to add a new option later, if
nobody else objects to it.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2016-02-05 15:52 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 12:57 [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 01/10] qom: add helpers for UserCreatable object types Daniel P. Berrange
2016-02-02 14:47 ` Andreas Färber
2016-02-02 23:38 ` Eric Blake
2016-02-02 23:41 ` Andreas Färber
2016-02-03 0:15 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg Daniel P. Berrange
2016-02-03 0:24 ` Eric Blake
2016-02-03 10:09 ` Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 03/10] qemu-nbd: " Daniel P. Berrange
2016-02-03 2:33 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 04/10] qemu-io: " Daniel P. Berrange
2016-02-03 2:42 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
2016-02-03 15:37 ` Eric Blake
2016-02-03 17:13 ` Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 06/10] qemu-nbd: " Daniel P. Berrange
2016-02-03 15:47 ` Eric Blake
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 07/10] qemu-img: " Daniel P. Berrange
2016-02-04 15:42 ` Kevin Wolf
2016-02-04 15:47 ` Daniel P. Berrange
2016-02-04 16:06 ` Kevin Wolf
2016-02-04 16:35 ` Daniel P. Berrange
2016-02-05 15:52 ` Kevin Wolf
2016-02-04 15:59 ` Eric Blake
2016-02-04 16:03 ` Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 08/10] qemu-nbd: don't overlap long option values with short options Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 09/10] qemu-nbd: use no_argument/required_argument constants Daniel P. Berrange
2016-02-02 12:57 ` [Qemu-devel] [PATCH v5 10/10] qemu-io: " Daniel P. Berrange
2016-02-04 15:44 ` [Qemu-devel] [PATCH v5 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.