On Wed, Jan 27, 2016 at 02:57:51PM +0100, Kevin Wolf wrote:
> Am 26.01.2016 um 14:34 hat Daniel P. Berrange geschrieben:
> > 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 <berra...@redhat.com>
> > ---
> > qemu-nbd.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > qemu-nbd.texi | 6 ++++++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/qemu-nbd.c b/qemu-nbd.c
> > index ede4a54..8e5d36c 100644
> > --- a/qemu-nbd.c
> > +++ b/qemu-nbd.c
> > @@ -23,9 +23,12 @@
> > #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 "qapi/opts-visitor.h"
> > +#include "qom/object_interfaces.h"
> >
> > #include <stdarg.h>
> > #include <stdio.h>
> > @@ -44,6 +47,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 +81,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 +381,35 @@ 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 = {
> > + { }
> > + },
> > +};
> > +
> > +static int object_create(void *opaque, QemuOpts *opts, Error **errp)
> > +{
> > + Error *err = NULL;
> > + OptsVisitor *ov;
> > + QDict *pdict;
> > +
> > + ov = opts_visitor_new(opts);
> > + pdict = qemu_opts_to_qdict(opts, NULL);
> > +
> > + user_creatable_add(pdict, opts_get_visitor(ov), &err);
> > + opts_visitor_cleanup(ov);
> > + QDECREF(pdict);
> > +
> > + if (err) {
> > + error_propagate(errp, err);
> > + return -1;
> > + }
> > + return 0;
> > +}
>
> Hm... This looks very similar to the same function in qemu-img.c. And
> actually, vl.c is a bit different, but not too much either. Wouldn't it
> make sense to share this code?
>
> Also, now that I compared this to vl.c, there seem to be two differences
> compared to the tools: Support for filtering (the tools don't seem to
> need this; passing NULL as the callback could skip it) and an additional
> obj_unref(obj).
Ok, I'll push more of this into qom/object_interfaces.c to allow
more sharing.
> I don't understand much of QOM, so I don't know what the obj_unref() is
> for, but I'm wondering whether we need to do the same in the tools. Can
> you explain?
Yes, we ought to have the obj_unref too from a technical correctness
POV, even though its harmless since we need the object around forever
anyway.
> > @@ -436,6 +474,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);
>
> Like in qemu-img, I'd prefer directly using the pointer.
Yes, will fix
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 :|
