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

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

[Roman Kagan]

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

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

[Roman Kagan]

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

[Qemu-devel] [PATCH 2/2] vmgenid: use UUID property type

[Roman Kagan]

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

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

[Marc-André Lureau]

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

Re: [Qemu-devel] [PATCH 2/2] vmgenid: use UUID property type

[Marc-André Lureau]

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

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

[Corey Minyard]

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

Re: [Qemu-devel] [PATCH 2/2] vmgenid: use UUID property type

[Ben Warren]

[-- 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 --]

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

[Ben Warren]

[-- 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 --]

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

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

[Daniel P. Berrange]

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

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

[Corey Minyard]

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