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

[Qemu-devel] [PATCH v2 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

v1 -> v2:
 - make the property default to autogeneration if not specified
   explicitly
 - use the corresponding define for "guid" field name

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

-- 
2.14.3

[Qemu-devel] [PATCH v2 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.

The property accepts a string-formatted UUID or a special keyword "auto"
meaning a randomly generated UUID; the latter is also the default when
the property is not given a value explicitly.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
v1 -> v2:
 - make the property default to autogeneration if not specified
   explicitly

 include/hw/qdev-properties.h |  9 +++++++
 hw/core/qdev-properties.c    | 61 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index e2321f1cc1..efb22ba80c 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;
 
@@ -213,6 +214,14 @@ extern const PropertyInfo qdev_prop_link;
 #define DEFINE_PROP_MEMORY_REGION(_n, _s, _f)             \
     DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
 
+#define DEFINE_PROP_UUID(_name, _state, _field) {                  \
+        .name      = (_name),                                      \
+        .info      = &qdev_prop_uuid,                              \
+        .offset    = offsetof(_state, _field)                      \
+            + type_check(QemuUUID, typeof_field(_state, _field)),  \
+        .set_default = true,                                       \
+        }
+
 #define DEFINE_PROP_END_OF_LIST()               \
     {}
 
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 1dc80fcea2..24c17800e3 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,66 @@ 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);
+}
+
+#define UUID_VALUE_AUTO        "auto"
+
+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, UUID_VALUE_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);
+}
+
+static void set_default_uuid_auto(Object *obj, const Property *prop)
+{
+    object_property_set_str(obj, UUID_VALUE_AUTO, prop->name, &error_abort);
+}
+
+const PropertyInfo qdev_prop_uuid = {
+    .name  = "str",
+    .description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
+        "\" for random value (default)",
+    .get   = get_uuid,
+    .set   = set_uuid,
+    .set_default_value = set_default_uuid_auto,
+};
+
 /* --- support for array properties --- */
 
 /* Used as an opaque for the object properties we add for each
-- 
2.14.3

[Qemu-devel] [PATCH v2 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>
---
v1 -> v2:
 - use the corresponding define for "guid" field name

 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..ba6f47b67b 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(VMGENID_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 v2 2/2] vmgenid: use UUID property type

[Ben Warren]

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

It looks like you dropped Marc-André and my Reviewed-by lines.  Please put them back.

> On Nov 27, 2017, at 5:05 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>
> ---
> v1 -> v2:
> - use the corresponding define for "guid" field name
> 
> 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..ba6f47b67b 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(VMGENID_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
> 


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

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

[Marc-André Lureau]

On Mon, Nov 27, 2017 at 2:05 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>


> ---
> v1 -> v2:
>  - use the corresponding define for "guid" field name
>
>  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..ba6f47b67b 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(VMGENID_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 v2 1/2] qdev-properties: add UUID property type

[Marc-André Lureau]

On Mon, Nov 27, 2017 at 2:05 PM, Roman Kagan <rkagan@virtuozzo.com> wrote:
> UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated
> property type becomes helpful.
>
> The property accepts a string-formatted UUID or a special keyword "auto"
> meaning a randomly generated UUID; the latter is also the default when
> the property is not given a value explicitly.
>
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>

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


> ---
> v1 -> v2:
>  - make the property default to autogeneration if not specified
>    explicitly
>
>  include/hw/qdev-properties.h |  9 +++++++
>  hw/core/qdev-properties.c    | 61 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
>
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index e2321f1cc1..efb22ba80c 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;
>
> @@ -213,6 +214,14 @@ extern const PropertyInfo qdev_prop_link;
>  #define DEFINE_PROP_MEMORY_REGION(_n, _s, _f)             \
>      DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
>
> +#define DEFINE_PROP_UUID(_name, _state, _field) {                  \
> +        .name      = (_name),                                      \
> +        .info      = &qdev_prop_uuid,                              \
> +        .offset    = offsetof(_state, _field)                      \
> +            + type_check(QemuUUID, typeof_field(_state, _field)),  \
> +        .set_default = true,                                       \
> +        }
> +
>  #define DEFINE_PROP_END_OF_LIST()               \
>      {}
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 1dc80fcea2..24c17800e3 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,66 @@ 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);
> +}
> +
> +#define UUID_VALUE_AUTO        "auto"
> +
> +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, UUID_VALUE_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);
> +}
> +
> +static void set_default_uuid_auto(Object *obj, const Property *prop)
> +{
> +    object_property_set_str(obj, UUID_VALUE_AUTO, prop->name, &error_abort);
> +}
> +
> +const PropertyInfo qdev_prop_uuid = {
> +    .name  = "str",
> +    .description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
> +        "\" for random value (default)",
> +    .get   = get_uuid,
> +    .set   = set_uuid,
> +    .set_default_value = set_default_uuid_auto,
> +};
> +
>  /* --- 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 v2 2/2] vmgenid: use UUID property type

[Roman Kagan]

On Mon, Nov 27, 2017 at 08:38:41AM -0800, Ben Warren wrote:
> It looks like you dropped Marc-André and my Reviewed-by lines.  Please put them back.

Oops.  I thought that the amount of changes in the first patch was too
big to keep the R-b; however, the second patch was basically the same so
I should have put the R-b and save you extra work re-reviewing it.
Sorry about that; hope the maintainer who will pick the patches will
take care of R-b (unless I get more feedback that will make me respin the
set).

Roman.

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

[Michael S. Tsirkin]

On Tue, Nov 28, 2017 at 11:02:13AM +0300, Roman Kagan wrote:
> On Mon, Nov 27, 2017 at 08:38:41AM -0800, Ben Warren wrote:
> > It looks like you dropped Marc-André and my Reviewed-by lines.  Please put them back.
> 
> Oops.  I thought that the amount of changes in the first patch was too
> big to keep the R-b; however, the second patch was basically the same so
> I should have put the R-b and save you extra work re-reviewing it.
> Sorry about that; hope the maintainer who will pick the patches will
> take care of R-b (unless I get more feedback that will make me respin the
> set).
> 
> Roman.

Nope, I don't have the time to mangle patches manually, but people can
ack series, or you can reply to patch youself with some else's ack.

-- 
MST

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

[Roman Kagan]

On Mon, Nov 27, 2017 at 04:05:16PM +0300, Roman Kagan wrote:
> 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
> 
> v1 -> v2:
>  - make the property default to autogeneration if not specified
>    explicitly
>  - use the corresponding define for "guid" field name
> 
>  include/hw/qdev-properties.h |  9 +++++++
>  hw/acpi/vmgenid.c            | 30 ++++++----------------
>  hw/core/qdev-properties.c    | 61 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+), 22 deletions(-)

Ping?

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

[Michael S. Tsirkin]

On Wed, Dec 20, 2017 at 04:51:31PM +0300, Roman Kagan wrote:
> On Mon, Nov 27, 2017 at 04:05:16PM +0300, Roman Kagan wrote:
> > 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
> > 
> > v1 -> v2:
> >  - make the property default to autogeneration if not specified
> >    explicitly
> >  - use the corresponding define for "guid" field name
> > 
> >  include/hw/qdev-properties.h |  9 +++++++
> >  hw/acpi/vmgenid.c            | 30 ++++++----------------
> >  hw/core/qdev-properties.c    | 61 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 78 insertions(+), 22 deletions(-)
> 
> Ping?

It's in my queue, will be in the next pull request.