[Qemu-devel] [PATCH 0/2] add UUID property type

All of lore.kernel.org
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH 0/2] add UUID property type
@ 2017-11-24 15:36 Roman Kagan
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 1/2] qdev-properties: " Roman Kagan
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 2/2] vmgenid: use " Roman Kagan
  0 siblings, 2 replies; 11+ messages in thread
From: Roman Kagan @ 2017-11-24 15:36 UTC (permalink / raw)
  To: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ben Warren
  Cc: Markus Armbruster, Marc-André Lureau, Denis V. Lunev

UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated
property type becomes helpful.

In the existing code, vmgenid can immediately profit from it.

Roman Kagan (2):
  qdev-properties: add UUID property type
  vmgenid: use UUID property type

 include/hw/qdev-properties.h |  3 +++
 hw/acpi/vmgenid.c            | 30 +++++++------------------
 hw/core/qdev-properties.c    | 52 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 22 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-24 15:36 [Qemu-devel] [PATCH 0/2] add UUID property type Roman Kagan
@ 2017-11-24 15:36 ` Roman Kagan
  2017-11-24 17:57   ` Marc-André Lureau
                     ` (2 more replies)
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 2/2] vmgenid: use " Roman Kagan
  1 sibling, 3 replies; 11+ messages in thread
From: Roman Kagan @ 2017-11-24 15:36 UTC (permalink / raw)
  To: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ben Warren
  Cc: Markus Armbruster, Marc-André Lureau, Denis V. Lunev

UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated
property type becomes helpful.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 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,
+};
+
 /* --- support for array properties --- */
 
 /* Used as an opaque for the object properties we add for each
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 1/2] qdev-properties: " Roman Kagan
@ 2017-11-24 17:57   ` Marc-André Lureau
  2017-11-25  2:36   ` Corey Minyard
  2017-11-25  6:58   ` Ben Warren
  2 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2017-11-24 17:57 UTC (permalink / raw)
  To: Roman Kagan
  Cc: QEMU, Michael S. Tsirkin, Igor Mammedov, Ben Warren,
	Markus Armbruster, Denis V. Lunev

On Fri, Nov 24, 2017 at 4:36 PM, Roman Kagan <rkagan@virtuozzo.com> wrote:
> UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated
> property type becomes helpful.
>
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  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,
> +};
> +
>  /* --- support for array properties --- */
>
>  /* Used as an opaque for the object properties we add for each
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 1/2] qdev-properties: " Roman Kagan
  2017-11-24 17:57   ` Marc-André Lureau
@ 2017-11-25  2:36   ` Corey Minyard
  2017-11-27 10:06     ` Roman Kagan
  2017-11-27 10:35     ` Daniel P. Berrange
  2017-11-25  6:58   ` Ben Warren
  2 siblings, 2 replies; 11+ messages in thread
From: Corey Minyard @ 2017-11-25  2:36 UTC (permalink / raw)
  To: Roman Kagan, qemu-devel, Michael S. Tsirkin, Igor Mammedov,
	Ben Warren
  Cc: Marc-André Lureau, Markus Armbruster, Denis V. Lunev

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 <rkagan@virtuozzo.com>
> ---
>   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.

-corey

> +};
> +
>   /* --- support for array properties --- */
>   
>   /* Used as an opaque for the object properties we add for each

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-25  2:36   ` Corey Minyard
@ 2017-11-27 10:06     ` Roman Kagan
  2017-12-07 19:48       ` Corey Minyard
  2017-11-27 10:35     ` Daniel P. Berrange
  1 sibling, 1 reply; 11+ messages in thread
From: Roman Kagan @ 2017-11-27 10:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ben Warren,
	Marc-André Lureau, Markus Armbruster, Denis V. Lunev

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 <rkagan@virtuozzo.com>
> > ---
> >   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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-27 10:06     ` Roman Kagan
@ 2017-12-07 19:48       ` Corey Minyard
  0 siblings, 0 replies; 11+ messages in thread
From: Corey Minyard @ 2017-12-07 19:48 UTC (permalink / raw)
  To: Roman Kagan, minyard, qemu-devel, Michael S. Tsirkin,
	Igor Mammedov, Ben Warren, Marc-André Lureau,
	Markus Armbruster, Denis V. Lunev

On 11/27/2017 04:06 AM, Roman Kagan wrote:
> On Fri, Nov 24, 2017 at 08:36:53PM -0600, Corey Minyard wrote:
>> On 11/24/2017 09:36 AM, Roman Kagan wrote:
snip
>>> +
>>> +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...

It depends on what the uuid is for.  If you are trying to uniquely 
identify devices to
QEMU or the OS, then it make no sense.  If you are trying to identify 
qemu or what
is running in it to outside entities, then it might make sense.  I 
didn't know the
purpose of this uuid, so I thought I would suggest.

-corey

> 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.
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-25  2:36   ` Corey Minyard
  2017-11-27 10:06     ` Roman Kagan
@ 2017-11-27 10:35     ` Daniel P. Berrange
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel P. Berrange @ 2017-11-27 10:35 UTC (permalink / raw)
  To: minyard
  Cc: Roman Kagan, qemu-devel, Michael S. Tsirkin, Igor Mammedov,
	Ben Warren, Marc-André Lureau, Markus Armbruster,
	Denis V. Lunev

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 <rkagan@virtuozzo.com>
> > ---
> >   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 don't think that would be a good idea. If this UUID property is used in
a device type and the default were used, then every instance of the device
would use the same UUID, at which point it is a non-unique ID. Much better
to ue a random UUID, unless you know there is guaranteed to only ever be
one instance of the device type.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev-properties: add UUID property type
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 1/2] qdev-properties: " Roman Kagan
  2017-11-24 17:57   ` Marc-André Lureau
  2017-11-25  2:36   ` Corey Minyard
@ 2017-11-25  6:58   ` Ben Warren
  2 siblings, 0 replies; 11+ messages in thread
From: Ben Warren @ 2017-11-25  6:58 UTC (permalink / raw)
  To: Roman Kagan
  Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov,
	Markus Armbruster, Marc-André Lureau, Denis V. Lunev

[-- Attachment #1: Type: text/plain, Size: 3642 bytes --]


> On Nov 24, 2017, at 7:36 AM, Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated
> property type becomes helpful.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Reviewed-by: Ben Warren <ben@skyportsystems.com>
> ---
> 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,
> +};
> +
> /* --- support for array properties --- */
> 
> /* Used as an opaque for the object properties we add for each
> -- 
> 2.14.3
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3866 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/2] vmgenid: use UUID property type
  2017-11-24 15:36 [Qemu-devel] [PATCH 0/2] add UUID property type Roman Kagan
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 1/2] qdev-properties: " Roman Kagan
@ 2017-11-24 15:36 ` Roman Kagan
  2017-11-24 17:59   ` Marc-André Lureau
  2017-11-25  6:50   ` Ben Warren
  1 sibling, 2 replies; 11+ messages in thread
From: Roman Kagan @ 2017-11-24 15:36 UTC (permalink / raw)
  To: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ben Warren
  Cc: Markus Armbruster, Marc-André Lureau, Denis V. Lunev

Switch vmgenid device to use the UUID property type introduced in the
previous patch for its 'guid' property.

One semantic change it introduces is that post-realize modification of
'guid' via HMP or QMP will now be rejected with an error; however,
according to docs/specs/vmgenid.txt this is actually desirable.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 hw/acpi/vmgenid.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index 105044f666..84ff015bda 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -162,21 +162,6 @@ static void vmgenid_update_guest(VmGenIdState *vms)
     }
 }
 
-static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
-{
-    VmGenIdState *vms = VMGENID(obj);
-
-    if (!strcmp(value, "auto")) {
-        qemu_uuid_generate(&vms->guid);
-    } else if (qemu_uuid_parse(value, &vms->guid) < 0) {
-        error_setg(errp, "'%s. %s': Failed to parse GUID string: %s",
-                   object_get_typename(OBJECT(vms)), VMGENID_GUID, value);
-        return;
-    }
-
-    vmgenid_update_guest(vms);
-}
-
 /* After restoring an image, we need to update the guest memory and notify
  * it of a potential change to VM Generation ID
  */
@@ -224,23 +209,24 @@ static void vmgenid_realize(DeviceState *dev, Error **errp)
     }
 
     qemu_register_reset(vmgenid_handle_reset, vms);
+
+    vmgenid_update_guest(vms);
 }
 
+static Property vmgenid_device_properties[] = {
+    DEFINE_PROP_UUID("guid", VmGenIdState, guid),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void vmgenid_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_vmgenid;
     dc->realize = vmgenid_realize;
+    dc->props = vmgenid_device_properties;
     dc->hotpluggable = false;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-
-    object_class_property_add_str(klass, VMGENID_GUID, NULL,
-                                  vmgenid_set_guid, NULL);
-    object_class_property_set_description(klass, VMGENID_GUID,
-                                    "Set Global Unique Identifier "
-                                    "(big-endian) or auto for random value",
-                                    NULL);
 }
 
 static const TypeInfo vmgenid_device_info = {
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] vmgenid: use UUID property type
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 2/2] vmgenid: use " Roman Kagan
@ 2017-11-24 17:59   ` Marc-André Lureau
  2017-11-25  6:50   ` Ben Warren
  1 sibling, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2017-11-24 17:59 UTC (permalink / raw)
  To: Roman Kagan
  Cc: QEMU, Michael S. Tsirkin, Igor Mammedov, Ben Warren,
	Markus Armbruster, Denis V. Lunev

Hi

On Fri, Nov 24, 2017 at 4:36 PM, Roman Kagan <rkagan@virtuozzo.com> wrote:
> Switch vmgenid device to use the UUID property type introduced in the
> previous patch for its 'guid' property.
>
> One semantic change it introduces is that post-realize modification of
> 'guid' via HMP or QMP will now be rejected with an error; however,
> according to docs/specs/vmgenid.txt this is actually desirable.
>
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  hw/acpi/vmgenid.c | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
> index 105044f666..84ff015bda 100644
> --- a/hw/acpi/vmgenid.c
> +++ b/hw/acpi/vmgenid.c
> @@ -162,21 +162,6 @@ static void vmgenid_update_guest(VmGenIdState *vms)
>      }
>  }
>
> -static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
> -{
> -    VmGenIdState *vms = VMGENID(obj);
> -
> -    if (!strcmp(value, "auto")) {
> -        qemu_uuid_generate(&vms->guid);
> -    } else if (qemu_uuid_parse(value, &vms->guid) < 0) {
> -        error_setg(errp, "'%s. %s': Failed to parse GUID string: %s",
> -                   object_get_typename(OBJECT(vms)), VMGENID_GUID, value);
> -        return;
> -    }
> -
> -    vmgenid_update_guest(vms);
> -}
> -
>  /* After restoring an image, we need to update the guest memory and notify
>   * it of a potential change to VM Generation ID
>   */
> @@ -224,23 +209,24 @@ static void vmgenid_realize(DeviceState *dev, Error **errp)
>      }
>
>      qemu_register_reset(vmgenid_handle_reset, vms);
> +
> +    vmgenid_update_guest(vms);
>  }
>
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_UUID("guid", VmGenIdState, guid),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void vmgenid_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->vmsd = &vmstate_vmgenid;
>      dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
>      dc->hotpluggable = false;
>      set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> -
> -    object_class_property_add_str(klass, VMGENID_GUID, NULL,
> -                                  vmgenid_set_guid, NULL);
> -    object_class_property_set_description(klass, VMGENID_GUID,
> -                                    "Set Global Unique Identifier "
> -                                    "(big-endian) or auto for random value",
> -                                    NULL);
>  }
>
>  static const TypeInfo vmgenid_device_info = {
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] vmgenid: use UUID property type
  2017-11-24 15:36 ` [Qemu-devel] [PATCH 2/2] vmgenid: use " Roman Kagan
  2017-11-24 17:59   ` Marc-André Lureau
@ 2017-11-25  6:50   ` Ben Warren
  1 sibling, 0 replies; 11+ messages in thread
From: Ben Warren @ 2017-11-25  6:50 UTC (permalink / raw)
  To: Roman Kagan
  Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov,
	Markus Armbruster, Marc-André Lureau, Denis V. Lunev

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

Thanks Roman, this seems like a nice improvement.  Fix the minor point below, and you can add my R-B.

> On Nov 24, 2017, at 7:36 AM, Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> Switch vmgenid device to use the UUID property type introduced in the
> previous patch for its 'guid' property.
> 
> One semantic change it introduces is that post-realize modification of
> 'guid' via HMP or QMP will now be rejected with an error; however,
> according to docs/specs/vmgenid.txt this is actually desirable.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Reviewed-by: Ben Warren <ben@skyportsystems.com>
> ---
> hw/acpi/vmgenid.c | 30 ++++++++----------------------
> 1 file changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
> index 105044f666..84ff015bda 100644
> --- a/hw/acpi/vmgenid.c
> +++ b/hw/acpi/vmgenid.c
> @@ -162,21 +162,6 @@ static void vmgenid_update_guest(VmGenIdState *vms)
>     }
> }
> 
> -static void vmgenid_set_guid(Object *obj, const char *value, Error **errp)
> -{
> -    VmGenIdState *vms = VMGENID(obj);
> -
> -    if (!strcmp(value, "auto")) {
> -        qemu_uuid_generate(&vms->guid);
> -    } else if (qemu_uuid_parse(value, &vms->guid) < 0) {
> -        error_setg(errp, "'%s. %s': Failed to parse GUID string: %s",
> -                   object_get_typename(OBJECT(vms)), VMGENID_GUID, value);
> -        return;
> -    }
> -
> -    vmgenid_update_guest(vms);
> -}
> -
> /* After restoring an image, we need to update the guest memory and notify
>  * it of a potential change to VM Generation ID
>  */
> @@ -224,23 +209,24 @@ static void vmgenid_realize(DeviceState *dev, Error **errp)
>     }
> 
>     qemu_register_reset(vmgenid_handle_reset, vms);
> +
> +    vmgenid_update_guest(vms);
> }
> 
> +static Property vmgenid_device_properties[] = {
> +    DEFINE_PROP_UUID("guid", VmGenIdState, guid),
Use VMGENID_GUID instead of “guid"
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void vmgenid_device_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
> 
>     dc->vmsd = &vmstate_vmgenid;
>     dc->realize = vmgenid_realize;
> +    dc->props = vmgenid_device_properties;
>     dc->hotpluggable = false;
>     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> -
> -    object_class_property_add_str(klass, VMGENID_GUID, NULL,
> -                                  vmgenid_set_guid, NULL);
> -    object_class_property_set_description(klass, VMGENID_GUID,
> -                                    "Set Global Unique Identifier "
> -                                    "(big-endian) or auto for random value",
> -                                    NULL);
> }
> 
> static const TypeInfo vmgenid_device_info = {
> -- 
> 2.14.3
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3866 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-12-07 19:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-24 15:36 [Qemu-devel] [PATCH 0/2] add UUID property type Roman Kagan
2017-11-24 15:36 ` [Qemu-devel] [PATCH 1/2] qdev-properties: " Roman Kagan
2017-11-24 17:57   ` Marc-André Lureau
2017-11-25  2:36   ` Corey Minyard
2017-11-27 10:06     ` Roman Kagan
2017-12-07 19:48       ` Corey Minyard
2017-11-27 10:35     ` Daniel P. Berrange
2017-11-25  6:58   ` Ben Warren
2017-11-24 15:36 ` [Qemu-devel] [PATCH 2/2] vmgenid: use " Roman Kagan
2017-11-24 17:59   ` Marc-André Lureau
2017-11-25  6:50   ` Ben Warren

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.