Re: [Qemu-block] [PATCH v1 02/15] block: add ability to set a prefix for opt names

[Daniel P. Berrange]

On Mon, Jan 16, 2017 at 08:31:55PM +0100, Max Reitz wrote:
> On 03.01.2017 19:27, Daniel P. Berrange wrote:
> > When integrating the crypto support with qcow/qcow2, we don't
> > want to use the bare LUKS option names "hash-alg", "key-secret",
> > etc. We want to namespace them "luks-hash-alg", "luks-key-secret"
> > so that they don't clash with any general qcow options at a later
> > date.
> > 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >  block/crypto.c | 110 
> > +++++++++++++++++++++++++++++++++++++++++++++++++--------
> >  block/crypto.h |  42 +++++++++++-----------
> >  2 files changed, 118 insertions(+), 34 deletions(-)
> > 
> > diff --git a/block/crypto.c b/block/crypto.c
> > index d281de6..1037c70 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> 
> [...]
> 
> > +static int block_crypto_copy_value(void *opaque, const char *name,
> > +                                   const char *value, Error **errp)
> > +{
> > +    struct BlockCryptoCopyData *data = opaque;
> > +
> > +    if (g_str_has_prefix(name, data->prefix)) {
> > +        Error *local_err = NULL;
> > +        const char *newname = name + strlen(data->prefix);
> 
> strstart() would be shorter:
> 
> const char *newname;
> 
> if (strstart(name, data->prefix, &newname)) {
>     /* ... */
> }

Ah, didn't know that function existed.

> 
> > +
> > +        qemu_opt_set(data->opts, newname, value, &local_err);
> > +        if (local_err) {
> > +            error_propagate(errp, local_err);
> > +            return 1;
> 
> I'd prefer -1, because 0/1 looks more like false/true to me, which in
> turn looks like failure/success.

Yes, that makes more sense.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|