Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type

[Roman Kagan]

On Fri, Nov 24, 2017 at 08:36:53PM -0600, Corey Minyard wrote:
> On 11/24/2017 09:36 AM, Roman Kagan wrote:
> > UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated
> > property type becomes helpful.
> > 
> > Signed-off-by: Roman Kagan <address@hidden>
> > ---
> >   include/hw/qdev-properties.h |  3 +++
> >   hw/core/qdev-properties.c    | 52 
> > ++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 55 insertions(+)
> > 
> > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> > index e2321f1cc1..d4da7dd1f1 100644
> > --- a/include/hw/qdev-properties.h
> > +++ b/include/hw/qdev-properties.h
> > @@ -30,6 +30,7 @@ extern const PropertyInfo qdev_prop_vlan;
> >   extern const PropertyInfo qdev_prop_pci_devfn;
> >   extern const PropertyInfo qdev_prop_blocksize;
> >   extern const PropertyInfo qdev_prop_pci_host_devaddr;
> > +extern const PropertyInfo qdev_prop_uuid;
> >   extern const PropertyInfo qdev_prop_arraylen;
> >   extern const PropertyInfo qdev_prop_link;
> > @@ -212,6 +213,8 @@ extern const PropertyInfo qdev_prop_link;
> >       DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, 
> > PCIHostDeviceAddress)
> >   #define DEFINE_PROP_MEMORY_REGION(_n, _s, _f)             \
> >       DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
> > +#define DEFINE_PROP_UUID(_n, _s, _f) \
> > +    DEFINE_PROP(_n, _s, _f, qdev_prop_uuid, QemuUUID)
> >   #define DEFINE_PROP_END_OF_LIST()               \
> >       {}
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 1dc80fcea2..49fea5a40a 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -10,6 +10,7 @@
> >   #include "net/hub.h"
> >   #include "qapi/visitor.h"
> >   #include "chardev/char.h"
> > +#include "qemu/uuid.h"
> >   void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
> >                                     Error **errp)
> > @@ -883,6 +884,57 @@ const PropertyInfo qdev_prop_pci_host_devaddr = {
> >       .set = set_pci_host_devaddr,
> >   };
> > +/* --- UUID --- */
> > +
> > +static void get_uuid(Object *obj, Visitor *v, const char *name, void 
> > *opaque,
> > +                     Error **errp)
> > +{
> > +    DeviceState *dev = DEVICE(obj);
> > +    Property *prop = opaque;
> > +    QemuUUID *uuid = qdev_get_prop_ptr(dev, prop);
> > +    char buffer[UUID_FMT_LEN + 1];
> > +    char *p = buffer;
> > +
> > +    qemu_uuid_unparse(uuid, buffer);
> > +
> > +    visit_type_str(v, name, &p, errp);
> > +}
> > +
> > +static void set_uuid(Object *obj, Visitor *v, const char *name, void 
> > *opaque,
> > +                    Error **errp)
> > +{
> > +    DeviceState *dev = DEVICE(obj);
> > +    Property *prop = opaque;
> > +    QemuUUID *uuid = qdev_get_prop_ptr(dev, prop);
> > +    Error *local_err = NULL;
> > +    char *str;
> > +
> > +    if (dev->realized) {
> > +        qdev_prop_set_after_realize(dev, name, errp);
> > +        return;
> > +    }
> > +
> > +    visit_type_str(v, name, &str, &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +
> > +    if (!strcmp(str, "auto")) {
> > +        qemu_uuid_generate(uuid);
> > +    } else if (qemu_uuid_parse(str, uuid) < 0) {
> > +        error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
> > +    }
> > +    g_free(str);
> > +}
> > +
> > +const PropertyInfo qdev_prop_uuid = {
> > +    .name  = "str",
> > +    .description = "UUID (aka GUID) or \"auto\" for random value",
> > +    .get   = get_uuid,
> > +    .set   = set_uuid,
> 
> There is a UUID value you can use as a default in  vl.c, named qemu_uuid.
> Just in case you want to set a default value here.

I'm not sure what a common non-null default for all uuid properties
would mean...

IMO most of the time it makes sense to default to "auto".  I'll have a
look if I can do it within this patch.

Thanks,
Roman.