[Qemu-devel] [PATCH v3 0/4]

Multiple sub-systems in QEMU may find it useful to generate IDs for objects that a user may reference via QMP or HMP. This patch presents a standardized way to do it, so that automatic ID generation follows the same rules.

This patch enforces the following rules when generating an ID:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictability, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

The scheme for this is as follows (no spaces):

                # subsys D RR
Reserved char --|    |   | |
Subsystem String ----|   | |
Unique number (64-bit) --| |
Two-digit random number ---|

For example, a generated node-name for the block sub-system may look like this:

    #block076

The caller of id_generate() is responsible for freeing the generated node name string with g_free().

Reviewed-by John Snow <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Reviewed-by Alberto Garcia <[email protected]>

Signed-off-by Jeff Cody <[email protected]>

---
 include/qemu-common.h |  8 ++++++++
 util/id.c             | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

include/qemu-common.h +0 -0

index 0bd212b..2f74540 100644

include/qemu-common.h +5 -0

+    ID_BLOCK,
+    ID_MAX      /* last element, used as array size */
+} IdSubSystems;
+
+char *id_generate(IdSubSystems id);
 bool id_wellformed(const char *id);
 
 /* path.c */

If a node-name is not specified, automatically generate the node-name.

Generated node-names will use the "block" sub-system identifier.

Reviewed-by Eric Blake <[email protected]>

Reviewed-by John Snow <[email protected]>

Signed-off-by Jeff Cody <[email protected]>

---
 block.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

block.c +2 -4

index 1f90b47..5947704 100644
@@ -763,12 +763,15 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
                                   const char *node_name,
                                   Error **errp)
 {
+    char *gen_node_name = NULL;
+
     if (!node_name) {
-        return;
-    }
-
-    /* Check for empty string or invalid characters */

block.c +2 -2

@@ -777,18 +780,20 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
     if (blk_by_name(node_name)) {
         error_setg(errp, "node-name=%s is conflicting with a device id",
                    node_name);
-        return;
+        goto out;
     }
 
     /* takes care of avoiding duplicates node names */
     if (bdrv_find_node(node_name)) {
         error_setg(errp, "Duplicate node name");
-        return;
+        goto out;

Quoted 46 lines · Jeff Cody

Multiple sub-systems in QEMU may find it useful to generate IDs for objects that a user may reference via QMP or HMP. This patch presents a standardized way to do it, so that automatic ID generation follows the same rules.

This patch enforces the following rules when generating an ID:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictability, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

The scheme for this is as follows (no spaces):

                # subsys D RR
Reserved char --|    |   | |
Subsystem String ----|   | |
Unique number (64-bit) --| |
Two-digit random number ---|

For example, a generated node-name for the block sub-system may look like this:

    #block076

The caller of id_generate() is responsible for freeing the generated node name string with g_free().

Reviewed-by John Snow <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Reviewed-by Alberto Garcia <[email protected]>

Signed-off-by Jeff Cody <[email protected]>

---
 include/qemu-common.h |  8 ++++++++
 util/id.c             | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

include/qemu-common.h +0 -0

index 0bd212b..2f74540 100644
@@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end,
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 

include/qemu-common.h int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end, · line 251

Quoted 4 lines

include/qemu-common.h +3 -0

 /* id.c */
+
+typedef enum IdSubSystems {
+    ID_QDEV,

ID_QDEV is not used in this series. Do you intend to use it in a followup-series? Can we reasonably expect that series will be accepted?

You could sidestep these questions by making id_generate() take a string argument ;)

Quoted 8 lines

include/qemu-common.h +5 -0

+    ID_BLOCK,
+    ID_MAX      /* last element, used as array size */
+} IdSubSystems;
+
+char *id_generate(IdSubSystems id);
 bool id_wellformed(const char *id);
 
 /* path.c */

[...]

Shouldn't PATCH 3+4 be squashed into PATCH 2 to keep bisection working?

Born from the conversation on qemu-devel, this generation scheme uses the format ultimately proposed by Kevin, after list discussion.

It attempts to keep the ID strings as small as possible, while fulfilling:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictibility, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

See patch 1 for the generation scheme details.

Quoted 48 lines · Markus Armbruster

Quoted 46 lines · Jeff Cody

Multiple sub-systems in QEMU may find it useful to generate IDs for objects that a user may reference via QMP or HMP. This patch presents a standardized way to do it, so that automatic ID generation follows the same rules.

This patch enforces the following rules when generating an ID:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictability, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

The scheme for this is as follows (no spaces):

                # subsys D RR
Reserved char --|    |   | |
Subsystem String ----|   | |
Unique number (64-bit) --| |
Two-digit random number ---|

For example, a generated node-name for the block sub-system may look like this:

    #block076

The caller of id_generate() is responsible for freeing the generated node name string with g_free().

Reviewed-by John Snow <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Reviewed-by Alberto Garcia <[email protected]>

Signed-off-by Jeff Cody <[email protected]>

---
 include/qemu-common.h |  8 ++++++++
 util/id.c             | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

include/qemu-common.h +0 -0

index 0bd212b..2f74540 100644
@@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end,
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 

Quoted 8 lines

include/qemu-common.h int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end, · line 251

Quoted 4 lines

include/qemu-common.h +3 -0

 /* id.c */
+
+typedef enum IdSubSystems {
+    ID_QDEV,

ID_QDEV is not used in this series. Do you intend to use it in a followup-series? Can we reasonably expect that series will be accepted?

John Arbuckle has a patch on list that uses it. I haven't reviewed it, however - but I guess it depends ultimately on whether qdev will allow autogeneration for its IDs or not.

Quoted 3 lines

You could sidestep these questions by making id_generate() take a string argument ;)

I'd rather avoid having each system specifying a string inline in their code. It is cleaner to have the strings defined in a central location, I think (not to mention, easier to reference).

Quoted 9 lines

Quoted 8 lines

include/qemu-common.h +5 -0

+    ID_BLOCK,
+    ID_MAX      /* last element, used as array size */
+} IdSubSystems;
+
+char *id_generate(IdSubSystems id);
 bool id_wellformed(const char *id);
 
 /* path.c */

[...]

Am 13.10.2015 um 01:36 hat Jeff Cody geschrieben:

Quoted 12 lines

Born from the conversation on qemu-devel, this generation scheme uses the format ultimately proposed by Kevin, after list discussion.

It attempts to keep the ID strings as small as possible, while fulfilling:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictibility, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

Quoted 1 line

See patch 1 for the generation scheme details.

Thanks, squashed patches 2-4 and applied to the block branch.

Quoted 58 lines · Jeff Cody

Quoted 48 lines · Markus Armbruster

Quoted 46 lines · Jeff Cody

Multiple sub-systems in QEMU may find it useful to generate IDs for objects that a user may reference via QMP or HMP. This patch presents a standardized way to do it, so that automatic ID generation follows the same rules.

This patch enforces the following rules when generating an ID:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictability, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

The scheme for this is as follows (no spaces):

                # subsys D RR
Reserved char --|    |   | |
Subsystem String ----|   | |
Unique number (64-bit) --| |
Two-digit random number ---|

For example, a generated node-name for the block sub-system may look like this:

    #block076

The caller of id_generate() is responsible for freeing the generated node name string with g_free().

Reviewed-by John Snow <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Reviewed-by Alberto Garcia <[email protected]>

Signed-off-by Jeff Cody <[email protected]>

---
 include/qemu-common.h |  8 ++++++++
 util/id.c             | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

include/qemu-common.h +0 -0

index 0bd212b..2f74540 100644
@@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end,
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 

Quoted 8 lines

include/qemu-common.h int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end, · line 251

Quoted 4 lines

include/qemu-common.h +3 -0

 /* id.c */
+
+typedef enum IdSubSystems {
+    ID_QDEV,

ID_QDEV is not used in this series. Do you intend to use it in a followup-series? Can we reasonably expect that series will be accepted?

Quoted 3 lines

John Arbuckle has a patch on list that uses it. I haven't reviewed it, however - but I guess it depends ultimately on whether qdev will allow autogeneration for its IDs or not.

Then that patch should add ID_QDEV.

Quoted 4 lines

Quoted 3 lines

You could sidestep these questions by making id_generate() take a string argument ;)

Quoted 3 lines

I'd rather avoid having each system specifying a string inline in their code. It is cleaner to have the strings defined in a central location, I think (not to mention, easier to reference).

Covered by your artistic license :)

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

include/qemu-common.h +5 -0

+    ID_BLOCK,
+    ID_MAX      /* last element, used as array size */
+} IdSubSystems;
+
+char *id_generate(IdSubSystems id);
 bool id_wellformed(const char *id);
 
 /* path.c */

[...]

Quoted 58 lines · Jeff Cody

Quoted 56 lines · Markus Armbruster

Quoted 44 lines · Jeff Cody

Multiple sub-systems in QEMU may find it useful to generate IDs for objects that a user may reference via QMP or HMP. This patch presents a standardized way to do it, so that automatic ID generation follows the same rules.

This patch enforces the following rules when generating an ID:

1.) Guarantee no collisions with a user-specified ID
2.) Identify the sub-system the ID belongs to
3.) Guarantee of uniqueness
4.) Spoiling predictability, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think they can guess the next ID based on prior behavior).

The scheme for this is as follows (no spaces):

               # subsys D RR
Reserved char --|    |   | |
Subsystem String ----|   | |
Unique number (64-bit) --| |
Two-digit random number ---|

For example, a generated node-name for the block sub-system may look like this:

   #block076

The caller of id_generate() is responsible for freeing the generated node name string with g_free().

Reviewed-by John Snow <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Reviewed-by Alberto Garcia <[email protected]>

Signed-off-by Jeff Cody <[email protected]>

---
include/qemu-common.h | 8 ++++++++
util/id.c | 37 +++++++++++++++++++++++++++++++++++++

2 files changed, 45 insertions(+)

include/qemu-common.h +0 -0

index 0bd212b..2f74540 100644

include/qemu-common.h int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end, · line 251

Quoted 6 lines

include/qemu-common.h +3 -0

@@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, char **end,
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 
 /* id.c */
+
+typedef enum IdSubSystems {
+    ID_QDEV,

ID_QDEV is not used in this series. Do you intend to use it in a followup-series? Can we reasonably expect that series will be accepted?

Quoted 3 lines

John Arbuckle has a patch on list that uses it. I haven't reviewed it, however - but I guess it depends ultimately on whether qdev will allow autogeneration for its IDs or not.

Then that patch should add ID_QDEV.

Quoted 7 lines

Quoted 3 lines

You could sidestep these questions by making id_generate() take a string argument ;)

I'd rather avoid having each system specifying a string inline in their code. It is cleaner to have the strings defined in a central location, I think (not to mention, easier to reference).