diff mbox

qdev-monitor.c: Add device id generation

Message ID 29C62C49-06A5-4F99-8062-7269A28C29A3@gmail.com
State New
Headers show

Commit Message

Programmingkid Aug. 24, 2015, 6:53 p.m. UTC
Add device ID generation to each device if an ID isn't given.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

---
This patch can be tested by adding adding usb devices using the monitor.
Start QEMU with the -usb option. Then go to the monitor and type
"device_add usb-mouse". The ID of the device will be set to a number.
Since QEMU will not allow an user to add a device with an ID set to a
number, there is no chance for ID collisions. 

 qdev-monitor.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

Comments

Eric Blake Aug. 24, 2015, 10:21 p.m. UTC | #1
On 08/24/2015 12:53 PM, Programmingkid wrote:
> Add device ID generation to each device if an ID isn't given.
> 
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> 
> ---

>          dev->id = id;
> +    } else { /* create an id for a device if none is provided */
> +        static int device_id_count;
> +
> +        /* Add one for '\0' character */
> +        char *device_id = (char *) malloc(sizeof(char) *
> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
> +        sprintf(device_id, "%d", device_id_count++);

g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
overflow...

> +        dev->id = (const char *) device_id;
> +
> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
> +            printf("Warning: Maximum number of device ID's generated!\n\a");
> +            printf("Time for you to make your own device ID's.\n");

besides, printf() is probably the wrong way to do error reporting, and
we don't use \a BEL sequences anywhere else in qemu code.

> +        }
>      }
>  
>      if (dev->id) {

This if would now be a dead check if your patch is applied.

>          object_property_add_child(qdev_get_peripheral(), dev->id,
>                                    OBJECT(dev), NULL);
> -    } else {
> -        static int anon_count;
> -        gchar *name = g_strdup_printf("device[%d]", anon_count++);
> -        object_property_add_child(qdev_get_peripheral_anon(), name,
> -                                  OBJECT(dev), NULL);
> -        g_free(name);
>      }

It looks like your goal was to move this code earlier, but you changed
enough aspects of it that I'm not sure what the right fix should be.
Markus Armbruster Aug. 25, 2015, 12:38 p.m. UTC | #2
You're proposing to revise a qdev design decision, namely the purpose of
IDs.  This has been discussed before, and IDs remained unchanged.
Perhaps it's time to revisit this issue.  Cc'ing a few more people.

Relevant prior threads:
* [PATCH] qdev: Reject duplicate and anti-social device IDs
  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
* [PATCH 6/6] qdev: Generate IDs for anonymous devices
  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
* [PATCH] qdev: Assign a default device ID when none is provided.
  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
* IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381

Probably more I can't remember anymore :)

Programmingkid <programmingkidx@gmail.com> writes:

> Add device ID generation to each device if an ID isn't given.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
> This patch can be tested by adding adding usb devices using the monitor.
> Start QEMU with the -usb option. Then go to the monitor and type
> "device_add usb-mouse". The ID of the device will be set to a number.
> Since QEMU will not allow an user to add a device with an ID set to a
> number, there is no chance for ID collisions. 

The second sentence should really be part of your commit message.
The first sentence wouldn't hurt, either.

Another useful addition would be *why* you want generated IDs.  I
believe you do because you need them for device_del.

In prior discussion, we always concluded that device_del should accept
QOM paths.  It still doesn't.

Many things in QEMU have IDs.  They all work pretty much the same:

1. The ID is set by the user.  If the user doesn't, there is none.

   Exception: a few old interfaces set well-known IDs.  If the user uses
   these interfaces, he needs to take care that his own IDs don't clash.

   Example: drive_add picks an ID based on interface type, media type,
   bus and unit number.  blockdev_add doesn't.  Instead, it requires the
   user to pick one.

2. The ID must be well-formed.

   Exception: inconsistently enforced for QOM, see last thread quoted
   above.

3. If the user may need to address the thing, either the ID must be
   mandatory, or there has to be another way to address it.

   Example: netdev-add requires ID.  Rationale: the only way to put it
   to use is referencing it from a device, and that requires an ID.

   Example: device_add doesn't require ID.  If you don't specify one,
   you can't device_del it.  Annoying trap for the unwary.  There are
   *two* other ways to address it: qdev path and QOM path.  qdev path is
   basically too botched to be usable.  QOM path should do just fine,
   but device_del doesn't accept it.  It could.

We could revise rule 1 to always generate IDs, in a way that can't clash
with the user's IDs (impossible unless rule 2 is actually observed).
Rule 3 then becomes moot.

Whatever we do, I want it done consistently.  I don't want different
rules for different kinds of IDs.
Markus Armbruster Aug. 25, 2015, 12:42 p.m. UTC | #3
My other reply is about design issues.  This one is about the actual
code.  Until we get rough consensus on the former, the latter doesn't
really matter, but here goes anyway.

Eric Blake <eblake@redhat.com> writes:

> On 08/24/2015 12:53 PM, Programmingkid wrote:
>> Add device ID generation to each device if an ID isn't given.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
>> This patch can be tested by adding adding usb devices using the monitor.
>> Start QEMU with the -usb option. Then go to the monitor and type
>> "device_add usb-mouse". The ID of the device will be set to a number.
>> Since QEMU will not allow an user to add a device with an ID set to a
>> number, there is no chance for ID collisions. 
>>
>>  qdev-monitor.c |   24 ++++++++++++++++++------
>>  1 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index f9e2d62..98267c4 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -26,6 +26,10 @@
>>  #include "qapi/qmp/qerror.h"
>>  #include "qemu/config-file.h"
>>  #include "qemu/error-report.h"
>> +#include <math.h>
>> +
>> +/* USB's max number of devices is 127. This number is 3 digits long. */
>> +#define MAX_NUM_DIGITS_FOR_USB_ID 3

This limit makes no sense to me.

>>  
>>  /*
>>   * Aliases were a bad idea from the start.  Let's keep them
>> @@ -574,17 +578,25 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>>      id = qemu_opts_id(opts);
>>      if (id) {
>>          dev->id = id;
>> +    } else { /* create an id for a device if none is provided */
>> +        static int device_id_count;
>> +
>> +        /* Add one for '\0' character */
>> +        char *device_id = (char *) malloc(sizeof(char) *
>> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
>> +        sprintf(device_id, "%d", device_id_count++);
>
> g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
> overflow...
>
>> +        dev->id = (const char *) device_id;
>> +
>> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
>> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
>> +            printf("Warning: Maximum number of device ID's generated!\n\a");
>> +            printf("Time for you to make your own device ID's.\n");
>
> besides, printf() is probably the wrong way to do error reporting, and
> we don't use \a BEL sequences anywhere else in qemu code.
>
>> +        }
>>      }

When device_id_count reaches the limit, you warn.  Next time around, you
overrun the buffer.  Not good.

Eric is right, g_strdup_printf() is easier and safer.

I'd make the count 64 bits, so wrapping becomes vanishingly unlikely.

>>  
>>      if (dev->id) {
>
> This if would now be a dead check if your patch is applied.
>
>>          object_property_add_child(qdev_get_peripheral(), dev->id,
>>                                    OBJECT(dev), NULL);
>> -    } else {
>> -        static int anon_count;
>> -        gchar *name = g_strdup_printf("device[%d]", anon_count++);
>> -        object_property_add_child(qdev_get_peripheral_anon(), name,
>> -                                  OBJECT(dev), NULL);
>> -        g_free(name);
>>      }
>
> It looks like your goal was to move this code earlier, but you changed
> enough aspects of it that I'm not sure what the right fix should be.

Drop the conditional, it's both useless and confusing after your patch.
Programmingkid Aug. 25, 2015, 2:33 p.m. UTC | #4
On Aug 24, 2015, at 6:21 PM, Eric Blake wrote:

> On 08/24/2015 12:53 PM, Programmingkid wrote:
>> Add device ID generation to each device if an ID isn't given.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
> 
>>         dev->id = id;
>> +    } else { /* create an id for a device if none is provided */
>> +        static int device_id_count;
>> +
>> +        /* Add one for '\0' character */
>> +        char *device_id = (char *) malloc(sizeof(char) *
>> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
>> +        sprintf(device_id, "%d", device_id_count++);
> 
> g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
> overflow...
I prefer to use well known functions that work well, but I guess it shouldn't be too
painful to use the g_strdup_printf() function. Do you really think there is a possible
overflow condition here?


> 
>> +        dev->id = (const char *) device_id;
>> +
>> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
>> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
>> +            printf("Warning: Maximum number of device ID's generated!\n\a");
>> +            printf("Time for you to make your own device ID's.\n");
> 
> besides, printf() is probably the wrong way to do error reporting, and
Why do you believe this? 

> we don't use \a BEL sequences anywhere else in qemu code.
Innovation has to start somewhere :)


> 
>> +        }
>>     }
>> 
>>     if (dev->id) {
> 
> This if would now be a dead check if your patch is applied.
I think you are right. It will be removed.

> 
>>         object_property_add_child(qdev_get_peripheral(), dev->id,
>>                                   OBJECT(dev), NULL);
>> -    } else {
>> -        static int anon_count;
>> -        gchar *name = g_strdup_printf("device[%d]", anon_count++);
>> -        object_property_add_child(qdev_get_peripheral_anon(), name,
>> -                                  OBJECT(dev), NULL);
>> -        g_free(name);
>>     }
> 
> It looks like your goal was to move this code earlier, but you changed
> enough aspects of it that I'm not sure what the right fix should be.

I didn't want to move the code. It just was in a condition that would never
be true, so I thought why keep it.

> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266

Thank you very much for reviewing my patch. 

> Libvirt virtualization library http://libvirt.org

You work with this project? Any chance libvirt could support Mac OS X?
Programmingkid Aug. 25, 2015, 3:15 p.m. UTC | #5
On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:

> You're proposing to revise a qdev design decision, namely the purpose of
> IDs.  This has been discussed before, and IDs remained unchanged.
> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> 
> Relevant prior threads:
> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> * [PATCH] qdev: Assign a default device ID when none is provided.
>  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> 
> Probably more I can't remember anymore :)
> 
> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> Add device ID generation to each device if an ID isn't given.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
>> This patch can be tested by adding adding usb devices using the monitor.
>> Start QEMU with the -usb option. Then go to the monitor and type
>> "device_add usb-mouse". The ID of the device will be set to a number.
>> Since QEMU will not allow an user to add a device with an ID set to a
>> number, there is no chance for ID collisions. 
> 
> The second sentence should really be part of your commit message.
> The first sentence wouldn't hurt, either.
Ok.

> Another useful addition would be *why* you want generated IDs.  I
> believe you do because you need them for device_del.
Sounds like a good idea.

> 
> In prior discussion, we always concluded that device_del should accept
> QOM paths.  It still doesn't.
> 
> Many things in QEMU have IDs.  They all work pretty much the same:
> 
> 1. The ID is set by the user.  If the user doesn't, there is none.
> 
>   Exception: a few old interfaces set well-known IDs.  If the user uses
>   these interfaces, he needs to take care that his own IDs don't clash.
> 
>   Example: drive_add picks an ID based on interface type, media type,
>   bus and unit number.  blockdev_add doesn't.  Instead, it requires the
>   user to pick one.
> 
> 2. The ID must be well-formed.
> 
>   Exception: inconsistently enforced for QOM, see last thread quoted
>   above.

This is a definite possibility. All USB devices could be given a id like this:
USB<number>. All block devices could be HD<number>. 

> 
> 3. If the user may need to address the thing, either the ID must be
>   mandatory, or there has to be another way to address it.
> 
>   Example: netdev-add requires ID.  Rationale: the only way to put it
>   to use is referencing it from a device, and that requires an ID.
> 
>   Example: device_add doesn't require ID.  If you don't specify one,
>   you can't device_del it.  Annoying trap for the unwary.  There are
>   *two* other ways to address it: qdev path and QOM path.  qdev path is
>   basically too botched to be usable.  QOM path should do just fine,
>   but device_del doesn't accept it.  It could.
After looking up what a QOM path looks like (/i440fx/slot[1.0]/bus/piix3/i8042/aux)
I'm thinking that would not be a good idea. It is an awful
lot to type. It isn't as user friendly as using something simple like USB1 or 
HD3. 

> We could revise rule 1 to always generate IDs, in a way that can't clash
> with the user's IDs (impossible unless rule 2 is actually observed).

I think if we follow the rule that only QEMU can give an ID that is only a number,
we should be fine. That seems to be the rule now. 

> Rule 3 then becomes moot.
> 
> Whatever we do, I want it done consistently.  I don't want different
> rules for different kinds of IDs.
Agreed. Maybe we should include an easy and consistent way of finding out these ID's.

Wow, didn't think this patch to fix USB device removal would have such 
consequences.
Programmingkid Aug. 25, 2015, 3:25 p.m. UTC | #6
On Aug 25, 2015, at 8:42 AM, Markus Armbruster wrote:

> My other reply is about design issues.  This one is about the actual
> code.  Until we get rough consensus on the former, the latter doesn't
> really matter, but here goes anyway.
> 
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 08/24/2015 12:53 PM, Programmingkid wrote:
>>> Add device ID generation to each device if an ID isn't given.
>>> 
>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>> 
>>> ---
>>> This patch can be tested by adding adding usb devices using the monitor.
>>> Start QEMU with the -usb option. Then go to the monitor and type
>>> "device_add usb-mouse". The ID of the device will be set to a number.
>>> Since QEMU will not allow an user to add a device with an ID set to a
>>> number, there is no chance for ID collisions. 
>>> 
>>> qdev-monitor.c |   24 ++++++++++++++++++------
>>> 1 files changed, 18 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>> index f9e2d62..98267c4 100644
>>> --- a/qdev-monitor.c
>>> +++ b/qdev-monitor.c
>>> @@ -26,6 +26,10 @@
>>> #include "qapi/qmp/qerror.h"
>>> #include "qemu/config-file.h"
>>> #include "qemu/error-report.h"
>>> +#include <math.h>
>>> +
>>> +/* USB's max number of devices is 127. This number is 3 digits long. */
>>> +#define MAX_NUM_DIGITS_FOR_USB_ID 3
> 
> This limit makes no sense to me. 

The limit is used to decide how many characters the device_id string is going to have. 
Three digits would be 0 to 999 device ID's would be supported. I can't imagine
anyone spending the time to add that many devices.

> 
>>> 
>>> /*
>>>  * Aliases were a bad idea from the start.  Let's keep them
>>> @@ -574,17 +578,25 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>>>     id = qemu_opts_id(opts);
>>>     if (id) {
>>>         dev->id = id;
>>> +    } else { /* create an id for a device if none is provided */
>>> +        static int device_id_count;
>>> +
>>> +        /* Add one for '\0' character */
>>> +        char *device_id = (char *) malloc(sizeof(char) *
>>> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
>>> +        sprintf(device_id, "%d", device_id_count++);
>> 
>> g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
>> overflow...
>>> +        dev->id = (const char *) device_id;
>>> +
>>> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
>>> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
>>> +            printf("Warning: Maximum number of device ID's generated!\n\a");
>>> +            printf("Time for you to make your own device ID's.\n");
>> 
>> besides, printf() is probably the wrong way to do error reporting, and
>> we don't use \a BEL sequences anywhere else in qemu code.
>> 
>>> +        }
>>>     }
> 
> When device_id_count reaches the limit, you warn.  Next time around, you
> overrun the buffer.  Not good.

I could change it so next time around, only the warning is displayed. 

> 
> Eric is right, g_strdup_printf() is easier and safer.

If you say so. I have never heard of it myself.

> 
> I'd make the count 64 bits, so wrapping becomes vanishingly unlikely.

That big of a number seems unreasonably big. I can see the advantage of
using such a big number. Can QEMU even handle that many devices?

> 
>>> 
>>>     if (dev->id) {
>> 
>> This if would now be a dead check if your patch is applied.
>> 
>>>         object_property_add_child(qdev_get_peripheral(), dev->id,
>>>                                   OBJECT(dev), NULL);
>>> -    } else {
>>> -        static int anon_count;
>>> -        gchar *name = g_strdup_printf("device[%d]", anon_count++);
>>> -        object_property_add_child(qdev_get_peripheral_anon(), name,
>>> -                                  OBJECT(dev), NULL);
>>> -        g_free(name);
>>>     }
>> 
>> It looks like your goal was to move this code earlier, but you changed
>> enough aspects of it that I'm not sure what the right fix should be.
> 
> Drop the conditional, it's both useless and confusing after your patch.
Ok.

I'm thinking I will wait until the other maintainers and whoever else is interested,
say how they feel on the subject of generated ID's for devices before making
a new patch.
Peter Maydell Aug. 25, 2015, 3:33 p.m. UTC | #7
On 25 August 2015 at 16:25, Programmingkid <programmingkidx@gmail.com> wrote:
> On Aug 25, 2015, at 8:42 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> On 08/24/2015 12:53 PM, Programmingkid wrote:
>>>> +/* USB's max number of devices is 127. This number is 3 digits long. */
>>>> +#define MAX_NUM_DIGITS_FOR_USB_ID 3
>>
>> This limit makes no sense to me.
>
> The limit is used to decide how many characters the device_id string is going to have.
> Three digits would be 0 to 999 device ID's would be supported. I can't imagine
> anyone spending the time to add that many devices.

Arbitrary limits are often a bad idea, especially when
they're easy to avoid, as here.

>>>> +        /* Add one for '\0' character */
>>>> +        char *device_id = (char *) malloc(sizeof(char) *
>>>> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
>>>> +        sprintf(device_id, "%d", device_id_count++);
>>>
>>> g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
>>> overflow...

>>>> +        dev->id = (const char *) device_id;
>>>> +
>>>> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
>>>> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
>>>> +            printf("Warning: Maximum number of device ID's generated!\n\a");
>>>> +            printf("Time for you to make your own device ID's.\n");
>>>
>>> besides, printf() is probably the wrong way to do error reporting, and
>>> we don't use \a BEL sequences anywhere else in qemu code.
>>>
>>>> +        }
>>>>     }
>>
>> When device_id_count reaches the limit, you warn.  Next time around, you
>> overrun the buffer.  Not good.
>
> I could change it so next time around, only the warning is displayed.
>
>>
>> Eric is right, g_strdup_printf() is easier and safer.
>
> If you say so. I have never heard of it myself.

It's a glib function. Glib has a lot of useful utility functions
for this kind of thing (and the general idea of "have an
sprintf-alike which allocates the buffer for you" has been
around long before glib came along). Note that HACKING says that
you shouldn't use 'malloc' anyway, but 'malloc and then sprintf
into the buffer' is a particular antipattern that will get picked
up on in code review.

thanks
-- PMM
Programmingkid Aug. 25, 2015, 3:50 p.m. UTC | #8
On Aug 25, 2015, at 11:33 AM, Peter Maydell wrote:

> On 25 August 2015 at 16:25, Programmingkid <programmingkidx@gmail.com> wrote:
>> On Aug 25, 2015, at 8:42 AM, Markus Armbruster wrote:
>>> Eric Blake <eblake@redhat.com> writes:
>>> 
>>>> On 08/24/2015 12:53 PM, Programmingkid wrote:
>>>>> +/* USB's max number of devices is 127. This number is 3 digits long. */
>>>>> +#define MAX_NUM_DIGITS_FOR_USB_ID 3
>>> 
>>> This limit makes no sense to me.
>> 
>> The limit is used to decide how many characters the device_id string is going to have.
>> Three digits would be 0 to 999 device ID's would be supported. I can't imagine
>> anyone spending the time to add that many devices.
> 
> Arbitrary limits are often a bad idea, especially when
> they're easy to avoid, as here.

Knowing QEMU's limits can save the user from crashes and other problems. There is
only a finite amount of memory available to QEMU. 

> 
>>>>> +        /* Add one for '\0' character */
>>>>> +        char *device_id = (char *) malloc(sizeof(char) *
>>>>> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
>>>>> +        sprintf(device_id, "%d", device_id_count++);
>>>> 
>>>> g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
>>>> overflow...
> 
>>>>> +        dev->id = (const char *) device_id;
>>>>> +
>>>>> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
>>>>> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
>>>>> +            printf("Warning: Maximum number of device ID's generated!\n\a");
>>>>> +            printf("Time for you to make your own device ID's.\n");
>>>> 
>>>> besides, printf() is probably the wrong way to do error reporting, and
>>>> we don't use \a BEL sequences anywhere else in qemu code.
>>>> 
>>>>> +        }
>>>>>    }
>>> 
>>> When device_id_count reaches the limit, you warn.  Next time around, you
>>> overrun the buffer.  Not good.
>> 
>> I could change it so next time around, only the warning is displayed.
>> 
>>> 
>>> Eric is right, g_strdup_printf() is easier and safer.
>> 
>> If you say so. I have never heard of it myself.
> 
> It's a glib function. Glib has a lot of useful utility functions
> for this kind of thing (and the general idea of "have an
> sprintf-alike which allocates the buffer for you" has been
> around long before glib came along). Note that HACKING says that
> you shouldn't use 'malloc' anyway, but 'malloc and then sprintf
> into the buffer' is a particular antipattern that will get picked
> up on in code review.

Thank you very much for this info. Once the generated device ID
issue has been hammered down, I will make a new patch that
implements g_malloc and g_strdup_printf().
Markus Armbruster Aug. 25, 2015, 6:30 p.m. UTC | #9
Programmingkid <programmingkidx@gmail.com> writes:

> On Aug 25, 2015, at 11:33 AM, Peter Maydell wrote:
>
>> On 25 August 2015 at 16:25, Programmingkid <programmingkidx@gmail.com> wrote:
>>> On Aug 25, 2015, at 8:42 AM, Markus Armbruster wrote:
>>>> Eric Blake <eblake@redhat.com> writes:
>>>> 
>>>>> On 08/24/2015 12:53 PM, Programmingkid wrote:
>>>>>> +/* USB's max number of devices is 127. This number is 3 digits long. */
>>>>>> +#define MAX_NUM_DIGITS_FOR_USB_ID 3
>>>> 
>>>> This limit makes no sense to me.
>>> 
>>> The limit is used to decide how many characters the device_id
>>> string is going to have.
>>> Three digits would be 0 to 999 device ID's would be supported. I
>>> can't imagine
>>> anyone spending the time to add that many devices.
>> 
>> Arbitrary limits are often a bad idea, especially when
>> they're easy to avoid, as here.
>
> Knowing QEMU's limits can save the user from crashes and other
> problems. There is
> only a finite amount of memory available to QEMU. 

Repeat N times:
    device_add FOO,id=hot
    device_del hot

Memory usage independent of N (unless we leak, but that would be a bug).

[...]
Programmingkid Aug. 25, 2015, 7:05 p.m. UTC | #10
On Aug 25, 2015, at 2:30 PM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Aug 25, 2015, at 11:33 AM, Peter Maydell wrote:
>> 
>>> On 25 August 2015 at 16:25, Programmingkid <programmingkidx@gmail.com> wrote:
>>>> On Aug 25, 2015, at 8:42 AM, Markus Armbruster wrote:
>>>>> Eric Blake <eblake@redhat.com> writes:
>>>>> 
>>>>>> On 08/24/2015 12:53 PM, Programmingkid wrote:
>>>>>>> +/* USB's max number of devices is 127. This number is 3 digits long. */
>>>>>>> +#define MAX_NUM_DIGITS_FOR_USB_ID 3
>>>>> 
>>>>> This limit makes no sense to me.
>>>> 
>>>> The limit is used to decide how many characters the device_id
>>>> string is going to have.
>>>> Three digits would be 0 to 999 device ID's would be supported. I
>>>> can't imagine
>>>> anyone spending the time to add that many devices.
>>> 
>>> Arbitrary limits are often a bad idea, especially when
>>> they're easy to avoid, as here.
>> 
>> Knowing QEMU's limits can save the user from crashes and other
>> problems. There is
>> only a finite amount of memory available to QEMU. 
> 
> Repeat N times:
>    device_add FOO,id=hot
>    device_del hot
> 
> Memory usage independent of N (unless we leak, but that would be a bug).
> 
> [...]

Good example.
Programmingkid Aug. 26, 2015, 2:52 p.m. UTC | #11
On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:

> You're proposing to revise a qdev design decision, namely the purpose of
> IDs.  This has been discussed before, and IDs remained unchanged.
> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> 
> Relevant prior threads:
> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> * [PATCH] qdev: Assign a default device ID when none is provided.
>  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> 

After reading all the threads, I realize why all the attempts to accept a device ID patch failed.
It is because it was assumed everyone would agree on one patch to accept. This is
very unlikely. It would take someone in a leadership position to decide which patch
should be accepted. From one of the threads above, I saw Anthony Liguori participate.
He was in the perfect position to make the choice. The person who is in his position now
is Peter Maydell. Maybe we should just ask him to look at all the candidate patches and
have him pick one to use.
Markus Armbruster Aug. 26, 2015, 4:31 p.m. UTC | #12
Did you drop cc's intentionally?  I put them right back.

Programmingkid <programmingkidx@gmail.com> writes:

> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>
>> You're proposing to revise a qdev design decision, namely the purpose of
>> IDs.  This has been discussed before, and IDs remained unchanged.
>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>> 
>> Relevant prior threads:
>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>> 
>
> After reading all the threads, I realize why all the attempts to
> accept a device ID patch failed.
> It is because it was assumed everyone would agree on one patch to
> accept. This is
> very unlikely. It would take someone in a leadership position to
> decide which patch
> should be accepted. From one of the threads above, I saw Anthony
> Liguori participate.
> He was in the perfect position to make the choice. The person who is
> in his position now
> is Peter Maydell. Maybe we should just ask him to look at all the
> candidate patches and
> have him pick one to use. 

Yes, when no consensus emerges, problems tend to go unsolved.

Before we appeal to authority to break the deadlock, we should make
another attempt at finding consensus.

I know that we've entertained the idea of automatically generated IDs
for block layer objects (that's why I cc'ed some block guys).

I definitely want to hear Andreas's and Paolo's opinion (also cc'ed), if
they have one.
Programmingkid Aug. 26, 2015, 5:16 p.m. UTC | #13
On Aug 26, 2015, at 12:31 PM, Markus Armbruster wrote:

> Did you drop cc's intentionally?  I put them right back.

Sorry I didn't think they would care so I removed them. Will keep them in the loop for now on.

> 
> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>> 
>>> You're proposing to revise a qdev design decision, namely the purpose of
>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>> 
>>> Relevant prior threads:
>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>> 
>> 
>> After reading all the threads, I realize why all the attempts to
>> accept a device ID patch failed.
>> It is because it was assumed everyone would agree on one patch to
>> accept. This is
>> very unlikely. It would take someone in a leadership position to
>> decide which patch
>> should be accepted. From one of the threads above, I saw Anthony
>> Liguori participate.
>> He was in the perfect position to make the choice. The person who is
>> in his position now
>> is Peter Maydell. Maybe we should just ask him to look at all the
>> candidate patches and
>> have him pick one to use. 
> 
> Yes, when no consensus emerges, problems tend to go unsolved.
> 
> Before we appeal to authority to break the deadlock, we should make
> another attempt at finding consensus.

The four threads you sent indicate four failures already. Having someone
in a leadership position decide this sounds logical.

> I know that we've entertained the idea of automatically generated IDs
> for block layer objects (that's why I cc'ed some block guys).
> 
> I definitely want to hear Andreas's and Paolo's opinion (also cc'ed), if
> they have one.

That is assuming they have the time and/or the interest in solving this problem. I
suppose giving them some time to respond would be reasonable. I'm thinking if 
no consensus has been reached in one weeks time (starting today), we turn to
Peter Maydell for the answer. Hopefully he will just pick which of the patches he
likes the best. Judging by how long this problem has been ongoing, someone
pick the answer is probably the best we can expect.
Jeff Cody Aug. 26, 2015, 5:25 p.m. UTC | #14
On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
> Did you drop cc's intentionally?  I put them right back.
> 
> Programmingkid <programmingkidx@gmail.com> writes:
> 
> > On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
> >
> >> You're proposing to revise a qdev design decision, namely the purpose of
> >> IDs.  This has been discussed before, and IDs remained unchanged.
> >> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> >> 
> >> Relevant prior threads:
> >> * [PATCH] qdev: Reject duplicate and anti-social device IDs
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> >> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> >> * [PATCH] qdev: Assign a default device ID when none is provided.
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> >> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> >> 
> >
> > After reading all the threads, I realize why all the attempts to
> > accept a device ID patch failed.
> > It is because it was assumed everyone would agree on one patch to
> > accept. This is
> > very unlikely. It would take someone in a leadership position to
> > decide which patch
> > should be accepted. From one of the threads above, I saw Anthony
> > Liguori participate.
> > He was in the perfect position to make the choice. The person who is
> > in his position now
> > is Peter Maydell. Maybe we should just ask him to look at all the
> > candidate patches and
> > have him pick one to use. 
> 
> Yes, when no consensus emerges, problems tend to go unsolved.
> 
> Before we appeal to authority to break the deadlock, we should make
> another attempt at finding consensus.
> 
> I know that we've entertained the idea of automatically generated IDs
> for block layer objects (that's why I cc'ed some block guys).

Yeah, I was one of the ones that proposed some auto-generated IDs for
the block layer, specifically for BlockDriverState, making use of the
node-name field that Benoit introduced a while ago.  Here is my patch
(not sure if this is the latest version, but it is sufficient for this
discussion):

http://patchwork.ozlabs.org/patch/355990/

I'm not sure about the requirements needed by device ID names, and
they may of course differ from what I was thinking for BDS entries.

Here is what I was after with my patch for node-name auto-generation:

* Identifiable as QEMU generated / reserved namespace

* Guaranteed uniqueness

* Non-predictable (don't want users trying to guess / assume
  generated node-names)

My approach was overkill in some ways (24 characters!).  But for
better or worse, what I had was:

                __qemu##00000000IAIYNXXR
                ^^^^^^^^
QEMU namespace ----|    ^^^^^^^^
                          |     ^^^^^^^^^
Increment counter, unique |         |
                                    |
Random string, to spoil prediction  |

> 
> I definitely want to hear Andreas's and Paolo's opinion (also cc'ed), if
> they have one.
Daniel P. Berrangé Aug. 26, 2015, 5:28 p.m. UTC | #15
On Tue, Aug 25, 2015 at 02:38:17PM +0200, Markus Armbruster wrote:
> You're proposing to revise a qdev design decision, namely the purpose of
> IDs.  This has been discussed before, and IDs remained unchanged.
> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> 
> Relevant prior threads:
> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>   http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>   http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> * [PATCH] qdev: Assign a default device ID when none is provided.
>   http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>   http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> 
> Probably more I can't remember anymore :)
> 
> Programmingkid <programmingkidx@gmail.com> writes:
> 
> > Add device ID generation to each device if an ID isn't given.
> >
> > Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> >
> > ---
> > This patch can be tested by adding adding usb devices using the monitor.
> > Start QEMU with the -usb option. Then go to the monitor and type
> > "device_add usb-mouse". The ID of the device will be set to a number.
> > Since QEMU will not allow an user to add a device with an ID set to a
> > number, there is no chance for ID collisions. 
> 
> The second sentence should really be part of your commit message.
> The first sentence wouldn't hurt, either.
> 
> Another useful addition would be *why* you want generated IDs.  I
> believe you do because you need them for device_del.
> 
> In prior discussion, we always concluded that device_del should accept
> QOM paths.  It still doesn't.
> 
> Many things in QEMU have IDs.  They all work pretty much the same:
> 
> 1. The ID is set by the user.  If the user doesn't, there is none.
> 
>    Exception: a few old interfaces set well-known IDs.  If the user uses
>    these interfaces, he needs to take care that his own IDs don't clash.
> 
>    Example: drive_add picks an ID based on interface type, media type,
>    bus and unit number.  blockdev_add doesn't.  Instead, it requires the
>    user to pick one.
> 
> 2. The ID must be well-formed.
> 
>    Exception: inconsistently enforced for QOM, see last thread quoted
>    above.
> 
> 3. If the user may need to address the thing, either the ID must be
>    mandatory, or there has to be another way to address it.
> 
>    Example: netdev-add requires ID.  Rationale: the only way to put it
>    to use is referencing it from a device, and that requires an ID.
> 
>    Example: device_add doesn't require ID.  If you don't specify one,
>    you can't device_del it.  Annoying trap for the unwary.  There are
>    *two* other ways to address it: qdev path and QOM path.  qdev path is
>    basically too botched to be usable.  QOM path should do just fine,
>    but device_del doesn't accept it.  It could.
> 
> We could revise rule 1 to always generate IDs, in a way that can't clash
> with the user's IDs (impossible unless rule 2 is actually observed).
> Rule 3 then becomes moot.

If QEMU auto-generates IDs, then the user still has to query QEMU to
figure out what ID was assigned. If the device was not assigned an
ID, then it surely becomes hard for the user to identify which device
they just added in order to ask what its ID is. Which is a chicken
and egg problem. Even if the user could figure out what device they
just added, why go to the extra trouble of querying QEMU to find out
the auto-generated ID, when you could just provide an ID explicitly
upfront avoiding the entire problem. IMHO auto-generating IDs is a
just road to nowhere. Ideally we would mandate user provided IDs
but we sadly can't for back-compat reasons.

Regards,
Daniel
Programmingkid Aug. 26, 2015, 5:29 p.m. UTC | #16
On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:

> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
>> Did you drop cc's intentionally?  I put them right back.
>> 
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>>> 
>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>> 
>>>> Relevant prior threads:
>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>> 
>>> 
>>> After reading all the threads, I realize why all the attempts to
>>> accept a device ID patch failed.
>>> It is because it was assumed everyone would agree on one patch to
>>> accept. This is
>>> very unlikely. It would take someone in a leadership position to
>>> decide which patch
>>> should be accepted. From one of the threads above, I saw Anthony
>>> Liguori participate.
>>> He was in the perfect position to make the choice. The person who is
>>> in his position now
>>> is Peter Maydell. Maybe we should just ask him to look at all the
>>> candidate patches and
>>> have him pick one to use. 
>> 
>> Yes, when no consensus emerges, problems tend to go unsolved.
>> 
>> Before we appeal to authority to break the deadlock, we should make
>> another attempt at finding consensus.
>> 
>> I know that we've entertained the idea of automatically generated IDs
>> for block layer objects (that's why I cc'ed some block guys).
> 
> Yeah, I was one of the ones that proposed some auto-generated IDs for
> the block layer, specifically for BlockDriverState, making use of the
> node-name field that Benoit introduced a while ago.  Here is my patch
> (not sure if this is the latest version, but it is sufficient for this
> discussion):
> 
> http://patchwork.ozlabs.org/patch/355990/
> 
> I'm not sure about the requirements needed by device ID names, and
> they may of course differ from what I was thinking for BDS entries.
> 
> Here is what I was after with my patch for node-name auto-generation:
> 
> * Identifiable as QEMU generated / reserved namespace
> 
> * Guaranteed uniqueness
> 
> * Non-predictable (don't want users trying to guess / assume
>  generated node-names)
> 
> My approach was overkill in some ways (24 characters!).  But for
> better or worse, what I had was:
> 
>                __qemu##00000000IAIYNXXR
>                ^^^^^^^^
> QEMU namespace ----|    ^^^^^^^^
>                          |     ^^^^^^^^^
> Increment counter, unique |         |
>                                    |
> Random string, to spoil prediction  |

Yikes! 24 characters long. That is a bit much to type. Thank you very much
for your effort.
Programmingkid Aug. 26, 2015, 5:46 p.m. UTC | #17
On Aug 26, 2015, at 1:28 PM, Daniel P. Berrange wrote:

> On Tue, Aug 25, 2015 at 02:38:17PM +0200, Markus Armbruster wrote:
>> You're proposing to revise a qdev design decision, namely the purpose of
>> IDs.  This has been discussed before, and IDs remained unchanged.
>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>> 
>> Relevant prior threads:
>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>> 
>> Probably more I can't remember anymore :)
>> 
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> Add device ID generation to each device if an ID isn't given.
>>> 
>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>> 
>>> ---
>>> This patch can be tested by adding adding usb devices using the monitor.
>>> Start QEMU with the -usb option. Then go to the monitor and type
>>> "device_add usb-mouse". The ID of the device will be set to a number.
>>> Since QEMU will not allow an user to add a device with an ID set to a
>>> number, there is no chance for ID collisions. 
>> 
>> The second sentence should really be part of your commit message.
>> The first sentence wouldn't hurt, either.
>> 
>> Another useful addition would be *why* you want generated IDs.  I
>> believe you do because you need them for device_del.
>> 
>> In prior discussion, we always concluded that device_del should accept
>> QOM paths.  It still doesn't.
>> 
>> Many things in QEMU have IDs.  They all work pretty much the same:
>> 
>> 1. The ID is set by the user.  If the user doesn't, there is none.
>> 
>>   Exception: a few old interfaces set well-known IDs.  If the user uses
>>   these interfaces, he needs to take care that his own IDs don't clash.
>> 
>>   Example: drive_add picks an ID based on interface type, media type,
>>   bus and unit number.  blockdev_add doesn't.  Instead, it requires the
>>   user to pick one.
>> 
>> 2. The ID must be well-formed.
>> 
>>   Exception: inconsistently enforced for QOM, see last thread quoted
>>   above.
>> 
>> 3. If the user may need to address the thing, either the ID must be
>>   mandatory, or there has to be another way to address it.
>> 
>>   Example: netdev-add requires ID.  Rationale: the only way to put it
>>   to use is referencing it from a device, and that requires an ID.
>> 
>>   Example: device_add doesn't require ID.  If you don't specify one,
>>   you can't device_del it.  Annoying trap for the unwary.  There are
>>   *two* other ways to address it: qdev path and QOM path.  qdev path is
>>   basically too botched to be usable.  QOM path should do just fine,
>>   but device_del doesn't accept it.  It could.
>> 
>> We could revise rule 1 to always generate IDs, in a way that can't clash
>> with the user's IDs (impossible unless rule 2 is actually observed).
>> Rule 3 then becomes moot.
> 
> If QEMU auto-generates IDs, then the user still has to query QEMU to
> figure out what ID was assigned.

Querying can be easy to do. Typing "info usb" in the monitor and 
seeing the ID seems easy enough. The user can use the "device_del <id>" to
remove the device. I made a patch for "info usb" to print the ID of each
usb device.

> If the device was not assigned an
> ID, then it surely becomes hard for the user to identify which device
> they just added in order to ask what its ID is. Which is a chicken
> and egg problem. Even if the user could figure out what device they
> just added, why go to the extra trouble of querying QEMU to find out
> the auto-generated ID, when you could just provide an ID explicitly
> upfront avoiding the entire problem. IMHO auto-generating IDs is a
> just road to nowhere. Ideally we would mandate user provided IDs
> but we sadly can't for back-compat reasons.

Auto-generated ID's can be a good thing. If the user adds a usb device
to QEMU, but forgot to give it an ID, QEMU can be nice enough to do it for
that user. This feature would make the monitor command device_del
actually useful in this situation. Right now if the user forgets to give a 
device an ID, that user is out of luck when it comes time to removing
the device. This device id generation feature makes QEMU more
robust.
Daniel P. Berrangé Aug. 26, 2015, 5:53 p.m. UTC | #18
On Wed, Aug 26, 2015 at 01:46:43PM -0400, Programmingkid wrote:
> 
> On Aug 26, 2015, at 1:28 PM, Daniel P. Berrange wrote:
> 
> > On Tue, Aug 25, 2015 at 02:38:17PM +0200, Markus Armbruster wrote:
> >> You're proposing to revise a qdev design decision, namely the purpose of
> >> IDs.  This has been discussed before, and IDs remained unchanged.
> >> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> >> 
> >> Relevant prior threads:
> >> * [PATCH] qdev: Reject duplicate and anti-social device IDs
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> >> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> >> * [PATCH] qdev: Assign a default device ID when none is provided.
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> >> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
> >>  http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> >> 
> >> Probably more I can't remember anymore :)
> >> 
> >> Programmingkid <programmingkidx@gmail.com> writes:
> >> 
> >>> Add device ID generation to each device if an ID isn't given.
> >>> 
> >>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> >>> 
> >>> ---
> >>> This patch can be tested by adding adding usb devices using the monitor.
> >>> Start QEMU with the -usb option. Then go to the monitor and type
> >>> "device_add usb-mouse". The ID of the device will be set to a number.
> >>> Since QEMU will not allow an user to add a device with an ID set to a
> >>> number, there is no chance for ID collisions. 
> >> 
> >> The second sentence should really be part of your commit message.
> >> The first sentence wouldn't hurt, either.
> >> 
> >> Another useful addition would be *why* you want generated IDs.  I
> >> believe you do because you need them for device_del.
> >> 
> >> In prior discussion, we always concluded that device_del should accept
> >> QOM paths.  It still doesn't.
> >> 
> >> Many things in QEMU have IDs.  They all work pretty much the same:
> >> 
> >> 1. The ID is set by the user.  If the user doesn't, there is none.
> >> 
> >>   Exception: a few old interfaces set well-known IDs.  If the user uses
> >>   these interfaces, he needs to take care that his own IDs don't clash.
> >> 
> >>   Example: drive_add picks an ID based on interface type, media type,
> >>   bus and unit number.  blockdev_add doesn't.  Instead, it requires the
> >>   user to pick one.
> >> 
> >> 2. The ID must be well-formed.
> >> 
> >>   Exception: inconsistently enforced for QOM, see last thread quoted
> >>   above.
> >> 
> >> 3. If the user may need to address the thing, either the ID must be
> >>   mandatory, or there has to be another way to address it.
> >> 
> >>   Example: netdev-add requires ID.  Rationale: the only way to put it
> >>   to use is referencing it from a device, and that requires an ID.
> >> 
> >>   Example: device_add doesn't require ID.  If you don't specify one,
> >>   you can't device_del it.  Annoying trap for the unwary.  There are
> >>   *two* other ways to address it: qdev path and QOM path.  qdev path is
> >>   basically too botched to be usable.  QOM path should do just fine,
> >>   but device_del doesn't accept it.  It could.
> >> 
> >> We could revise rule 1 to always generate IDs, in a way that can't clash
> >> with the user's IDs (impossible unless rule 2 is actually observed).
> >> Rule 3 then becomes moot.
> > 
> > If QEMU auto-generates IDs, then the user still has to query QEMU to
> > figure out what ID was assigned.
> 
> Querying can be easy to do. Typing "info usb" in the monitor and 
> seeing the ID seems easy enough. The user can use the "device_del <id>" to
> remove the device. I made a patch for "info usb" to print the ID of each
> usb device.

That only works if you look 'info usb' after adding each device. If
you added multiple devices and then try to identify the ID after the
fact it is not guaranteed unambiguous. Using 'info usb' is also not
a general solution to the problem for other types of device.

> > If the device was not assigned an
> > ID, then it surely becomes hard for the user to identify which device
> > they just added in order to ask what its ID is. Which is a chicken
> > and egg problem. Even if the user could figure out what device they
> > just added, why go to the extra trouble of querying QEMU to find out
> > the auto-generated ID, when you could just provide an ID explicitly
> > upfront avoiding the entire problem. IMHO auto-generating IDs is a
> > just road to nowhere. Ideally we would mandate user provided IDs
> > but we sadly can't for back-compat reasons.
> 
> Auto-generated ID's can be a good thing. If the user adds a usb device
> to QEMU, but forgot to give it an ID, QEMU can be nice enough to do it for
> that user. This feature would make the monitor command device_del
> actually useful in this situation. Right now if the user forgets to give a 
> device an ID, that user is out of luck when it comes time to removing
> the device. This device id generation feature makes QEMU more
> robust.

If a user is talking to the QEMU monitor directly there are plenty of ways
to go wrong, of which forgetting to provide an ID is a really minor one.
That's why it is generally left to higher level mgmt layers to talk to
QEMU and deal with all the issues in this area. IOW if users are talking
to the monitor directly, IMHO they've already lost.

Regards,
Daniel
Programmingkid Aug. 26, 2015, 6:01 p.m. UTC | #19
On Aug 26, 2015, at 1:53 PM, Daniel P. Berrange wrote:

> On Wed, Aug 26, 2015 at 01:46:43PM -0400, Programmingkid wrote:
>> 
>> On Aug 26, 2015, at 1:28 PM, Daniel P. Berrange wrote:
>> 
>>> On Tue, Aug 25, 2015 at 02:38:17PM +0200, Markus Armbruster wrote:
>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>> 
>>>> Relevant prior threads:
>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>> 
>>>> Probably more I can't remember anymore :)
>>>> 
>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>> 
>>>>> Add device ID generation to each device if an ID isn't given.
>>>>> 
>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>>> 
>>>>> ---
>>>>> This patch can be tested by adding adding usb devices using the monitor.
>>>>> Start QEMU with the -usb option. Then go to the monitor and type
>>>>> "device_add usb-mouse". The ID of the device will be set to a number.
>>>>> Since QEMU will not allow an user to add a device with an ID set to a
>>>>> number, there is no chance for ID collisions. 
>>>> 
>>>> The second sentence should really be part of your commit message.
>>>> The first sentence wouldn't hurt, either.
>>>> 
>>>> Another useful addition would be *why* you want generated IDs.  I
>>>> believe you do because you need them for device_del.
>>>> 
>>>> In prior discussion, we always concluded that device_del should accept
>>>> QOM paths.  It still doesn't.
>>>> 
>>>> Many things in QEMU have IDs.  They all work pretty much the same:
>>>> 
>>>> 1. The ID is set by the user.  If the user doesn't, there is none.
>>>> 
>>>>  Exception: a few old interfaces set well-known IDs.  If the user uses
>>>>  these interfaces, he needs to take care that his own IDs don't clash.
>>>> 
>>>>  Example: drive_add picks an ID based on interface type, media type,
>>>>  bus and unit number.  blockdev_add doesn't.  Instead, it requires the
>>>>  user to pick one.
>>>> 
>>>> 2. The ID must be well-formed.
>>>> 
>>>>  Exception: inconsistently enforced for QOM, see last thread quoted
>>>>  above.
>>>> 
>>>> 3. If the user may need to address the thing, either the ID must be
>>>>  mandatory, or there has to be another way to address it.
>>>> 
>>>>  Example: netdev-add requires ID.  Rationale: the only way to put it
>>>>  to use is referencing it from a device, and that requires an ID.
>>>> 
>>>>  Example: device_add doesn't require ID.  If you don't specify one,
>>>>  you can't device_del it.  Annoying trap for the unwary.  There are
>>>>  *two* other ways to address it: qdev path and QOM path.  qdev path is
>>>>  basically too botched to be usable.  QOM path should do just fine,
>>>>  but device_del doesn't accept it.  It could.
>>>> 
>>>> We could revise rule 1 to always generate IDs, in a way that can't clash
>>>> with the user's IDs (impossible unless rule 2 is actually observed).
>>>> Rule 3 then becomes moot.
>>> 
>>> If QEMU auto-generates IDs, then the user still has to query QEMU to
>>> figure out what ID was assigned.
>> 
>> Querying can be easy to do. Typing "info usb" in the monitor and 
>> seeing the ID seems easy enough. The user can use the "device_del <id>" to
>> remove the device. I made a patch for "info usb" to print the ID of each
>> usb device.
> 
> That only works if you look 'info usb' after adding each device. If
> you added multiple devices and then try to identify the ID after the
> fact it is not guaranteed unambiguous. Using 'info usb' is also not
> a general solution to the problem for other types of device.
> 
>>> If the device was not assigned an
>>> ID, then it surely becomes hard for the user to identify which device
>>> they just added in order to ask what its ID is. Which is a chicken
>>> and egg problem. Even if the user could figure out what device they
>>> just added, why go to the extra trouble of querying QEMU to find out
>>> the auto-generated ID, when you could just provide an ID explicitly
>>> upfront avoiding the entire problem. IMHO auto-generating IDs is a
>>> just road to nowhere. Ideally we would mandate user provided IDs
>>> but we sadly can't for back-compat reasons.
>> 
>> Auto-generated ID's can be a good thing. If the user adds a usb device
>> to QEMU, but forgot to give it an ID, QEMU can be nice enough to do it for
>> that user. This feature would make the monitor command device_del
>> actually useful in this situation. Right now if the user forgets to give a 
>> device an ID, that user is out of luck when it comes time to removing
>> the device. This device id generation feature makes QEMU more
>> robust.
> 
> If a user is talking to the QEMU monitor directly there are plenty of ways
> to go wrong, of which forgetting to provide an ID is a really minor one.

What other problems did you have in mind?

> That's why it is generally left to higher level mgmt layers to talk to
> QEMU and deal with all the issues in this area. IOW if users are talking
> to the monitor directly, IMHO they've already lost.

I'm not following you. What do you mean by higher level mgmt layers?

Let me put it this way, if a user were to add a usb device to QEMU, say
a usb-mouse, but forgot to give it an ID. How do you expect that user to
remove the device from QEMU?
Jeff Cody Aug. 26, 2015, 6:08 p.m. UTC | #20
On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
> 
> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
> 
> > On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
> >> Did you drop cc's intentionally?  I put them right back.
> >> 
> >> Programmingkid <programmingkidx@gmail.com> writes:
> >> 
> >>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
> >>> 
> >>>> You're proposing to revise a qdev design decision, namely the purpose of
> >>>> IDs.  This has been discussed before, and IDs remained unchanged.
> >>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> >>>> 
> >>>> Relevant prior threads:
> >>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
> >>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> >>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
> >>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> >>>> * [PATCH] qdev: Assign a default device ID when none is provided.
> >>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> >>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
> >>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> >>>> 
> >>> 
> >>> After reading all the threads, I realize why all the attempts to
> >>> accept a device ID patch failed.
> >>> It is because it was assumed everyone would agree on one patch to
> >>> accept. This is
> >>> very unlikely. It would take someone in a leadership position to
> >>> decide which patch
> >>> should be accepted. From one of the threads above, I saw Anthony
> >>> Liguori participate.
> >>> He was in the perfect position to make the choice. The person who is
> >>> in his position now
> >>> is Peter Maydell. Maybe we should just ask him to look at all the
> >>> candidate patches and
> >>> have him pick one to use. 
> >> 
> >> Yes, when no consensus emerges, problems tend to go unsolved.
> >> 
> >> Before we appeal to authority to break the deadlock, we should make
> >> another attempt at finding consensus.
> >> 
> >> I know that we've entertained the idea of automatically generated IDs
> >> for block layer objects (that's why I cc'ed some block guys).
> > 
> > Yeah, I was one of the ones that proposed some auto-generated IDs for
> > the block layer, specifically for BlockDriverState, making use of the
> > node-name field that Benoit introduced a while ago.  Here is my patch
> > (not sure if this is the latest version, but it is sufficient for this
> > discussion):
> > 
> > http://patchwork.ozlabs.org/patch/355990/
> > 
> > I'm not sure about the requirements needed by device ID names, and
> > they may of course differ from what I was thinking for BDS entries.
> > 
> > Here is what I was after with my patch for node-name auto-generation:
> > 
> > * Identifiable as QEMU generated / reserved namespace
> > 
> > * Guaranteed uniqueness
> > 
> > * Non-predictable (don't want users trying to guess / assume
> >  generated node-names)
> > 
> > My approach was overkill in some ways (24 characters!).  But for
> > better or worse, what I had was:
> > 
> >                __qemu##00000000IAIYNXXR
> >                ^^^^^^^^
> > QEMU namespace ----|    ^^^^^^^^
> >                          |     ^^^^^^^^^
> > Increment counter, unique |         |
> >                                    |
> > Random string, to spoil prediction  |
> 
> Yikes! 24 characters long. That is a bit much to type. Thank you very much
> for your effort.

IMO, the number of characters to type is pretty low on the list of
requirements, although it can still be addressed secondary to other
concerns.

I should have made this in reply to Markus' other email, because the
important part of this is try and address his point #2:

(from Markus' other email):
> 2. The ID must be well-formed.

To have a well-formed ID, we need to have know requirements of the ID
structure (i.e. the why and what of it all)

I don't know if the three requirements I had above apply to all areas
in QEMU, but I expect they do, in varying degrees of importance.  The
length itself can be tweaked.

Talking with John Snow over IRC (added to the CC), one thing he
suggested was adding in sub-domain spaces; e.g.:

__qemu#bn#00000000#IAIYNXXR

Where the 'bn' in this case would be for Block Nodes, etc..

This may make the scheme extensible through QEMU, where auto-generated
IDs are desired.

(sorry to say, this lengthens things, rather than shortening them!)

We can, of course, make the string shorter - if the random characters
are just there for spoiling predictability, then 2-3 should be
sufficient. We could then end up with something like this:

__qemu#bn#00000000#XR

The "__qemu" part of the namespace could be shortened as well, but it
would be nice if it was easy recognizable as being from QEMU.


Jeff
Programmingkid Aug. 26, 2015, 6:17 p.m. UTC | #21
On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:

> On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
>> 
>> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
>> 
>>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
>>>> Did you drop cc's intentionally?  I put them right back.
>>>> 
>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>> 
>>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>>>>> 
>>>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>>>> 
>>>>>> Relevant prior threads:
>>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>>>> 
>>>>> 
>>>>> After reading all the threads, I realize why all the attempts to
>>>>> accept a device ID patch failed.
>>>>> It is because it was assumed everyone would agree on one patch to
>>>>> accept. This is
>>>>> very unlikely. It would take someone in a leadership position to
>>>>> decide which patch
>>>>> should be accepted. From one of the threads above, I saw Anthony
>>>>> Liguori participate.
>>>>> He was in the perfect position to make the choice. The person who is
>>>>> in his position now
>>>>> is Peter Maydell. Maybe we should just ask him to look at all the
>>>>> candidate patches and
>>>>> have him pick one to use. 
>>>> 
>>>> Yes, when no consensus emerges, problems tend to go unsolved.
>>>> 
>>>> Before we appeal to authority to break the deadlock, we should make
>>>> another attempt at finding consensus.
>>>> 
>>>> I know that we've entertained the idea of automatically generated IDs
>>>> for block layer objects (that's why I cc'ed some block guys).
>>> 
>>> Yeah, I was one of the ones that proposed some auto-generated IDs for
>>> the block layer, specifically for BlockDriverState, making use of the
>>> node-name field that Benoit introduced a while ago.  Here is my patch
>>> (not sure if this is the latest version, but it is sufficient for this
>>> discussion):
>>> 
>>> http://patchwork.ozlabs.org/patch/355990/
>>> 
>>> I'm not sure about the requirements needed by device ID names, and
>>> they may of course differ from what I was thinking for BDS entries.
>>> 
>>> Here is what I was after with my patch for node-name auto-generation:
>>> 
>>> * Identifiable as QEMU generated / reserved namespace
>>> 
>>> * Guaranteed uniqueness
>>> 
>>> * Non-predictable (don't want users trying to guess / assume
>>> generated node-names)
>>> 
>>> My approach was overkill in some ways (24 characters!).  But for
>>> better or worse, what I had was:
>>> 
>>>               __qemu##00000000IAIYNXXR
>>>               ^^^^^^^^
>>> QEMU namespace ----|    ^^^^^^^^
>>>                         |     ^^^^^^^^^
>>> Increment counter, unique |         |
>>>                                   |
>>> Random string, to spoil prediction  |
>> 
>> Yikes! 24 characters long. That is a bit much to type. Thank you very much
>> for your effort.
> 
> IMO, the number of characters to type is pretty low on the list of
> requirements, although it can still be addressed secondary to other
> concerns.
> 
> I should have made this in reply to Markus' other email, because the
> important part of this is try and address his point #2:
> 
> (from Markus' other email):
>> 2. The ID must be well-formed.
> 
> To have a well-formed ID, we need to have know requirements of the ID
> structure (i.e. the why and what of it all)
> 
> I don't know if the three requirements I had above apply to all areas
> in QEMU, but I expect they do, in varying degrees of importance.  The
> length itself can be tweaked.
> 
> Talking with John Snow over IRC (added to the CC), one thing he
> suggested was adding in sub-domain spaces; e.g.:
> 
> __qemu#bn#00000000#IAIYNXXR
> 
> Where the 'bn' in this case would be for Block Nodes, etc..
> 
> This may make the scheme extensible through QEMU, where auto-generated
> IDs are desired.
> 
> (sorry to say, this lengthens things, rather than shortening them!)
> 
> We can, of course, make the string shorter - if the random characters
> are just there for spoiling predictability, then 2-3 should be
> sufficient. We could then end up with something like this:
> 
> __qemu#bn#00000000#XR
> 
> The "__qemu" part of the namespace could be shortened as well, but it
> would be nice if it was easy recognizable as being from QEMU.

If this ID format was supported, I'm thinking being able to copy and paste from
the monitor is a necessary feature. 

Any way it could be shorted? I was hoping no more than three characters long. 

If this were the format of the ID, maybe we could put the value in a table that
would translate this long ID to a shorter version. Or maybe a mathematical function
could be applied to the value to give it some user-friendly value.

I do think your idea virtually eliminates the problem of ID collisions.
Peter Maydell Aug. 26, 2015, 6:45 p.m. UTC | #22
On 26 August 2015 at 18:16, Programmingkid <programmingkidx@gmail.com> wrote:
> That is assuming they have the time and/or the interest in solving this problem. I
> suppose giving them some time to respond would be reasonable. I'm thinking if
> no consensus has been reached in one weeks time (starting today), we turn to
> Peter Maydell for the answer. Hopefully he will just pick which of the patches he
> likes the best. Judging by how long this problem has been ongoing, someone
> pick the answer is probably the best we can expect.

This is the kind of thing I strongly prefer to leave to the
relevant subsystem maintainer(s). My opinion is not worth
a great deal since I don't have a strong familiarity with
this bit of QEMU.

-- PMM
Programmingkid Aug. 26, 2015, 9:48 p.m. UTC | #23
On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote:

> On 26 August 2015 at 18:16, Programmingkid <programmingkidx@gmail.com> wrote:
>> That is assuming they have the time and/or the interest in solving this problem. I
>> suppose giving them some time to respond would be reasonable. I'm thinking if
>> no consensus has been reached in one weeks time (starting today), we turn to
>> Peter Maydell for the answer. Hopefully he will just pick which of the patches he
>> likes the best. Judging by how long this problem has been ongoing, someone
>> pick the answer is probably the best we can expect.
> 
> This is the kind of thing I strongly prefer to leave to the
> relevant subsystem maintainer(s). My opinion is not worth
> a great deal since I don't have a strong familiarity with
> this bit of QEMU.

It looks unreasonable to assume any consensus can be reached over this issue.
The easy thing to do is to just let each maintainer deal with this problem
his own way. 

Markus:
I know you really wanted a single ID generating system, but it just isn't going
to happen. I will make a patch that would only effect USB devices. All other
devices would be untouched. At least the device_del problem will be solved.
Jeff Cody Aug. 26, 2015, 10:01 p.m. UTC | #24
On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
> 
> On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
> 
> > On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
> >> 
> >> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
> >> 
> >>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
> >>>> Did you drop cc's intentionally?  I put them right back.
> >>>> 
> >>>> Programmingkid <programmingkidx@gmail.com> writes:
> >>>> 
> >>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
> >>>>> 
> >>>>>> You're proposing to revise a qdev design decision, namely the purpose of
> >>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
> >>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> >>>>>> 
> >>>>>> Relevant prior threads:
> >>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
> >>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> >>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
> >>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> >>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
> >>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> >>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
> >>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> >>>>>> 
> >>>>> 
> >>>>> After reading all the threads, I realize why all the attempts to
> >>>>> accept a device ID patch failed.
> >>>>> It is because it was assumed everyone would agree on one patch to
> >>>>> accept. This is
> >>>>> very unlikely. It would take someone in a leadership position to
> >>>>> decide which patch
> >>>>> should be accepted. From one of the threads above, I saw Anthony
> >>>>> Liguori participate.
> >>>>> He was in the perfect position to make the choice. The person who is
> >>>>> in his position now
> >>>>> is Peter Maydell. Maybe we should just ask him to look at all the
> >>>>> candidate patches and
> >>>>> have him pick one to use. 
> >>>> 
> >>>> Yes, when no consensus emerges, problems tend to go unsolved.
> >>>> 
> >>>> Before we appeal to authority to break the deadlock, we should make
> >>>> another attempt at finding consensus.
> >>>> 
> >>>> I know that we've entertained the idea of automatically generated IDs
> >>>> for block layer objects (that's why I cc'ed some block guys).
> >>> 
> >>> Yeah, I was one of the ones that proposed some auto-generated IDs for
> >>> the block layer, specifically for BlockDriverState, making use of the
> >>> node-name field that Benoit introduced a while ago.  Here is my patch
> >>> (not sure if this is the latest version, but it is sufficient for this
> >>> discussion):
> >>> 
> >>> http://patchwork.ozlabs.org/patch/355990/
> >>> 
> >>> I'm not sure about the requirements needed by device ID names, and
> >>> they may of course differ from what I was thinking for BDS entries.
> >>> 
> >>> Here is what I was after with my patch for node-name auto-generation:
> >>> 
> >>> * Identifiable as QEMU generated / reserved namespace
> >>> 
> >>> * Guaranteed uniqueness
> >>> 
> >>> * Non-predictable (don't want users trying to guess / assume
> >>> generated node-names)
> >>> 
> >>> My approach was overkill in some ways (24 characters!).  But for
> >>> better or worse, what I had was:
> >>> 
> >>>               __qemu##00000000IAIYNXXR
> >>>               ^^^^^^^^
> >>> QEMU namespace ----|    ^^^^^^^^
> >>>                         |     ^^^^^^^^^
> >>> Increment counter, unique |         |
> >>>                                   |
> >>> Random string, to spoil prediction  |
> >> 
> >> Yikes! 24 characters long. That is a bit much to type. Thank you very much
> >> for your effort.
> > 
> > IMO, the number of characters to type is pretty low on the list of
> > requirements, although it can still be addressed secondary to other
> > concerns.
> > 
> > I should have made this in reply to Markus' other email, because the
> > important part of this is try and address his point #2:
> > 
> > (from Markus' other email):
> >> 2. The ID must be well-formed.
> > 
> > To have a well-formed ID, we need to have know requirements of the ID
> > structure (i.e. the why and what of it all)
> > 
> > I don't know if the three requirements I had above apply to all areas
> > in QEMU, but I expect they do, in varying degrees of importance.  The
> > length itself can be tweaked.
> > 
> > Talking with John Snow over IRC (added to the CC), one thing he
> > suggested was adding in sub-domain spaces; e.g.:
> > 
> > __qemu#bn#00000000#IAIYNXXR
> > 
> > Where the 'bn' in this case would be for Block Nodes, etc..
> > 
> > This may make the scheme extensible through QEMU, where auto-generated
> > IDs are desired.
> > 
> > (sorry to say, this lengthens things, rather than shortening them!)
> > 
> > We can, of course, make the string shorter - if the random characters
> > are just there for spoiling predictability, then 2-3 should be
> > sufficient. We could then end up with something like this:
> > 
> > __qemu#bn#00000000#XR
> > 
> > The "__qemu" part of the namespace could be shortened as well, but it
> > would be nice if it was easy recognizable as being from QEMU.
> 
> If this ID format was supported, I'm thinking being able to copy and paste from
> the monitor is a necessary feature. 
> 
> Any way it could be shorted? I was hoping no more than three characters long. 
>

Likely could be shorter, but something in the realm of three
characters doesn't seem very realistic.

> If this were the format of the ID, maybe we could put the value in a table that
> would translate this long ID to a shorter version. Or maybe a mathematical function
> could be applied to the value to give it some user-friendly value.

I'm afraid this would discard pretty much all the benefits of the ID
generation scheme.

> 
> I do think your idea virtually eliminates the problem of ID collisions.
John Snow Aug. 26, 2015, 10:04 p.m. UTC | #25
On 08/26/2015 06:01 PM, Jeff Cody wrote:
> On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
>>
>> On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
>>
>>> On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
>>>>
>>>> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
>>>>
>>>>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
>>>>>> Did you drop cc's intentionally?  I put them right back.
>>>>>>
>>>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>>>>
>>>>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>>>>>>>
>>>>>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>>>>>>
>>>>>>>> Relevant prior threads:
>>>>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>>>>>>
>>>>>>>
>>>>>>> After reading all the threads, I realize why all the attempts to
>>>>>>> accept a device ID patch failed.
>>>>>>> It is because it was assumed everyone would agree on one patch to
>>>>>>> accept. This is
>>>>>>> very unlikely. It would take someone in a leadership position to
>>>>>>> decide which patch
>>>>>>> should be accepted. From one of the threads above, I saw Anthony
>>>>>>> Liguori participate.
>>>>>>> He was in the perfect position to make the choice. The person who is
>>>>>>> in his position now
>>>>>>> is Peter Maydell. Maybe we should just ask him to look at all the
>>>>>>> candidate patches and
>>>>>>> have him pick one to use. 
>>>>>>
>>>>>> Yes, when no consensus emerges, problems tend to go unsolved.
>>>>>>
>>>>>> Before we appeal to authority to break the deadlock, we should make
>>>>>> another attempt at finding consensus.
>>>>>>
>>>>>> I know that we've entertained the idea of automatically generated IDs
>>>>>> for block layer objects (that's why I cc'ed some block guys).
>>>>>
>>>>> Yeah, I was one of the ones that proposed some auto-generated IDs for
>>>>> the block layer, specifically for BlockDriverState, making use of the
>>>>> node-name field that Benoit introduced a while ago.  Here is my patch
>>>>> (not sure if this is the latest version, but it is sufficient for this
>>>>> discussion):
>>>>>
>>>>> http://patchwork.ozlabs.org/patch/355990/
>>>>>
>>>>> I'm not sure about the requirements needed by device ID names, and
>>>>> they may of course differ from what I was thinking for BDS entries.
>>>>>
>>>>> Here is what I was after with my patch for node-name auto-generation:
>>>>>
>>>>> * Identifiable as QEMU generated / reserved namespace
>>>>>
>>>>> * Guaranteed uniqueness
>>>>>
>>>>> * Non-predictable (don't want users trying to guess / assume
>>>>> generated node-names)
>>>>>
>>>>> My approach was overkill in some ways (24 characters!).  But for
>>>>> better or worse, what I had was:
>>>>>
>>>>>               __qemu##00000000IAIYNXXR
>>>>>               ^^^^^^^^
>>>>> QEMU namespace ----|    ^^^^^^^^
>>>>>                         |     ^^^^^^^^^
>>>>> Increment counter, unique |         |
>>>>>                                   |
>>>>> Random string, to spoil prediction  |
>>>>
>>>> Yikes! 24 characters long. That is a bit much to type. Thank you very much
>>>> for your effort.
>>>
>>> IMO, the number of characters to type is pretty low on the list of
>>> requirements, although it can still be addressed secondary to other
>>> concerns.
>>>
>>> I should have made this in reply to Markus' other email, because the
>>> important part of this is try and address his point #2:
>>>
>>> (from Markus' other email):
>>>> 2. The ID must be well-formed.
>>>
>>> To have a well-formed ID, we need to have know requirements of the ID
>>> structure (i.e. the why and what of it all)
>>>
>>> I don't know if the three requirements I had above apply to all areas
>>> in QEMU, but I expect they do, in varying degrees of importance.  The
>>> length itself can be tweaked.
>>>
>>> Talking with John Snow over IRC (added to the CC), one thing he
>>> suggested was adding in sub-domain spaces; e.g.:
>>>
>>> __qemu#bn#00000000#IAIYNXXR
>>>
>>> Where the 'bn' in this case would be for Block Nodes, etc..
>>>
>>> This may make the scheme extensible through QEMU, where auto-generated
>>> IDs are desired.
>>>
>>> (sorry to say, this lengthens things, rather than shortening them!)
>>>
>>> We can, of course, make the string shorter - if the random characters
>>> are just there for spoiling predictability, then 2-3 should be
>>> sufficient. We could then end up with something like this:
>>>
>>> __qemu#bn#00000000#XR
>>>
>>> The "__qemu" part of the namespace could be shortened as well, but it
>>> would be nice if it was easy recognizable as being from QEMU.
>>
>> If this ID format was supported, I'm thinking being able to copy and paste from
>> the monitor is a necessary feature. 
>>
>> Any way it could be shorted? I was hoping no more than three characters long. 
>>
> 
> Likely could be shorter, but something in the realm of three
> characters doesn't seem very realistic.
> 
>> If this were the format of the ID, maybe we could put the value in a table that
>> would translate this long ID to a shorter version. Or maybe a mathematical function
>> could be applied to the value to give it some user-friendly value.
> 
> I'm afraid this would discard pretty much all the benefits of the ID
> generation scheme.

At this point, why not specify a user-friendly ID yourself? If there is
some technical reason you cannot, maybe we should fix the interface to
allow you to do so.

Auto-generated IDs are not likely to be short, pretty, or easy to type
due to the constraints Jeff Cody laid out earlier.

> 
>>
>> I do think your idea virtually eliminates the problem of ID collisions.
John Snow Aug. 26, 2015, 10:08 p.m. UTC | #26
On 08/26/2015 05:48 PM, Programmingkid wrote:
> 
> On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote:
> 
>> On 26 August 2015 at 18:16, Programmingkid <programmingkidx@gmail.com> wrote:
>>> That is assuming they have the time and/or the interest in solving this problem. I
>>> suppose giving them some time to respond would be reasonable. I'm thinking if
>>> no consensus has been reached in one weeks time (starting today), we turn to
>>> Peter Maydell for the answer. Hopefully he will just pick which of the patches he
>>> likes the best. Judging by how long this problem has been ongoing, someone
>>> pick the answer is probably the best we can expect.
>>
>> This is the kind of thing I strongly prefer to leave to the
>> relevant subsystem maintainer(s). My opinion is not worth
>> a great deal since I don't have a strong familiarity with
>> this bit of QEMU.
> 
> It looks unreasonable to assume any consensus can be reached over this issue.
> The easy thing to do is to just let each maintainer deal with this problem
> his own way. 
> 

What feedback was there that seemed insurmountable? Last I talked to
Jeff Cody he said there was no "overwhelming pushback" against his
patches, just a list of concerns.

This doesn't sound like a dead end so much as it sounds like we haven't
planned the feature enough yet.

> Markus:
> I know you really wanted a single ID generating system, but it just isn't going
> to happen. I will make a patch that would only effect USB devices. All other
> devices would be untouched. At least the device_del problem will be solved.
> 

I think this is being unnecessarily hasty. We should make sure that an
auto-generated ID system does not create problems for other areas of
code before we rush ahead with one to solve a single problem.

Let's give the universal approach some more time before we jump to the
conclusion that it's impossible.

--js
Programmingkid Aug. 27, 2015, 3:22 a.m. UTC | #27
On Aug 26, 2015, at 6:01 PM, Jeff Cody wrote:

> On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
>> 
>> On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
>> 
>>> On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
>>>> 
>>>>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
>>>>>> Did you drop cc's intentionally?  I put them right back.
>>>>>> 
>>>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>>>> 
>>>>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>>>>>>> 
>>>>>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>>>>>> 
>>>>>>>> Relevant prior threads:
>>>>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>>>>>> 
>>>>>>> 
>>>>>>> After reading all the threads, I realize why all the attempts to
>>>>>>> accept a device ID patch failed.
>>>>>>> It is because it was assumed everyone would agree on one patch to
>>>>>>> accept. This is
>>>>>>> very unlikely. It would take someone in a leadership position to
>>>>>>> decide which patch
>>>>>>> should be accepted. From one of the threads above, I saw Anthony
>>>>>>> Liguori participate.
>>>>>>> He was in the perfect position to make the choice. The person who is
>>>>>>> in his position now
>>>>>>> is Peter Maydell. Maybe we should just ask him to look at all the
>>>>>>> candidate patches and
>>>>>>> have him pick one to use. 
>>>>>> 
>>>>>> Yes, when no consensus emerges, problems tend to go unsolved.
>>>>>> 
>>>>>> Before we appeal to authority to break the deadlock, we should make
>>>>>> another attempt at finding consensus.
>>>>>> 
>>>>>> I know that we've entertained the idea of automatically generated IDs
>>>>>> for block layer objects (that's why I cc'ed some block guys).
>>>>> 
>>>>> Yeah, I was one of the ones that proposed some auto-generated IDs for
>>>>> the block layer, specifically for BlockDriverState, making use of the
>>>>> node-name field that Benoit introduced a while ago.  Here is my patch
>>>>> (not sure if this is the latest version, but it is sufficient for this
>>>>> discussion):
>>>>> 
>>>>> http://patchwork.ozlabs.org/patch/355990/
>>>>> 
>>>>> I'm not sure about the requirements needed by device ID names, and
>>>>> they may of course differ from what I was thinking for BDS entries.
>>>>> 
>>>>> Here is what I was after with my patch for node-name auto-generation:
>>>>> 
>>>>> * Identifiable as QEMU generated / reserved namespace
>>>>> 
>>>>> * Guaranteed uniqueness
>>>>> 
>>>>> * Non-predictable (don't want users trying to guess / assume
>>>>> generated node-names)
>>>>> 
>>>>> My approach was overkill in some ways (24 characters!).  But for
>>>>> better or worse, what I had was:
>>>>> 
>>>>>              __qemu##00000000IAIYNXXR
>>>>>              ^^^^^^^^
>>>>> QEMU namespace ----|    ^^^^^^^^
>>>>>                        |     ^^^^^^^^^
>>>>> Increment counter, unique |         |
>>>>>                                  |
>>>>> Random string, to spoil prediction  |
>>>> 
>>>> Yikes! 24 characters long. That is a bit much to type. Thank you very much
>>>> for your effort.
>>> 
>>> IMO, the number of characters to type is pretty low on the list of
>>> requirements, although it can still be addressed secondary to other
>>> concerns.
>>> 
>>> I should have made this in reply to Markus' other email, because the
>>> important part of this is try and address his point #2:
>>> 
>>> (from Markus' other email):
>>>> 2. The ID must be well-formed.
>>> 
>>> To have a well-formed ID, we need to have know requirements of the ID
>>> structure (i.e. the why and what of it all)
>>> 
>>> I don't know if the three requirements I had above apply to all areas
>>> in QEMU, but I expect they do, in varying degrees of importance.  The
>>> length itself can be tweaked.
>>> 
>>> Talking with John Snow over IRC (added to the CC), one thing he
>>> suggested was adding in sub-domain spaces; e.g.:
>>> 
>>> __qemu#bn#00000000#IAIYNXXR
>>> 
>>> Where the 'bn' in this case would be for Block Nodes, etc..
>>> 
>>> This may make the scheme extensible through QEMU, where auto-generated
>>> IDs are desired.
>>> 
>>> (sorry to say, this lengthens things, rather than shortening them!)
>>> 
>>> We can, of course, make the string shorter - if the random characters
>>> are just there for spoiling predictability, then 2-3 should be
>>> sufficient. We could then end up with something like this:
>>> 
>>> __qemu#bn#00000000#XR
>>> 
>>> The "__qemu" part of the namespace could be shortened as well, but it
>>> would be nice if it was easy recognizable as being from QEMU.
>> 
>> If this ID format was supported, I'm thinking being able to copy and paste from
>> the monitor is a necessary feature. 
>> 
>> Any way it could be shorted? I was hoping no more than three characters long. 
>> 
> 
> Likely could be shorter, but something in the realm of three
> characters doesn't seem very realistic.

Sure it is. Just set device id's like this: 0, 1, 2, 3, 4, 5, 6....
Programmingkid Aug. 27, 2015, 3:26 a.m. UTC | #28
On Aug 26, 2015, at 6:04 PM, John Snow wrote:

> 
> 
> On 08/26/2015 06:01 PM, Jeff Cody wrote:
>> On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
>>> 
>>> On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
>>> 
>>>> On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
>>>>> 
>>>>> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
>>>>> 
>>>>>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
>>>>>>> Did you drop cc's intentionally?  I put them right back.
>>>>>>> 
>>>>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>>>>> 
>>>>>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>>>>>>>> 
>>>>>>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>>>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>>>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>>>>>>> 
>>>>>>>>> Relevant prior threads:
>>>>>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>>>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>>>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>>>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> After reading all the threads, I realize why all the attempts to
>>>>>>>> accept a device ID patch failed.
>>>>>>>> It is because it was assumed everyone would agree on one patch to
>>>>>>>> accept. This is
>>>>>>>> very unlikely. It would take someone in a leadership position to
>>>>>>>> decide which patch
>>>>>>>> should be accepted. From one of the threads above, I saw Anthony
>>>>>>>> Liguori participate.
>>>>>>>> He was in the perfect position to make the choice. The person who is
>>>>>>>> in his position now
>>>>>>>> is Peter Maydell. Maybe we should just ask him to look at all the
>>>>>>>> candidate patches and
>>>>>>>> have him pick one to use. 
>>>>>>> 
>>>>>>> Yes, when no consensus emerges, problems tend to go unsolved.
>>>>>>> 
>>>>>>> Before we appeal to authority to break the deadlock, we should make
>>>>>>> another attempt at finding consensus.
>>>>>>> 
>>>>>>> I know that we've entertained the idea of automatically generated IDs
>>>>>>> for block layer objects (that's why I cc'ed some block guys).
>>>>>> 
>>>>>> Yeah, I was one of the ones that proposed some auto-generated IDs for
>>>>>> the block layer, specifically for BlockDriverState, making use of the
>>>>>> node-name field that Benoit introduced a while ago.  Here is my patch
>>>>>> (not sure if this is the latest version, but it is sufficient for this
>>>>>> discussion):
>>>>>> 
>>>>>> http://patchwork.ozlabs.org/patch/355990/
>>>>>> 
>>>>>> I'm not sure about the requirements needed by device ID names, and
>>>>>> they may of course differ from what I was thinking for BDS entries.
>>>>>> 
>>>>>> Here is what I was after with my patch for node-name auto-generation:
>>>>>> 
>>>>>> * Identifiable as QEMU generated / reserved namespace
>>>>>> 
>>>>>> * Guaranteed uniqueness
>>>>>> 
>>>>>> * Non-predictable (don't want users trying to guess / assume
>>>>>> generated node-names)
>>>>>> 
>>>>>> My approach was overkill in some ways (24 characters!).  But for
>>>>>> better or worse, what I had was:
>>>>>> 
>>>>>>              __qemu##00000000IAIYNXXR
>>>>>>              ^^^^^^^^
>>>>>> QEMU namespace ----|    ^^^^^^^^
>>>>>>                        |     ^^^^^^^^^
>>>>>> Increment counter, unique |         |
>>>>>>                                  |
>>>>>> Random string, to spoil prediction  |
>>>>> 
>>>>> Yikes! 24 characters long. That is a bit much to type. Thank you very much
>>>>> for your effort.
>>>> 
>>>> IMO, the number of characters to type is pretty low on the list of
>>>> requirements, although it can still be addressed secondary to other
>>>> concerns.
>>>> 
>>>> I should have made this in reply to Markus' other email, because the
>>>> important part of this is try and address his point #2:
>>>> 
>>>> (from Markus' other email):
>>>>> 2. The ID must be well-formed.
>>>> 
>>>> To have a well-formed ID, we need to have know requirements of the ID
>>>> structure (i.e. the why and what of it all)
>>>> 
>>>> I don't know if the three requirements I had above apply to all areas
>>>> in QEMU, but I expect they do, in varying degrees of importance.  The
>>>> length itself can be tweaked.
>>>> 
>>>> Talking with John Snow over IRC (added to the CC), one thing he
>>>> suggested was adding in sub-domain spaces; e.g.:
>>>> 
>>>> __qemu#bn#00000000#IAIYNXXR
>>>> 
>>>> Where the 'bn' in this case would be for Block Nodes, etc..
>>>> 
>>>> This may make the scheme extensible through QEMU, where auto-generated
>>>> IDs are desired.
>>>> 
>>>> (sorry to say, this lengthens things, rather than shortening them!)
>>>> 
>>>> We can, of course, make the string shorter - if the random characters
>>>> are just there for spoiling predictability, then 2-3 should be
>>>> sufficient. We could then end up with something like this:
>>>> 
>>>> __qemu#bn#00000000#XR
>>>> 
>>>> The "__qemu" part of the namespace could be shortened as well, but it
>>>> would be nice if it was easy recognizable as being from QEMU.
>>> 
>>> If this ID format was supported, I'm thinking being able to copy and paste from
>>> the monitor is a necessary feature. 
>>> 
>>> Any way it could be shorted? I was hoping no more than three characters long. 
>>> 
>> 
>> Likely could be shorter, but something in the realm of three
>> characters doesn't seem very realistic.
>> 
>>> If this were the format of the ID, maybe we could put the value in a table that
>>> would translate this long ID to a shorter version. Or maybe a mathematical function
>>> could be applied to the value to give it some user-friendly value.
>> 
>> I'm afraid this would discard pretty much all the benefits of the ID
>> generation scheme.
> 
> At this point, why not specify a user-friendly ID yourself?
Right now, if the user forgets to specifiy an ID, device_del can't work. That's not good enough.

> If there is
> some technical reason you cannot, maybe we should fix the interface to
> allow you to do so.
I can do it myself, but having QEMU do it for the user makes things better and easier for the user.

> 
> Auto-generated IDs are not likely to be short, pretty, or easy to type
> due to the constraints Jeff Cody laid out earlier.
I think we are going to have to agree to disagree on this point.
Programmingkid Aug. 27, 2015, 3:40 a.m. UTC | #29
On Aug 26, 2015, at 6:08 PM, John Snow wrote:

> 
> 
> On 08/26/2015 05:48 PM, Programmingkid wrote:
>> 
>> On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote:
>> 
>>> On 26 August 2015 at 18:16, Programmingkid <programmingkidx@gmail.com> wrote:
>>>> That is assuming they have the time and/or the interest in solving this problem. I
>>>> suppose giving them some time to respond would be reasonable. I'm thinking if
>>>> no consensus has been reached in one weeks time (starting today), we turn to
>>>> Peter Maydell for the answer. Hopefully he will just pick which of the patches he
>>>> likes the best. Judging by how long this problem has been ongoing, someone
>>>> pick the answer is probably the best we can expect.
>>> 
>>> This is the kind of thing I strongly prefer to leave to the
>>> relevant subsystem maintainer(s). My opinion is not worth
>>> a great deal since I don't have a strong familiarity with
>>> this bit of QEMU.
>> 
>> It looks unreasonable to assume any consensus can be reached over this issue.
>> The easy thing to do is to just let each maintainer deal with this problem
>> his own way. 
>> 
> 
> What feedback was there that seemed insurmountable? Last I talked to
> Jeff Cody he said there was no "overwhelming pushback" against his
> patches, just a list of concerns.

Markus Armbruster sent me four different threads each trying to solve this problem.
Some of those threads were many years old. The situation is the same then as it
is now. There is no judicator to decide how this problem is to be solved. Expecting
all the maintainers to agree on one patch is unrealistic. 

> This doesn't sound like a dead end so much as it sounds like we haven't
> planned the feature enough yet.

The threads did have some really good patches that did seem to solve the problem.
I could send you the threads if you haven't read them yet.

> 
>> Markus:
>> I know you really wanted a single ID generating system, but it just isn't going
>> to happen. I will make a patch that would only effect USB devices. All other
>> devices would be untouched. At least the device_del problem will be solved.
>> 
> 
> I think this is being unnecessarily hasty. We should make sure that an
> auto-generated ID system does not create problems for other areas of
> code before we rush ahead with one to solve a single problem.

I would make sure my patch only affects USB devices. No other systems
would be affected. 

> Let's give the universal approach some more time before we jump to the
> conclusion that it's impossible.

I suppose 5 more years will do ;)

Maybe that's too soon...
Markus Armbruster Aug. 27, 2015, 5:39 a.m. UTC | #30
Programmingkid <programmingkidx@gmail.com> writes:

> On Aug 26, 2015, at 6:08 PM, John Snow wrote:
>
>> 
>> 
>> On 08/26/2015 05:48 PM, Programmingkid wrote:
>>> 
>>> On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote:
>>> 
>>>> On 26 August 2015 at 18:16, Programmingkid
>>>> <programmingkidx@gmail.com> wrote:
>>>>> That is assuming they have the time and/or the interest in
>>>>> solving this problem. I
>>>>> suppose giving them some time to respond would be reasonable. I'm
>>>>> thinking if
>>>>> no consensus has been reached in one weeks time (starting today),
>>>>> we turn to
>>>>> Peter Maydell for the answer. Hopefully he will just pick which
>>>>> of the patches he
>>>>> likes the best. Judging by how long this problem has been ongoing, someone
>>>>> pick the answer is probably the best we can expect.
>>>> 
>>>> This is the kind of thing I strongly prefer to leave to the
>>>> relevant subsystem maintainer(s). My opinion is not worth
>>>> a great deal since I don't have a strong familiarity with
>>>> this bit of QEMU.
>>> 
>>> It looks unreasonable to assume any consensus can be reached over this issue.
>>> The easy thing to do is to just let each maintainer deal with this problem
>>> his own way. 
>>> 
>> 
>> What feedback was there that seemed insurmountable? Last I talked to
>> Jeff Cody he said there was no "overwhelming pushback" against his
>> patches, just a list of concerns.
>
> Markus Armbruster sent me four different threads each trying to solve
> this problem.
> Some of those threads were many years old. The situation is the same then as it
> is now. There is no judicator to decide how this problem is to be
> solved. Expecting
> all the maintainers to agree on one patch is unrealistic. 
>
>> This doesn't sound like a dead end so much as it sounds like we haven't
>> planned the feature enough yet.
>
> The threads did have some really good patches that did seem to solve
> the problem.
> I could send you the threads if you haven't read them yet.

Back then was then and now is now.  I understand your impatience to get
stuff done, but things may have changed, people may have changed.  We
really need to talk it over one more time.  If we can reach rough
consensus, swell.  If we can't, we can still narrow the scope to
subsystems where we can.

>>> Markus:
>>> I know you really wanted a single ID generating system, but it just
>>> isn't going
>>> to happen. I will make a patch that would only effect USB devices. All other
>>> devices would be untouched. At least the device_del problem will be solved.
>>> 
>> 
>> I think this is being unnecessarily hasty. We should make sure that an
>> auto-generated ID system does not create problems for other areas of
>> code before we rush ahead with one to solve a single problem.

Right.

> I would make sure my patch only affects USB devices. No other systems
> would be affected. 

Device IDs are in the qdev/QOM subsystem, so that's the subsystem you
have to attack should QEMU-wide consensus remain elusive.  The USB
subsystem is merely a user of qdev/QOM here.

>> Let's give the universal approach some more time before we jump to the
>> conclusion that it's impossible.
>
> I suppose 5 more years will do ;)
>
> Maybe that's too soon...

I understand your frustration with our collective inability to solve
this problem for 5+ years.  Heck, I share it!  I certainly don't want
the problem to be shelved *again*.  But a proper discussion should take
us perhaps days, at worst weeks (KVM Forum was last week, several folks
are taking time off right now), certainly not months, let alone years.

We can afford time for discussion.  That's how we work.  Rough consensus
and running code.  You're welcome to provide running code early, just be
prepared for it to be thrown out and redone :)
Jeff Cody Aug. 27, 2015, 12:32 p.m. UTC | #31
(Added Eric back in to the CC list.  Looks like he got dropped
somewhere along the way)

On Wed, Aug 26, 2015 at 11:22:08PM -0400, Programmingkid wrote:
> 
> On Aug 26, 2015, at 6:01 PM, Jeff Cody wrote:
> 
> > On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
> >> 
> >> On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
> >> 
> >>> On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
> >>>> 
> >>>> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
> >>>> 
> >>>>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
> >>>>>> Did you drop cc's intentionally?  I put them right back.
> >>>>>> 
> >>>>>> Programmingkid <programmingkidx@gmail.com> writes:
> >>>>>> 
> >>>>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
> >>>>>>> 
> >>>>>>>> You're proposing to revise a qdev design decision, namely the purpose of
> >>>>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
> >>>>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
> >>>>>>>> 
> >>>>>>>> Relevant prior threads:
> >>>>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
> >>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
> >>>>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
> >>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
> >>>>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
> >>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
> >>>>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
> >>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
> >>>>>>>> 
> >>>>>>> 
> >>>>>>> After reading all the threads, I realize why all the attempts to
> >>>>>>> accept a device ID patch failed.
> >>>>>>> It is because it was assumed everyone would agree on one patch to
> >>>>>>> accept. This is
> >>>>>>> very unlikely. It would take someone in a leadership position to
> >>>>>>> decide which patch
> >>>>>>> should be accepted. From one of the threads above, I saw Anthony
> >>>>>>> Liguori participate.
> >>>>>>> He was in the perfect position to make the choice. The person who is
> >>>>>>> in his position now
> >>>>>>> is Peter Maydell. Maybe we should just ask him to look at all the
> >>>>>>> candidate patches and
> >>>>>>> have him pick one to use. 
> >>>>>> 
> >>>>>> Yes, when no consensus emerges, problems tend to go unsolved.
> >>>>>> 
> >>>>>> Before we appeal to authority to break the deadlock, we should make
> >>>>>> another attempt at finding consensus.
> >>>>>> 
> >>>>>> I know that we've entertained the idea of automatically generated IDs
> >>>>>> for block layer objects (that's why I cc'ed some block guys).
> >>>>> 
> >>>>> Yeah, I was one of the ones that proposed some auto-generated IDs for
> >>>>> the block layer, specifically for BlockDriverState, making use of the
> >>>>> node-name field that Benoit introduced a while ago.  Here is my patch
> >>>>> (not sure if this is the latest version, but it is sufficient for this
> >>>>> discussion):
> >>>>> 
> >>>>> http://patchwork.ozlabs.org/patch/355990/
> >>>>> 
> >>>>> I'm not sure about the requirements needed by device ID names, and
> >>>>> they may of course differ from what I was thinking for BDS entries.
> >>>>> 
> >>>>> Here is what I was after with my patch for node-name auto-generation:
> >>>>> 
> >>>>> * Identifiable as QEMU generated / reserved namespace
> >>>>> 
> >>>>> * Guaranteed uniqueness
> >>>>> 
> >>>>> * Non-predictable (don't want users trying to guess / assume
> >>>>> generated node-names)
> >>>>> 
> >>>>> My approach was overkill in some ways (24 characters!).  But for
> >>>>> better or worse, what I had was:
> >>>>> 
> >>>>>              __qemu##00000000IAIYNXXR
> >>>>>              ^^^^^^^^
> >>>>> QEMU namespace ----|    ^^^^^^^^
> >>>>>                        |     ^^^^^^^^^
> >>>>> Increment counter, unique |         |
> >>>>>                                  |
> >>>>> Random string, to spoil prediction  |
> >>>> 
> >>>> Yikes! 24 characters long. That is a bit much to type. Thank you very much
> >>>> for your effort.
> >>> 
> >>> IMO, the number of characters to type is pretty low on the list of
> >>> requirements, although it can still be addressed secondary to other
> >>> concerns.
> >>> 
> >>> I should have made this in reply to Markus' other email, because the
> >>> important part of this is try and address his point #2:
> >>> 
> >>> (from Markus' other email):
> >>>> 2. The ID must be well-formed.
> >>> 
> >>> To have a well-formed ID, we need to have know requirements of the ID
> >>> structure (i.e. the why and what of it all)
> >>> 
> >>> I don't know if the three requirements I had above apply to all areas
> >>> in QEMU, but I expect they do, in varying degrees of importance.  The
> >>> length itself can be tweaked.
> >>> 
> >>> Talking with John Snow over IRC (added to the CC), one thing he
> >>> suggested was adding in sub-domain spaces; e.g.:
> >>> 
> >>> __qemu#bn#00000000#IAIYNXXR
> >>> 
> >>> Where the 'bn' in this case would be for Block Nodes, etc..
> >>> 
> >>> This may make the scheme extensible through QEMU, where auto-generated
> >>> IDs are desired.
> >>> 
> >>> (sorry to say, this lengthens things, rather than shortening them!)
> >>> 
> >>> We can, of course, make the string shorter - if the random characters
> >>> are just there for spoiling predictability, then 2-3 should be
> >>> sufficient. We could then end up with something like this:
> >>> 
> >>> __qemu#bn#00000000#XR
> >>> 
> >>> The "__qemu" part of the namespace could be shortened as well, but it
> >>> would be nice if it was easy recognizable as being from QEMU.
> >> 
> >> If this ID format was supported, I'm thinking being able to copy and paste from
> >> the monitor is a necessary feature. 
> >> 
> >> Any way it could be shorted? I was hoping no more than three characters long. 
> >> 
> > 
> > Likely could be shorter, but something in the realm of three
> > characters doesn't seem very realistic.
> 
> Sure it is. Just set device id's like this: 0, 1, 2, 3, 4, 5, 6....

I'm not married to the ID generation scheme I proposed.  

What I am trying to do, however, is have a technical discussion on
generating an ID in a well-formed manner.  And hopefully, in a way
that is useful to all interested subsystems, if possible.

Do you disagree with the requirements I listed above?  If so, it would
be useful to begin the discussion around that.  For ease of
discussion, I'll list them again:

* Reserved namespaces
* Uniqueness
* Non-predictable (to avoid inadvertently creating a de facto ABI)


. . .

On the generation scheme proposed above:

I understand that something you desire is an ID that is easier to
type.

If we wanted to make it shorter, perhaps we could have the number
counter be variable length:

            qemu#ss#D#XY
              |   | | |
qemu reserved -   | | |
                  | | |
subsystem name ---| | |
                    | |
    counter --------| |
                      |
    2-digit random ---|


The counter would just grow to however many digits are needed.  There
is another benefit to growing that number as well - we can use
whatever integer size we think is adequate in the code, without
affecting the generation scheme.

-Jeff
Eric Blake Aug. 27, 2015, 1 p.m. UTC | #32
On 08/27/2015 06:32 AM, Jeff Cody wrote:
> (Added Eric back in to the CC list.  Looks like he got dropped
> somewhere along the way)

No thanks to mailman's inept behavior that thinks that it is okay to
rewrite cc's to drop anyone that doesn't want duplicate email.  But
don't worry about it; I have my local mail setup to flag any message
in-reply-to an earlier one where I was in cc, precisely to work around
mailman stupidly dropping me from cc. [Ideally, I'd filter the duplicate
messages on my side, and turn off the broken mailman setting
server-side, but I haven't yet figured out how to get filters working on
my side that do that correctly.]  I'm hoping that mailman3 is not so
inept, and that this list archives can migrate to hyperkitty/mailman3 in
the not-too-distant future.


> 
> Do you disagree with the requirements I listed above?  If so, it would
> be useful to begin the discussion around that.  For ease of
> discussion, I'll list them again:
> 
> * Reserved namespaces
> * Uniqueness
> * Non-predictable (to avoid inadvertently creating a de facto ABI)

Dan made the point that if a name is unpredictable, then we have to
query to learn what name was assigned.  But if you add two or more
devices before querying, then you don't know which device has which
name.  Predictable might actually be better than non-predictable.

Better still might be fixing things to where we add a global command
line option that outright fails any attempt to create an unnamed object.
 The option would be off by default for back-compat.  But management
apps like libvirt can turn it on once they are prepared to name every
object they create (which in turn may imply fixing any remaining
interfaces that cannot name an object to add in that ability for
management to pass in a name).  Then there would be no unnamed objects,
no ambiguity, and no need to generate names.
Programmingkid Aug. 27, 2015, 1:33 p.m. UTC | #33
On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:

> (Added Eric back in to the CC list.  Looks like he got dropped
> somewhere along the way)
> 
> On Wed, Aug 26, 2015 at 11:22:08PM -0400, Programmingkid wrote:
>> 
>> On Aug 26, 2015, at 6:01 PM, Jeff Cody wrote:
>> 
>>> On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
>>>> 
>>>>> On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
>>>>>> 
>>>>>> On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
>>>>>> 
>>>>>>> On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
>>>>>>>> Did you drop cc's intentionally?  I put them right back.
>>>>>>>> 
>>>>>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>>>>>> 
>>>>>>>>> On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
>>>>>>>>> 
>>>>>>>>>> You're proposing to revise a qdev design decision, namely the purpose of
>>>>>>>>>> IDs.  This has been discussed before, and IDs remained unchanged.
>>>>>>>>>> Perhaps it's time to revisit this issue.  Cc'ing a few more people.
>>>>>>>>>> 
>>>>>>>>>> Relevant prior threads:
>>>>>>>>>> * [PATCH] qdev: Reject duplicate and anti-social device IDs
>>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
>>>>>>>>>> * [PATCH 6/6] qdev: Generate IDs for anonymous devices
>>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
>>>>>>>>>> * [PATCH] qdev: Assign a default device ID when none is provided.
>>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/249702
>>>>>>>>>> * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt
>>>>>>>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> After reading all the threads, I realize why all the attempts to
>>>>>>>>> accept a device ID patch failed.
>>>>>>>>> It is because it was assumed everyone would agree on one patch to
>>>>>>>>> accept. This is
>>>>>>>>> very unlikely. It would take someone in a leadership position to
>>>>>>>>> decide which patch
>>>>>>>>> should be accepted. From one of the threads above, I saw Anthony
>>>>>>>>> Liguori participate.
>>>>>>>>> He was in the perfect position to make the choice. The person who is
>>>>>>>>> in his position now
>>>>>>>>> is Peter Maydell. Maybe we should just ask him to look at all the
>>>>>>>>> candidate patches and
>>>>>>>>> have him pick one to use. 
>>>>>>>> 
>>>>>>>> Yes, when no consensus emerges, problems tend to go unsolved.
>>>>>>>> 
>>>>>>>> Before we appeal to authority to break the deadlock, we should make
>>>>>>>> another attempt at finding consensus.
>>>>>>>> 
>>>>>>>> I know that we've entertained the idea of automatically generated IDs
>>>>>>>> for block layer objects (that's why I cc'ed some block guys).
>>>>>>> 
>>>>>>> Yeah, I was one of the ones that proposed some auto-generated IDs for
>>>>>>> the block layer, specifically for BlockDriverState, making use of the
>>>>>>> node-name field that Benoit introduced a while ago.  Here is my patch
>>>>>>> (not sure if this is the latest version, but it is sufficient for this
>>>>>>> discussion):
>>>>>>> 
>>>>>>> http://patchwork.ozlabs.org/patch/355990/
>>>>>>> 
>>>>>>> I'm not sure about the requirements needed by device ID names, and
>>>>>>> they may of course differ from what I was thinking for BDS entries.
>>>>>>> 
>>>>>>> Here is what I was after with my patch for node-name auto-generation:
>>>>>>> 
>>>>>>> * Identifiable as QEMU generated / reserved namespace
>>>>>>> 
>>>>>>> * Guaranteed uniqueness
>>>>>>> 
>>>>>>> * Non-predictable (don't want users trying to guess / assume
>>>>>>> generated node-names)
>>>>>>> 
>>>>>>> My approach was overkill in some ways (24 characters!).  But for
>>>>>>> better or worse, what I had was:
>>>>>>> 
>>>>>>>             __qemu##00000000IAIYNXXR
>>>>>>>             ^^^^^^^^
>>>>>>> QEMU namespace ----|    ^^^^^^^^
>>>>>>>                       |     ^^^^^^^^^
>>>>>>> Increment counter, unique |         |
>>>>>>>                                 |
>>>>>>> Random string, to spoil prediction  |
>>>>>> 
>>>>>> Yikes! 24 characters long. That is a bit much to type. Thank you very much
>>>>>> for your effort.
>>>>> 
>>>>> IMO, the number of characters to type is pretty low on the list of
>>>>> requirements, although it can still be addressed secondary to other
>>>>> concerns.
>>>>> 
>>>>> I should have made this in reply to Markus' other email, because the
>>>>> important part of this is try and address his point #2:
>>>>> 
>>>>> (from Markus' other email):
>>>>>> 2. The ID must be well-formed.
>>>>> 
>>>>> To have a well-formed ID, we need to have know requirements of the ID
>>>>> structure (i.e. the why and what of it all)
>>>>> 
>>>>> I don't know if the three requirements I had above apply to all areas
>>>>> in QEMU, but I expect they do, in varying degrees of importance.  The
>>>>> length itself can be tweaked.
>>>>> 
>>>>> Talking with John Snow over IRC (added to the CC), one thing he
>>>>> suggested was adding in sub-domain spaces; e.g.:
>>>>> 
>>>>> __qemu#bn#00000000#IAIYNXXR
>>>>> 
>>>>> Where the 'bn' in this case would be for Block Nodes, etc..
>>>>> 
>>>>> This may make the scheme extensible through QEMU, where auto-generated
>>>>> IDs are desired.
>>>>> 
>>>>> (sorry to say, this lengthens things, rather than shortening them!)
>>>>> 
>>>>> We can, of course, make the string shorter - if the random characters
>>>>> are just there for spoiling predictability, then 2-3 should be
>>>>> sufficient. We could then end up with something like this:
>>>>> 
>>>>> __qemu#bn#00000000#XR
>>>>> 
>>>>> The "__qemu" part of the namespace could be shortened as well, but it
>>>>> would be nice if it was easy recognizable as being from QEMU.
>>>> 
>>>> If this ID format was supported, I'm thinking being able to copy and paste from
>>>> the monitor is a necessary feature. 
>>>> 
>>>> Any way it could be shorted? I was hoping no more than three characters long. 
>>>> 
>>> 
>>> Likely could be shorter, but something in the realm of three
>>> characters doesn't seem very realistic.
>> 
>> Sure it is. Just set device id's like this: 0, 1, 2, 3, 4, 5, 6....
> 
> I'm not married to the ID generation scheme I proposed.  
> 
> What I am trying to do, however, is have a technical discussion on
> generating an ID in a well-formed manner.  And hopefully, in a way
> that is useful to all interested subsystems, if possible.
> 
> Do you disagree with the requirements I listed above?  If so, it would
> be useful to begin the discussion around that.  For ease of
> discussion, I'll list them again:
> 
> * Reserved namespaces
> * Uniqueness
> * Non-predictable (to avoid inadvertently creating a de facto ABI)

Uniqueness is a must. Reserve namespaces? Why do we need to do this?
What is wrong with having a predictable ID?

Maybe we need to discuss where this ID is going to be used. I know I 
need it for the device_del monitor command. Any other places you or
anyone else knows it is used?

>  . .
> 
> On the generation scheme proposed above:
> 
> I understand that something you desire is an ID that is easier to
> type.
> 
> If we wanted to make it shorter, perhaps we could have the number
> counter be variable length:
> 
>            qemu#ss#D#XY
>              |   | | |
> qemu reserved -   | | |
>                  | | |
> subsystem name ---| | |
>                    | |
>    counter --------| |
>                      |
>    2-digit random ---|
> 
> 
> The counter would just grow to however many digits are needed.  There
> is another benefit to growing that number as well - we can use
> whatever integer size we think is adequate in the code, without
> affecting the generation scheme.
> 
> -Jeff

This system does seem easy to type. Do we need the "qemu" part?
It seems unnecessary. Maybe we could do this:

<subsystem name><counter>

Examples:
For the third block device it would look like this: bl3
For the seventh USB device it would look like this: ub7

Each subsystem would receive a two character code.
Programmingkid Aug. 27, 2015, 1:39 p.m. UTC | #34
On Aug 27, 2015, at 9:00 AM, Eric Blake wrote:

> On 08/27/2015 06:32 AM, Jeff Cody wrote:
>> (Added Eric back in to the CC list.  Looks like he got dropped
>> somewhere along the way)
> 
> No thanks to mailman's inept behavior that thinks that it is okay to
> rewrite cc's to drop anyone that doesn't want duplicate email.  But
> don't worry about it; I have my local mail setup to flag any message
> in-reply-to an earlier one where I was in cc, precisely to work around
> mailman stupidly dropping me from cc. [Ideally, I'd filter the duplicate
> messages on my side, and turn off the broken mailman setting
> server-side, but I haven't yet figured out how to get filters working on
> my side that do that correctly.]  I'm hoping that mailman3 is not so
> inept, and that this list archives can migrate to hyperkitty/mailman3 in
> the not-too-distant future.
> 
> 
>> 
>> Do you disagree with the requirements I listed above?  If so, it would
>> be useful to begin the discussion around that.  For ease of
>> discussion, I'll list them again:
>> 
>> * Reserved namespaces
>> * Uniqueness
>> * Non-predictable (to avoid inadvertently creating a de facto ABI)
> 
> Dan made the point that if a name is unpredictable, then we have to
> query to learn what name was assigned.  But if you add two or more
> devices before querying, then you don't know which device has which
> name.  Predictable might actually be better than non-predictable.

Its also more user-friendly.

> Better still might be fixing things to where we add a global command
> line option that outright fails any attempt to create an unnamed object.
> The option would be off by default for back-compat.  But management
> apps like libvirt can turn it on once they are prepared to name every
> object they create (which in turn may imply fixing any remaining
> interfaces that cannot name an object to add in that ability for
> management to pass in a name).  Then there would be no unnamed objects,
> no ambiguity, and no need to generate names.

I do agree with giving every device an ID, but I don't think failing if the user
forgets to give one is necessary. If libvirt doesn't give devices and ID, it
would probably benefit from having QEMU do it for libvirt.
Daniel P. Berrangé Aug. 27, 2015, 1:49 p.m. UTC | #35
On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:
> 
> > 
> > On the generation scheme proposed above:
> > 
> > I understand that something you desire is an ID that is easier to
> > type.
> > 
> > If we wanted to make it shorter, perhaps we could have the number
> > counter be variable length:
> > 
> >            qemu#ss#D#XY
> >              |   | | |
> > qemu reserved -   | | |
> >                  | | |
> > subsystem name ---| | |
> >                    | |
> >    counter --------| |
> >                      |
> >    2-digit random ---|
> > 
> > 
> > The counter would just grow to however many digits are needed.  There
> > is another benefit to growing that number as well - we can use
> > whatever integer size we think is adequate in the code, without
> > affecting the generation scheme.
> > 
> > -Jeff
> 
> This system does seem easy to type. Do we need the "qemu" part?
> It seems unnecessary. Maybe we could do this:
> 
> <subsystem name><counter>
> 
> Examples:
> For the third block device it would look like this: bl3
> For the seventh USB device it would look like this: ub7
> 
> Each subsystem would receive a two character code.

If we did have auto-generated names, we would need to come up with a
scheme that is not going to clash with any existing naming that users
of QEMU may already be doing, otherwise we risk causing a regression.
Something as simple as what you suggest has non-trivial chance of
clashing.

We should look at what characters QEMU currently forbids users
from using in an explicitly passed ID, and include one or more
of them in the name.  eg IIUC, QEMU forbids use of a leading
underscore in ID names, so auto-generated names could use an
leading _ to avoid clashing.

Regards,
Daniel
Daniel P. Berrangé Aug. 27, 2015, 1:51 p.m. UTC | #36
On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote:
> 
> > Better still might be fixing things to where we add a global command
> > line option that outright fails any attempt to create an unnamed object.
> > The option would be off by default for back-compat.  But management
> > apps like libvirt can turn it on once they are prepared to name every
> > object they create (which in turn may imply fixing any remaining
> > interfaces that cannot name an object to add in that ability for
> > management to pass in a name).  Then there would be no unnamed objects,
> > no ambiguity, and no need to generate names.
> 
> I do agree with giving every device an ID, but I don't think failing if the user
> forgets to give one is necessary. If libvirt doesn't give devices and ID, it
> would probably benefit from having QEMU do it for libvirt.

Libvirt always gives an explicit ID. It is impossible to rely on QEMU
assigning IDs, because there is no reliable way to identify what ID
QEMU assigned to each device after the fact. Other management apps
would have the same problem, so auto-generated IDs are pretty useless
in that respect.

Regards,
Daniel
Daniel P. Berrangé Aug. 27, 2015, 1:54 p.m. UTC | #37
On Wed, Aug 26, 2015 at 02:01:41PM -0400, Programmingkid wrote:
> > If a user is talking to the QEMU monitor directly there are plenty of ways
> > to go wrong, of which forgetting to provide an ID is a really minor one.
> 
> What other problems did you have in mind?
> 
> > That's why it is generally left to higher level mgmt layers to talk to
> > QEMU and deal with all the issues in this area. IOW if users are talking
> > to the monitor directly, IMHO they've already lost.
> 
> I'm not following you. What do you mean by higher level mgmt layers?

Using QEMU via libvirt, or a similar management layer and not try
to talk to the monitor and/or CLI args which are complex to get
right and not really designed for user friendliness in general.

> Let me put it this way, if a user were to add a usb device to QEMU, say
> a usb-mouse, but forgot to give it an ID. How do you expect that user to
> remove the device from QEMU?

object_del should be made to accept the QOM object path, eg the first
anonymous device appears with a path  /machine/peripheral-anon/device[0]
so you could just do 'object_del /machine/peripheral-anon/device[0]'

If people really want pretty short IDs, then they can remember to
specify them upfront, or use a higher level app that avoids this
kind of problem.

Regards,
Daniel
Programmingkid Aug. 27, 2015, 1:56 p.m. UTC | #38
On Aug 27, 2015, at 9:49 AM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:
>> 
>>> 
>>> On the generation scheme proposed above:
>>> 
>>> I understand that something you desire is an ID that is easier to
>>> type.
>>> 
>>> If we wanted to make it shorter, perhaps we could have the number
>>> counter be variable length:
>>> 
>>>           qemu#ss#D#XY
>>>             |   | | |
>>> qemu reserved -   | | |
>>>                 | | |
>>> subsystem name ---| | |
>>>                   | |
>>>   counter --------| |
>>>                     |
>>>   2-digit random ---|
>>> 
>>> 
>>> The counter would just grow to however many digits are needed.  There
>>> is another benefit to growing that number as well - we can use
>>> whatever integer size we think is adequate in the code, without
>>> affecting the generation scheme.
>>> 
>>> -Jeff
>> 
>> This system does seem easy to type. Do we need the "qemu" part?
>> It seems unnecessary. Maybe we could do this:
>> 
>> <subsystem name><counter>
>> 
>> Examples:
>> For the third block device it would look like this: bl3
>> For the seventh USB device it would look like this: ub7
>> 
>> Each subsystem would receive a two character code.
> 
> If we did have auto-generated names, we would need to come up with a
> scheme that is not going to clash with any existing naming that users
> of QEMU may already be doing, otherwise we risk causing a regression.
> Something as simple as what you suggest has non-trivial chance of
> clashing.

Actually there is a way to prevent clashing. When QEMU auto-generates a
name, it could scan all the ID's to see if there is a clash. If the ID is already
taken, just increment the ID until it is detected to be unique. The previous
threads on this subject has patches that did just that. This means that a
ID scheme that is just a single number would work without clashes. 

> 
> We should look at what characters QEMU currently forbids users
> from using in an explicitly passed ID, and include one or more
> of them in the name.  eg IIUC, QEMU forbids use of a leading
> underscore in ID names, so auto-generated names could use an
> leading _ to avoid clashing.

I'm thinking that it might be unnecessary to do all that. ID clash
detection is pretty easy to do.
Eric Blake Aug. 27, 2015, 2:01 p.m. UTC | #39
On 08/27/2015 07:51 AM, Daniel P. Berrange wrote:
> On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote:
>>
>>> Better still might be fixing things to where we add a global command
>>> line option that outright fails any attempt to create an unnamed object.
>>> The option would be off by default for back-compat.  But management
>>> apps like libvirt can turn it on once they are prepared to name every
>>> object they create (which in turn may imply fixing any remaining
>>> interfaces that cannot name an object to add in that ability for
>>> management to pass in a name).  Then there would be no unnamed objects,
>>> no ambiguity, and no need to generate names.
>>
>> I do agree with giving every device an ID, but I don't think failing if the user
>> forgets to give one is necessary. If libvirt doesn't give devices and ID, it
>> would probably benefit from having QEMU do it for libvirt.

No, you're misunderstanding our argument. The moment there is more than
one device with an auto-assigned name is the moment that management
doesn't know which device got which name, so it's better for management
to pick a name in the first place.

> 
> Libvirt always gives an explicit ID.

Except it doesn't, yet.  Libvirt still needs to be taught to name all
node devices (and I'm slowly trying to work on patches towards that goal).

>     It is impossible to rely on QEMU
> assigning IDs, because there is no reliable way to identify what ID
> QEMU assigned to each device after the fact. Other management apps
> would have the same problem, so auto-generated IDs are pretty useless
> in that respect.

It's not to say that auto-generated names would be useless when running
qemu manually from the command line, but I agree that management
probably can't safely rely on auto-generated names, and therefore
solving the issue of auto-generating names is less important.
Programmingkid Aug. 27, 2015, 2:01 p.m. UTC | #40
On Aug 27, 2015, at 9:51 AM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote:
>> 
>>> Better still might be fixing things to where we add a global command
>>> line option that outright fails any attempt to create an unnamed object.
>>> The option would be off by default for back-compat.  But management
>>> apps like libvirt can turn it on once they are prepared to name every
>>> object they create (which in turn may imply fixing any remaining
>>> interfaces that cannot name an object to add in that ability for
>>> management to pass in a name).  Then there would be no unnamed objects,
>>> no ambiguity, and no need to generate names.
>> 
>> I do agree with giving every device an ID, but I don't think failing if the user
>> forgets to give one is necessary. If libvirt doesn't give devices and ID, it
>> would probably benefit from having QEMU do it for libvirt.
> 
> Libvirt always gives an explicit ID. It is impossible to rely on QEMU
> assigning IDs, because there is no reliable way to identify what ID
> QEMU assigned to each device after the fact.

I did submit a patch that prints the ID's when using the monitor command
"info usb". You did give me an idea. I don't know if it is implemented yet. Is there
some command that prints out every device in QEMU with its ID? info all-devices?
Eric Blake Aug. 27, 2015, 2:02 p.m. UTC | #41
On 08/27/2015 07:56 AM, Programmingkid wrote:

>> If we did have auto-generated names, we would need to come up with a
>> scheme that is not going to clash with any existing naming that users
>> of QEMU may already be doing, otherwise we risk causing a regression.
>> Something as simple as what you suggest has non-trivial chance of
>> clashing.
> 
> Actually there is a way to prevent clashing. When QEMU auto-generates a
> name, it could scan all the ID's to see if there is a clash. If the ID is already
> taken, just increment the ID until it is detected to be unique. The previous
> threads on this subject has patches that did just that. This means that a
> ID scheme that is just a single number would work without clashes. 

No, because you cannot predict what FUTURE names the user will request.
 The name generated by qemu must be IMPOSSIBLE to request manually, and
not just one that happens not to clash at the current moment.
Programmingkid Aug. 27, 2015, 2:03 p.m. UTC | #42
On Aug 27, 2015, at 9:54 AM, Daniel P. Berrange wrote:

> On Wed, Aug 26, 2015 at 02:01:41PM -0400, Programmingkid wrote:
>>> If a user is talking to the QEMU monitor directly there are plenty of ways
>>> to go wrong, of which forgetting to provide an ID is a really minor one.
>> 
>> What other problems did you have in mind?
>> 
>>> That's why it is generally left to higher level mgmt layers to talk to
>>> QEMU and deal with all the issues in this area. IOW if users are talking
>>> to the monitor directly, IMHO they've already lost.
>> 
>> I'm not following you. What do you mean by higher level mgmt layers?
> 
> Using QEMU via libvirt, or a similar management layer and not try
> to talk to the monitor and/or CLI args which are complex to get
> right and not really designed for user friendliness in general.
> 
>> Let me put it this way, if a user were to add a usb device to QEMU, say
>> a usb-mouse, but forgot to give it an ID. How do you expect that user to
>> remove the device from QEMU?
> 
> object_del should be made to accept the QOM object path, eg the first
> anonymous device appears with a path  /machine/peripheral-anon/device[0]
> so you could just do 'object_del /machine/peripheral-anon/device[0]'
> 
> If people really want pretty short IDs, then they can remember to
> specify them upfront, or use a higher level app that avoids this
> kind of problem.

That seems a bit unforgiving. It is 2015. I think we can automate a few more systems.
Daniel P. Berrangé Aug. 27, 2015, 2:06 p.m. UTC | #43
On Thu, Aug 27, 2015 at 09:56:47AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 9:49 AM, Daniel P. Berrange wrote:
> 
> > On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote:
> >> 
> >> On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:
> >> 
> >>> 
> >>> On the generation scheme proposed above:
> >>> 
> >>> I understand that something you desire is an ID that is easier to
> >>> type.
> >>> 
> >>> If we wanted to make it shorter, perhaps we could have the number
> >>> counter be variable length:
> >>> 
> >>>           qemu#ss#D#XY
> >>>             |   | | |
> >>> qemu reserved -   | | |
> >>>                 | | |
> >>> subsystem name ---| | |
> >>>                   | |
> >>>   counter --------| |
> >>>                     |
> >>>   2-digit random ---|
> >>> 
> >>> 
> >>> The counter would just grow to however many digits are needed.  There
> >>> is another benefit to growing that number as well - we can use
> >>> whatever integer size we think is adequate in the code, without
> >>> affecting the generation scheme.
> >>> 
> >>> -Jeff
> >> 
> >> This system does seem easy to type. Do we need the "qemu" part?
> >> It seems unnecessary. Maybe we could do this:
> >> 
> >> <subsystem name><counter>
> >> 
> >> Examples:
> >> For the third block device it would look like this: bl3
> >> For the seventh USB device it would look like this: ub7
> >> 
> >> Each subsystem would receive a two character code.
> > 
> > If we did have auto-generated names, we would need to come up with a
> > scheme that is not going to clash with any existing naming that users
> > of QEMU may already be doing, otherwise we risk causing a regression.
> > Something as simple as what you suggest has non-trivial chance of
> > clashing.
> 
> Actually there is a way to prevent clashing. When QEMU auto-generates a
> name, it could scan all the ID's to see if there is a clash. If the ID is already
> taken, just increment the ID until it is detected to be unique. The previous
> threads on this subject has patches that did just that. This means that a
> ID scheme that is just a single number would work without clashes.

Nope that is not sufficient. Consider an application was doing the
following

  $ qemu -device virtio-blk ....  -monitor stdio
  (qemu) device_add virtio-blk,id=ub1

Now if QEMU assigns the disk specified on the command line the
ID value 'ub1', the user later attempts to hotplug a disk
with that same ID will fail. So it will cause a regression
where something an app was doing with old QEMU will now
result in an error.

We don't know what possible naming schemes an app may already
be using with QEMU, so the only safe thing is to invent an
ID format which is currently illegal to specify manually, so
we have a completely separate namespace for auto-generated
IDs from user generated IDs.

> > We should look at what characters QEMU currently forbids users
> > from using in an explicitly passed ID, and include one or more
> > of them in the name.  eg IIUC, QEMU forbids use of a leading
> > underscore in ID names, so auto-generated names could use an
> > leading _ to avoid clashing.
> 
> I'm thinking that it might be unnecessary to do all that. ID clash
> detection is pretty easy to do. 

We really do need todo this.

Regards,
Daniel
Jeff Cody Aug. 27, 2015, 2:07 p.m. UTC | #44
On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:
>

[snip]

> > 
> > I'm not married to the ID generation scheme I proposed.  
> > 
> > What I am trying to do, however, is have a technical discussion on
> > generating an ID in a well-formed manner.  And hopefully, in a way
> > that is useful to all interested subsystems, if possible.
> > 
> > Do you disagree with the requirements I listed above?  If so, it would
> > be useful to begin the discussion around that.  For ease of
> > discussion, I'll list them again:
> > 
> > * Reserved namespaces
> > * Uniqueness
> > * Non-predictable (to avoid inadvertently creating a de facto ABI)
> 
> Uniqueness is a must. 

Agree

> Reserve namespaces? Why do we need to do this?

We need to prevent the user from selecting, inadvertently, the same ID
as a generated one.  This may also be harder to have consistent across
all subsystems.

> What is wrong with having a predictable ID?
> 

As Daniel and Eric have noted, it could be nice to have a predictable
ID.  My concern with a predictable ID is that it creates, across
multiple sub-systems, an ABI that we will then need to make sure
always works.

For instance, I don't want management software or a user to rely on us
parsing devices, or image filenames / block driver states in a certain
order, and then anticipate the ID name.  I am concerned about creating
an interface that may inadvertently "break" later on, and imposing a
burden on QEMU that isn't reasonable.  Perhaps it is enough to just
rely on documentation for this, without enforcing it in the scheme.


> Maybe we need to discuss where this ID is going to be used. I know I 
> need it for the device_del monitor command. Any other places you or
> anyone else knows it is used?
> 

In the block layer, we have BlockDriverStates, that represent image
nodes and backing files.  If we have a chain such as this:


Virtio0:

[base] <--- [filenameA] <--- [filenameB]

We used to reference an individual node by the device string (e.g.
"virtio0"), in conjunction with the filename.  The problem was, that
this was prone to error.  A node-name was added, which is essentially
a unique identifier for each device.  So then block commands (such as
block jobs) could reference a node in an unambiguous manner.

This is the area for an auto-generated ID that I was focused on.

> >  . .
> > 
> > On the generation scheme proposed above:
> > 
> > I understand that something you desire is an ID that is easier to
> > type.
> > 
> > If we wanted to make it shorter, perhaps we could have the number
> > counter be variable length:
> > 
> >            qemu#ss#D#XY
> >              |   | | |
> > qemu reserved -   | | |
> >                  | | |
> > subsystem name ---| | |
> >                    | |
> >    counter --------| |
> >                      |
> >    2-digit random ---|
> > 
> > 
> > The counter would just grow to however many digits are needed.  There
> > is another benefit to growing that number as well - we can use
> > whatever integer size we think is adequate in the code, without
> > affecting the generation scheme.
> > 
> > -Jeff
> 
> This system does seem easy to type. Do we need the "qemu" part?
> It seems unnecessary. Maybe we could do this:
> 
> <subsystem name><counter>
> 
> Examples:
> For the third block device it would look like this: bl3
> For the seventh USB device it would look like this: ub7
> 
> Each subsystem would receive a two character code. 
>
Jeff Cody Aug. 27, 2015, 2:18 p.m. UTC | #45
On Thu, Aug 27, 2015 at 08:01:12AM -0600, Eric Blake wrote:
> On 08/27/2015 07:51 AM, Daniel P. Berrange wrote:
> > On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote:
> >>
> >>> Better still might be fixing things to where we add a global command
> >>> line option that outright fails any attempt to create an unnamed object.
> >>> The option would be off by default for back-compat.  But management
> >>> apps like libvirt can turn it on once they are prepared to name every
> >>> object they create (which in turn may imply fixing any remaining
> >>> interfaces that cannot name an object to add in that ability for
> >>> management to pass in a name).  Then there would be no unnamed objects,
> >>> no ambiguity, and no need to generate names.
> >>
> >> I do agree with giving every device an ID, but I don't think failing if the user
> >> forgets to give one is necessary. If libvirt doesn't give devices and ID, it
> >> would probably benefit from having QEMU do it for libvirt.
> 
> No, you're misunderstanding our argument. The moment there is more than
> one device with an auto-assigned name is the moment that management
> doesn't know which device got which name, so it's better for management
> to pick a name in the first place.
> 
> > 
> > Libvirt always gives an explicit ID.
> 
> Except it doesn't, yet.  Libvirt still needs to be taught to name all
> node devices (and I'm slowly trying to work on patches towards that goal).
> 
> >     It is impossible to rely on QEMU
> > assigning IDs, because there is no reliable way to identify what ID
> > QEMU assigned to each device after the fact. Other management apps
> > would have the same problem, so auto-generated IDs are pretty useless
> > in that respect.
> 
> It's not to say that auto-generated names would be useless when running
> qemu manually from the command line, but I agree that management
> probably can't safely rely on auto-generated names, and therefore
> solving the issue of auto-generating names is less important.
>

I agree that the main benefit for auto-generated names in the
immediate timeframe is likely for the user directly invoking QEMU.
This can be painful and very cumbersome for a user opening a QEMU
image with a large backing chain, for instance.

However, at least in the block layer, it may be nice in the future for
QEMU to have essentially unique IDs that it knows unambiguously
identifies each BDS. Perhaps it would be for BDSs that are not created
or specified by libvirt, but done so internally, and exposed later.

Jeff
Programmingkid Aug. 27, 2015, 2:19 p.m. UTC | #46
On Aug 27, 2015, at 10:01 AM, Eric Blake wrote:

> On 08/27/2015 07:51 AM, Daniel P. Berrange wrote:
>> On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote:
>>> 
>>>> Better still might be fixing things to where we add a global command
>>>> line option that outright fails any attempt to create an unnamed object.
>>>> The option would be off by default for back-compat.  But management
>>>> apps like libvirt can turn it on once they are prepared to name every
>>>> object they create (which in turn may imply fixing any remaining
>>>> interfaces that cannot name an object to add in that ability for
>>>> management to pass in a name).  Then there would be no unnamed objects,
>>>> no ambiguity, and no need to generate names.
>>> 
>>> I do agree with giving every device an ID, but I don't think failing if the user
>>> forgets to give one is necessary. If libvirt doesn't give devices and ID, it
>>> would probably benefit from having QEMU do it for libvirt.
> 
> No, you're misunderstanding our argument. The moment there is more than
> one device with an auto-assigned name is the moment that management
> doesn't know which device got which name, so it's better for management
> to pick a name in the first place.

Ok.  I see. You might be assuming that QEMU will be the only one to give a device
an ID. The ID will ONLY be given if the user or libvirt *doesn't* give it. So libvirt would
be able to function just fine with an ID generating system. 

>> 
>> Libvirt always gives an explicit ID.
> 
> Except it doesn't, yet.  Libvirt still needs to be taught to name all
> node devices (and I'm slowly trying to work on patches towards that goal).

Well if Libvirt doesn't give an ID to a device, then it won't be able to use monitor
commands that depend on the ID. This sounds like a completely different problem.

> 
>>    It is impossible to rely on QEMU
>> assigning IDs, because there is no reliable way to identify what ID
>> QEMU assigned to each device after the fact. Other management apps
>> would have the same problem, so auto-generated IDs are pretty useless
>> in that respect.
> 
> It's not to say that auto-generated names would be useless when running
> qemu manually from the command line, but I agree that management
> probably can't safely rely on auto-generated names, and therefore
> solving the issue of auto-generating names is less important.

What if we change this so that management applications could actually query
devices and their properties. I'm thinking some kind of JSON-like output.
Something like 'info all-devices'. It prints the device name, location, and ID.
This list should be easy to parse by Libvirt.
Programmingkid Aug. 27, 2015, 2:34 p.m. UTC | #47
On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:

> On 08/27/2015 07:56 AM, Programmingkid wrote:
> 
>>> If we did have auto-generated names, we would need to come up with a
>>> scheme that is not going to clash with any existing naming that users
>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>> Something as simple as what you suggest has non-trivial chance of
>>> clashing.
>> 
>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>> taken, just increment the ID until it is detected to be unique. The previous
>> threads on this subject has patches that did just that. This means that a
>> ID scheme that is just a single number would work without clashes. 
> 
> No, because you cannot predict what FUTURE names the user will request.
> The name generated by qemu must be IMPOSSIBLE to request manually, and
> not just one that happens not to clash at the current moment.

If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
taken. I could just increment the ID myself until I find one that is unique. It is a
simple algorithm. Maybe you are talking about some program that has hard coded
ID's it depends on. If that is the case, perhaps this management program could be
made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
guaranteed to always be unique.

What about this scenario. There are 1 million devices added to QEMU, and I need to
add a device with an ID. Each of the 1 million devices already have an ID. If I don't want
to try to find a unique ID myself, having QEMU do it for me would make things much easier.
How would I find this device's ID? I could issue some kind of monitor command that gives me
the ID. This feature doesn't appear to be implemented yet. This will change. A future
' info all-devices '  command would probably do it.
Daniel P. Berrangé Aug. 27, 2015, 2:42 p.m. UTC | #48
On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
> 
> > On 08/27/2015 07:56 AM, Programmingkid wrote:
> > 
> >>> If we did have auto-generated names, we would need to come up with a
> >>> scheme that is not going to clash with any existing naming that users
> >>> of QEMU may already be doing, otherwise we risk causing a regression.
> >>> Something as simple as what you suggest has non-trivial chance of
> >>> clashing.
> >> 
> >> Actually there is a way to prevent clashing. When QEMU auto-generates a
> >> name, it could scan all the ID's to see if there is a clash. If the ID is already
> >> taken, just increment the ID until it is detected to be unique. The previous
> >> threads on this subject has patches that did just that. This means that a
> >> ID scheme that is just a single number would work without clashes. 
> > 
> > No, because you cannot predict what FUTURE names the user will request.
> > The name generated by qemu must be IMPOSSIBLE to request manually, and
> > not just one that happens not to clash at the current moment.
> 
> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
> taken. I could just increment the ID myself until I find one that is unique. It is a
> simple algorithm. Maybe you are talking about some program that has hard coded
> ID's it depends on. If that is the case, perhaps this management program could be
> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
> guaranteed to always be unique.

If breaking existing apps was OK, we would just mandate that users always
provide an ID which trivially avoids the problem of not having an ID to
use when deleting the object. We want to /avoid/ breaking existing apps
though, so saying an app should change the way it works in order to cope
with QEMU's ID generation is not appropriate. Hence any use of auto-generated
IDs, must use a separate namespace to avoid such clashes.

> 
> What about this scenario. There are 1 million devices added to QEMU, and I need to
> add a device with an ID. Each of the 1 million devices already have an ID. If I don't want
> to try to find a unique ID myself, having QEMU do it for me would make things much easier.
> How would I find this device's ID? I could issue some kind of monitor command that gives me
> the ID. This feature doesn't appear to be implemented yet. This will change. A future
> ' info all-devices '  command would probably do it.

If you're working at that kind of scale, then honestly apps are already
just going to be specifying an ID explicitly so we have no problem. It
is orders of magnitude simpler todo that than to try to parse any
'info all-devices' output in order to determine QEMU's auto-generated
ID value.

Regards,
Daniel
Programmingkid Aug. 27, 2015, 2:54 p.m. UTC | #49
On Aug 27, 2015, at 10:06 AM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 09:56:47AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 9:49 AM, Daniel P. Berrange wrote:
>> 
>>> On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:
>>>> 
>>>>> 
>>>>> On the generation scheme proposed above:
>>>>> 
>>>>> I understand that something you desire is an ID that is easier to
>>>>> type.
>>>>> 
>>>>> If we wanted to make it shorter, perhaps we could have the number
>>>>> counter be variable length:
>>>>> 
>>>>>          qemu#ss#D#XY
>>>>>            |   | | |
>>>>> qemu reserved -   | | |
>>>>>                | | |
>>>>> subsystem name ---| | |
>>>>>                  | |
>>>>>  counter --------| |
>>>>>                    |
>>>>>  2-digit random ---|
>>>>> 
>>>>> 
>>>>> The counter would just grow to however many digits are needed.  There
>>>>> is another benefit to growing that number as well - we can use
>>>>> whatever integer size we think is adequate in the code, without
>>>>> affecting the generation scheme.
>>>>> 
>>>>> -Jeff
>>>> 
>>>> This system does seem easy to type. Do we need the "qemu" part?
>>>> It seems unnecessary. Maybe we could do this:
>>>> 
>>>> <subsystem name><counter>
>>>> 
>>>> Examples:
>>>> For the third block device it would look like this: bl3
>>>> For the seventh USB device it would look like this: ub7
>>>> 
>>>> Each subsystem would receive a two character code.
>>> 
>>> If we did have auto-generated names, we would need to come up with a
>>> scheme that is not going to clash with any existing naming that users
>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>> Something as simple as what you suggest has non-trivial chance of
>>> clashing.
>> 
>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>> taken, just increment the ID until it is detected to be unique. The previous
>> threads on this subject has patches that did just that. This means that a
>> ID scheme that is just a single number would work without clashes.
> 
> Nope that is not sufficient. Consider an application was doing the
> following
> 
>  $ qemu -device virtio-blk ....  -monitor stdio
>  (qemu) device_add virtio-blk,id=ub1
> 
> Now if QEMU assigns the disk specified on the command line the
> ID value 'ub1', the user later attempts to hotplug a disk
> with that same ID will fail. So it will cause a regression
> where something an app was doing with old QEMU will now
> result in an error.

Yes adding a device with an ID that is taken would not work. Maybe
QEMU could help you in this situation. If in the above scenario, the user
were to issue a device_add using a already used ID, QEMU could tell
the user that that ID is taken, but it can have this auto-generated ID. 
Instead of being able to use ub1, QEMU could say:

Note: ID ub1 taken, use ub2 instead? [y] [n]

> 
> We don't know what possible naming schemes an app may already
> be using with QEMU, so the only safe thing is to invent an
> ID format which is currently illegal to specify manually, so
> we have a completely separate namespace for auto-generated
> IDs from user generated IDs.

Numbers go on forever. No matter what naming scheme an application
might be using, QEMU could just add one to the highest taken number
and solve the problem.

> 
>>> We should look at what characters QEMU currently forbids users
>>> from using in an explicitly passed ID, and include one or more
>>> of them in the name.  eg IIUC, QEMU forbids use of a leading
>>> underscore in ID names, so auto-generated names could use an
>>> leading _ to avoid clashing.
>> 
>> I'm thinking that it might be unnecessary to do all that. ID clash
>> detection is pretty easy to do. 
> 
> We really do need todo this.

I think most people will be ok with using an underscore in an auto-generated
ID system. The underscore rule sounds good. It would have the look of
a machine generated ID.
Programmingkid Aug. 27, 2015, 3:13 p.m. UTC | #50
On Aug 27, 2015, at 10:07 AM, Jeff Cody wrote:

> On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote:
>> 
> 
> [snip]
> 
>>> 
>>> I'm not married to the ID generation scheme I proposed.  
>>> 
>>> What I am trying to do, however, is have a technical discussion on
>>> generating an ID in a well-formed manner.  And hopefully, in a way
>>> that is useful to all interested subsystems, if possible.
>>> 
>>> Do you disagree with the requirements I listed above?  If so, it would
>>> be useful to begin the discussion around that.  For ease of
>>> discussion, I'll list them again:
>>> 
>>> * Reserved namespaces
>>> * Uniqueness
>>> * Non-predictable (to avoid inadvertently creating a de facto ABI)
>> 
>> Uniqueness is a must. 
> 
> Agree
> 
>> Reserve namespaces? Why do we need to do this?
> 
> We need to prevent the user from selecting, inadvertently, the same ID
> as a generated one.  This may also be harder to have consistent across
> all subsystems.

If preventing the user from entering the same ID as a generated one is that
important, enforcing rules on ID's could work. 

This is what might work:

QEMU's auto-generated ID's much begin with an underscore. Example:  _34.
User ID's cannot start with an underscore. Example: 34.

* No auto-generated/User ID clashes
* Easy to type
* Management application probably don't use underscores at the beginning
  of an ID making it safe for them.

> 
>> What is wrong with having a predictable ID?
>> 
> 
> As Daniel and Eric have noted, it could be nice to have a predictable
> ID.  My concern with a predictable ID is that it creates, across
> multiple sub-systems, an ABI that we will then need to make sure
> always works.
> 
> For instance, I don't want management software or a user to rely on us
> parsing devices, or image filenames / block driver states in a certain
> order, and then anticipate the ID name.  I am concerned about creating
> an interface that may inadvertently "break" later on, and imposing a
> burden on QEMU that isn't reasonable.  Perhaps it is enough to just
> rely on documentation for this, without enforcing it in the scheme.

If we do nothing, QEMU stays broken. The monitor command device_del
and others that need an ID will not work still. Hopefully any changes we
make to QEMU will be robust enough stand the test of time.

> 
>> Maybe we need to discuss where this ID is going to be used. I know I 
>> need it for the device_del monitor command. Any other places you or
>> anyone else knows it is used?
>> 
> 
> In the block layer, we have BlockDriverStates, that represent image
> nodes and backing files.  If we have a chain such as this:
> 
> 
> Virtio0:
> 
> [base] <--- [filenameA] <--- [filenameB]
> 
> We used to reference an individual node by the device string (e.g.
> "virtio0"), in conjunction with the filename.  The problem was, that
> this was prone to error.  A node-name was added, which is essentially
> a unique identifier for each device.  So then block commands (such as
> block jobs) could reference a node in an unambiguous manner.
> 
> This is the area for an auto-generated ID that I was focused on.

I'm not very familiar with this system. If auto-generated ID's could
harm this system, then maybe this system should be left alone by the
auto-generation feature. 

> 
>>> . .
>>> 
>>> On the generation scheme proposed above:
>>> 
>>> I understand that something you desire is an ID that is easier to
>>> type.
>>> 
>>> If we wanted to make it shorter, perhaps we could have the number
>>> counter be variable length:
>>> 
>>>           qemu#ss#D#XY
>>>             |   | | |
>>> qemu reserved -   | | |
>>>                 | | |
>>> subsystem name ---| | |
>>>                   | |
>>>   counter --------| |
>>>                     |
>>>   2-digit random ---|
>>> 
>>> 
>>> The counter would just grow to however many digits are needed.  There
>>> is another benefit to growing that number as well - we can use
>>> whatever integer size we think is adequate in the code, without
>>> affecting the generation scheme.
>>> 
>>> -Jeff
>> 
>> This system does seem easy to type. Do we need the "qemu" part?
>> It seems unnecessary. Maybe we could do this:
>> 
>> <subsystem name><counter>
>> 
>> Examples:
>> For the third block device it would look like this: bl3
>> For the seventh USB device it would look like this: ub7
>> 
>> Each subsystem would receive a two character code. 
>>
Daniel P. Berrangé Aug. 27, 2015, 3:19 p.m. UTC | #51
On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote:
> > 
> >> What is wrong with having a predictable ID?
> >> 
> > 
> > As Daniel and Eric have noted, it could be nice to have a predictable
> > ID.  My concern with a predictable ID is that it creates, across
> > multiple sub-systems, an ABI that we will then need to make sure
> > always works.
> > 
> > For instance, I don't want management software or a user to rely on us
> > parsing devices, or image filenames / block driver states in a certain
> > order, and then anticipate the ID name.  I am concerned about creating
> > an interface that may inadvertently "break" later on, and imposing a
> > burden on QEMU that isn't reasonable.  Perhaps it is enough to just
> > rely on documentation for this, without enforcing it in the scheme.
> 
> If we do nothing, QEMU stays broken. The monitor command device_del
> and others that need an ID will not work still. Hopefully any changes we
> make to QEMU will be robust enough stand the test of time.

That is not correct. It is possible for us to fix object_del / device_del
to accept the QOM object path. It isn't pretty but it is a solution that
gives everything a stable unique path ID to use for deletion even if the
user forgets to give a pretty path-less ID.

Regards,
Daniel
Programmingkid Aug. 27, 2015, 3:20 p.m. UTC | #52
On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
>> 
>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
>>> 
>>>>> If we did have auto-generated names, we would need to come up with a
>>>>> scheme that is not going to clash with any existing naming that users
>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>>>> Something as simple as what you suggest has non-trivial chance of
>>>>> clashing.
>>>> 
>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>>>> taken, just increment the ID until it is detected to be unique. The previous
>>>> threads on this subject has patches that did just that. This means that a
>>>> ID scheme that is just a single number would work without clashes. 
>>> 
>>> No, because you cannot predict what FUTURE names the user will request.
>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
>>> not just one that happens not to clash at the current moment.
>> 
>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
>> taken. I could just increment the ID myself until I find one that is unique. It is a
>> simple algorithm. Maybe you are talking about some program that has hard coded
>> ID's it depends on. If that is the case, perhaps this management program could be
>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
>> guaranteed to always be unique.
> 
> If breaking existing apps was OK, we would just mandate that users always
> provide an ID which trivially avoids the problem of not having an ID to
> use when deleting the object. We want to /avoid/ breaking existing apps
> though, so saying an app should change the way it works in order to cope
> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
> IDs, must use a separate namespace to avoid such clashes.

This is making the assumption that an auto-generated ID system would break any existing
application. We don't know this. In fact, I predict a future patch would allow existing
applications to continue running without modification. The patch would be a win-win
for both users and any management application.

> 
>> 
>> What about this scenario. There are 1 million devices added to QEMU, and I need to
>> add a device with an ID. Each of the 1 million devices already have an ID. If I don't want
>> to try to find a unique ID myself, having QEMU do it for me would make things much easier.
>> How would I find this device's ID? I could issue some kind of monitor command that gives me
>> the ID. This feature doesn't appear to be implemented yet. This will change. A future
>> ' info all-devices '  command would probably do it.
> 
> If you're working at that kind of scale, then honestly apps are already
> just going to be specifying an ID explicitly so we have no problem. It
> is orders of magnitude simpler todo that than to try to parse any
> 'info all-devices' output in order to determine QEMU's auto-generated
> ID value.
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
Programmingkid Aug. 27, 2015, 3:22 p.m. UTC | #53
On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote:
>>> 
>>>> What is wrong with having a predictable ID?
>>>> 
>>> 
>>> As Daniel and Eric have noted, it could be nice to have a predictable
>>> ID.  My concern with a predictable ID is that it creates, across
>>> multiple sub-systems, an ABI that we will then need to make sure
>>> always works.
>>> 
>>> For instance, I don't want management software or a user to rely on us
>>> parsing devices, or image filenames / block driver states in a certain
>>> order, and then anticipate the ID name.  I am concerned about creating
>>> an interface that may inadvertently "break" later on, and imposing a
>>> burden on QEMU that isn't reasonable.  Perhaps it is enough to just
>>> rely on documentation for this, without enforcing it in the scheme.
>> 
>> If we do nothing, QEMU stays broken. The monitor command device_del
>> and others that need an ID will not work still. Hopefully any changes we
>> make to QEMU will be robust enough stand the test of time.
> 
> That is not correct. It is possible for us to fix object_del / device_del
> to accept the QOM object path. It isn't pretty but it is a solution that
> gives everything a stable unique path ID to use for deletion even if the
> user forgets to give a pretty path-less ID.

This QOM path might be better than nothing. Hopefully someone will make this
patch and share it with us.
Programmingkid Aug. 27, 2015, 3:39 p.m. UTC | #54
On Aug 27, 2015, at 1:39 AM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Aug 26, 2015, at 6:08 PM, John Snow wrote:
>> 
>>> 
>>> 
>>> On 08/26/2015 05:48 PM, Programmingkid wrote:
>>>> 
>>>> On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote:
>>>> 
>>>>> On 26 August 2015 at 18:16, Programmingkid
>>>>> <programmingkidx@gmail.com> wrote:
>>>>>> That is assuming they have the time and/or the interest in
>>>>>> solving this problem. I
>>>>>> suppose giving them some time to respond would be reasonable. I'm
>>>>>> thinking if
>>>>>> no consensus has been reached in one weeks time (starting today),
>>>>>> we turn to
>>>>>> Peter Maydell for the answer. Hopefully he will just pick which
>>>>>> of the patches he
>>>>>> likes the best. Judging by how long this problem has been ongoing, someone
>>>>>> pick the answer is probably the best we can expect.
>>>>> 
>>>>> This is the kind of thing I strongly prefer to leave to the
>>>>> relevant subsystem maintainer(s). My opinion is not worth
>>>>> a great deal since I don't have a strong familiarity with
>>>>> this bit of QEMU.
>>>> 
>>>> It looks unreasonable to assume any consensus can be reached over this issue.
>>>> The easy thing to do is to just let each maintainer deal with this problem
>>>> his own way. 
>>>> 
>>> 
>>> What feedback was there that seemed insurmountable? Last I talked to
>>> Jeff Cody he said there was no "overwhelming pushback" against his
>>> patches, just a list of concerns.
>> 
>> Markus Armbruster sent me four different threads each trying to solve
>> this problem.
>> Some of those threads were many years old. The situation is the same then as it
>> is now. There is no judicator to decide how this problem is to be
>> solved. Expecting
>> all the maintainers to agree on one patch is unrealistic. 
>> 
>>> This doesn't sound like a dead end so much as it sounds like we haven't
>>> planned the feature enough yet.
>> 
>> The threads did have some really good patches that did seem to solve
>> the problem.
>> I could send you the threads if you haven't read them yet.
> 
> Back then was then and now is now.  I understand your impatience to get
> stuff done, but things may have changed, people may have changed.  We
> really need to talk it over one more time.  If we can reach rough
> consensus, swell.  If we can't, we can still narrow the scope to
> subsystems where we can.
> 
>>>> Markus:
>>>> I know you really wanted a single ID generating system, but it just
>>>> isn't going
>>>> to happen. I will make a patch that would only effect USB devices. All other
>>>> devices would be untouched. At least the device_del problem will be solved.
>>>> 
>>> 
>>> I think this is being unnecessarily hasty. We should make sure that an
>>> auto-generated ID system does not create problems for other areas of
>>> code before we rush ahead with one to solve a single problem.
> 
> Right.
> 
>> I would make sure my patch only affects USB devices. No other systems
>> would be affected. 
> 
> Device IDs are in the qdev/QOM subsystem, so that's the subsystem you
> have to attack should QEMU-wide consensus remain elusive.  The USB
> subsystem is merely a user of qdev/QOM here.

I could make a patch that detects if the device is a USB device, then
give it an ID if none was specified. This patch would have no affect on other
non-USB devices. We could implement this patch for the time being. Then *if*
an universal device ID system does become available, then just start using 
it instead.

> 
>>> Let's give the universal approach some more time before we jump to the
>>> conclusion that it's impossible.
>> 
>> I suppose 5 more years will do ;)
>> 
>> Maybe that's too soon...
> 
> I understand your frustration with our collective inability to solve
> this problem for 5+ years.  Heck, I share it!  I certainly don't want
> the problem to be shelved *again*.  But a proper discussion should take
> us perhaps days, at worst weeks (KVM Forum was last week, several folks
> are taking time off right now), certainly not months, let alone years.

Ok. I know you don't want a deadline. What about a goal. Our goal could be
solving this universal device ID problem within two weeks, or we switch over
to plan b. Plan b would be giving only USB devices an ID if none were
specified. That way device_del would always work, and we can finally move
on from this device ID problem.

> We can afford time for discussion.  That's how we work.  Rough consensus
> and running code.  You're welcome to provide running code early, just be
> prepared for it to be thrown out and redone :)


Is it possible you would accept a patch that *only* gives USB devices an ID if 
none were specified. If you would, I will gladly make this patch and send it in. 

Given all the different views expressed so far, this universal device ID problem
will not likely be solved soon.
Jeff Cody Aug. 27, 2015, 3:40 p.m. UTC | #55
On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
> 
> > On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
> >> 
> >> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
> >> 
> >>> On 08/27/2015 07:56 AM, Programmingkid wrote:
> >>> 
> >>>>> If we did have auto-generated names, we would need to come up with a
> >>>>> scheme that is not going to clash with any existing naming that users
> >>>>> of QEMU may already be doing, otherwise we risk causing a regression.
> >>>>> Something as simple as what you suggest has non-trivial chance of
> >>>>> clashing.
> >>>> 
> >>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
> >>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
> >>>> taken, just increment the ID until it is detected to be unique. The previous
> >>>> threads on this subject has patches that did just that. This means that a
> >>>> ID scheme that is just a single number would work without clashes. 
> >>> 
> >>> No, because you cannot predict what FUTURE names the user will request.
> >>> The name generated by qemu must be IMPOSSIBLE to request manually, and
> >>> not just one that happens not to clash at the current moment.
> >> 
> >> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
> >> taken. I could just increment the ID myself until I find one that is unique. It is a
> >> simple algorithm. Maybe you are talking about some program that has hard coded
> >> ID's it depends on. If that is the case, perhaps this management program could be
> >> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
> >> guaranteed to always be unique.
> > 
> > If breaking existing apps was OK, we would just mandate that users always
> > provide an ID which trivially avoids the problem of not having an ID to
> > use when deleting the object. We want to /avoid/ breaking existing apps
> > though, so saying an app should change the way it works in order to cope
> > with QEMU's ID generation is not appropriate. Hence any use of auto-generated
> > IDs, must use a separate namespace to avoid such clashes.
> 
> This is making the assumption that an auto-generated ID system would break any existing
> application. We don't know this. In fact, I predict a future patch would allow existing
> applications to continue running without modification. The patch would be a win-win
> for both users and any management application.
>

Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
that ID is already taken" will break existing applications.

But we are all striving to make your prediction true, with this very
conversation.
Daniel P. Berrangé Aug. 27, 2015, 3:55 p.m. UTC | #56
On Thu, Aug 27, 2015 at 11:22:58AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote:
> 
> > On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote:
> >>> 
> >>>> What is wrong with having a predictable ID?
> >>>> 
> >>> 
> >>> As Daniel and Eric have noted, it could be nice to have a predictable
> >>> ID.  My concern with a predictable ID is that it creates, across
> >>> multiple sub-systems, an ABI that we will then need to make sure
> >>> always works.
> >>> 
> >>> For instance, I don't want management software or a user to rely on us
> >>> parsing devices, or image filenames / block driver states in a certain
> >>> order, and then anticipate the ID name.  I am concerned about creating
> >>> an interface that may inadvertently "break" later on, and imposing a
> >>> burden on QEMU that isn't reasonable.  Perhaps it is enough to just
> >>> rely on documentation for this, without enforcing it in the scheme.
> >> 
> >> If we do nothing, QEMU stays broken. The monitor command device_del
> >> and others that need an ID will not work still. Hopefully any changes we
> >> make to QEMU will be robust enough stand the test of time.
> > 
> > That is not correct. It is possible for us to fix object_del / device_del
> > to accept the QOM object path. It isn't pretty but it is a solution that
> > gives everything a stable unique path ID to use for deletion even if the
> > user forgets to give a pretty path-less ID.
> 
> This QOM path might be better than nothing. Hopefully someone will make this
> patch and share it with us.

I sent a patch to support that, since it turned out to be pretty
trivial to implement. So that at least solves the immediate blocking
issue of deleting devices with an ID. The question of usability and
auto-generated IDs can continue in parallel....

Regards,
Daniel
Programmingkid Aug. 27, 2015, 3:58 p.m. UTC | #57
On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote:

> On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
>> 
>>> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
>>>> 
>>>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
>>>>> 
>>>>>>> If we did have auto-generated names, we would need to come up with a
>>>>>>> scheme that is not going to clash with any existing naming that users
>>>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>>>>>> Something as simple as what you suggest has non-trivial chance of
>>>>>>> clashing.
>>>>>> 
>>>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>>>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>>>>>> taken, just increment the ID until it is detected to be unique. The previous
>>>>>> threads on this subject has patches that did just that. This means that a
>>>>>> ID scheme that is just a single number would work without clashes. 
>>>>> 
>>>>> No, because you cannot predict what FUTURE names the user will request.
>>>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
>>>>> not just one that happens not to clash at the current moment.
>>>> 
>>>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
>>>> taken. I could just increment the ID myself until I find one that is unique. It is a
>>>> simple algorithm. Maybe you are talking about some program that has hard coded
>>>> ID's it depends on. If that is the case, perhaps this management program could be
>>>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
>>>> guaranteed to always be unique.
>>> 
>>> If breaking existing apps was OK, we would just mandate that users always
>>> provide an ID which trivially avoids the problem of not having an ID to
>>> use when deleting the object. We want to /avoid/ breaking existing apps
>>> though, so saying an app should change the way it works in order to cope
>>> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
>>> IDs, must use a separate namespace to avoid such clashes.
>> 
>> This is making the assumption that an auto-generated ID system would break any existing
>> application. We don't know this. In fact, I predict a future patch would allow existing
>> applications to continue running without modification. The patch would be a win-win
>> for both users and any management application.
>> 
> 
> Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
> that ID is already taken" will break existing applications.
> 
> But we are all striving to make your prediction true, with this very
> conversation.

Ok, it sounds like some are concerned that Libvirt would not be able to work with this
feature. Let me ask you this: does Libvirt interact with QEMU before the user has a
chance to do so? If the answer is yes, then that means Libvirt will have finished 
creating all its devices with their ID's long before the user has a chance to enter
his own devices.
Daniel P. Berrangé Aug. 27, 2015, 4:02 p.m. UTC | #58
On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote:
> 
> > On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
> >> 
> >> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
> >> 
> >>> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
> >>>> 
> >>>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
> >>>> 
> >>>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
> >>>>> 
> >>>>>>> If we did have auto-generated names, we would need to come up with a
> >>>>>>> scheme that is not going to clash with any existing naming that users
> >>>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
> >>>>>>> Something as simple as what you suggest has non-trivial chance of
> >>>>>>> clashing.
> >>>>>> 
> >>>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
> >>>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
> >>>>>> taken, just increment the ID until it is detected to be unique. The previous
> >>>>>> threads on this subject has patches that did just that. This means that a
> >>>>>> ID scheme that is just a single number would work without clashes. 
> >>>>> 
> >>>>> No, because you cannot predict what FUTURE names the user will request.
> >>>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
> >>>>> not just one that happens not to clash at the current moment.
> >>>> 
> >>>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
> >>>> taken. I could just increment the ID myself until I find one that is unique. It is a
> >>>> simple algorithm. Maybe you are talking about some program that has hard coded
> >>>> ID's it depends on. If that is the case, perhaps this management program could be
> >>>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
> >>>> guaranteed to always be unique.
> >>> 
> >>> If breaking existing apps was OK, we would just mandate that users always
> >>> provide an ID which trivially avoids the problem of not having an ID to
> >>> use when deleting the object. We want to /avoid/ breaking existing apps
> >>> though, so saying an app should change the way it works in order to cope
> >>> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
> >>> IDs, must use a separate namespace to avoid such clashes.
> >> 
> >> This is making the assumption that an auto-generated ID system would break any existing
> >> application. We don't know this. In fact, I predict a future patch would allow existing
> >> applications to continue running without modification. The patch would be a win-win
> >> for both users and any management application.
> >> 
> > 
> > Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
> > that ID is already taken" will break existing applications.
> > 
> > But we are all striving to make your prediction true, with this very
> > conversation.
> 
> Ok, it sounds like some are concerned that Libvirt would not be able to work with this
> feature. Let me ask you this: does Libvirt interact with QEMU before the user has a
> chance to do so? If the answer is yes, then that means Libvirt will have finished 
> creating all its devices with their ID's long before the user has a chance to enter
> his own devices.

Just to be clear - libvirt will *never* use an auto-generated device
IDs feature. It is way more complicated to let QEMU assign device IDs
and then auto-detect them from some 'info device-list' output, than
to just specify IDs upfront at device/object creation time which
it already does[1]. There is simply no benefit to auto-generating device
IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
IDs will only be of interest to humans talking to the monitor directly
without a mgmt app involved.

Regards,
Daniel

[1] we don't provide IDs for qcow2 image backing file chain, but that's
    part of a bigger story that's being dealt with in this area.
Programmingkid Aug. 27, 2015, 4:03 p.m. UTC | #59
On Aug 27, 2015, at 11:55 AM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 11:22:58AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote:
>> 
>>> On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote:
>>>>> 
>>>>>> What is wrong with having a predictable ID?
>>>>>> 
>>>>> 
>>>>> As Daniel and Eric have noted, it could be nice to have a predictable
>>>>> ID.  My concern with a predictable ID is that it creates, across
>>>>> multiple sub-systems, an ABI that we will then need to make sure
>>>>> always works.
>>>>> 
>>>>> For instance, I don't want management software or a user to rely on us
>>>>> parsing devices, or image filenames / block driver states in a certain
>>>>> order, and then anticipate the ID name.  I am concerned about creating
>>>>> an interface that may inadvertently "break" later on, and imposing a
>>>>> burden on QEMU that isn't reasonable.  Perhaps it is enough to just
>>>>> rely on documentation for this, without enforcing it in the scheme.
>>>> 
>>>> If we do nothing, QEMU stays broken. The monitor command device_del
>>>> and others that need an ID will not work still. Hopefully any changes we
>>>> make to QEMU will be robust enough stand the test of time.
>>> 
>>> That is not correct. It is possible for us to fix object_del / device_del
>>> to accept the QOM object path. It isn't pretty but it is a solution that
>>> gives everything a stable unique path ID to use for deletion even if the
>>> user forgets to give a pretty path-less ID.
>> 
>> This QOM path might be better than nothing. Hopefully someone will make this
>> patch and share it with us.
> 
> I sent a patch to support that, since it turned out to be pretty
> trivial to implement. So that at least solves the immediate blocking
> issue of deleting devices with an ID. The question of usability and
> auto-generated IDs can continue in parallel....
> 
> Regards,
> Daniel

I applied your patch, but saw this error message when I tried to 'make' QEMU:

  GEN   qmp-commands.txt
line 344: syntax error: expected EQMP, found SQMP
make: *** [qmp-commands.txt] Error 1
make: *** Deleting file `qmp-commands.txt'

Know what it means?
Daniel P. Berrangé Aug. 27, 2015, 4:06 p.m. UTC | #60
On Thu, Aug 27, 2015 at 12:03:38PM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 11:55 AM, Daniel P. Berrange wrote:
> 
> > On Thu, Aug 27, 2015 at 11:22:58AM -0400, Programmingkid wrote:
> >> 
> >> On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote:
> >> 
> >>> On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote:
> >>>>> 
> >>>>>> What is wrong with having a predictable ID?
> >>>>>> 
> >>>>> 
> >>>>> As Daniel and Eric have noted, it could be nice to have a predictable
> >>>>> ID.  My concern with a predictable ID is that it creates, across
> >>>>> multiple sub-systems, an ABI that we will then need to make sure
> >>>>> always works.
> >>>>> 
> >>>>> For instance, I don't want management software or a user to rely on us
> >>>>> parsing devices, or image filenames / block driver states in a certain
> >>>>> order, and then anticipate the ID name.  I am concerned about creating
> >>>>> an interface that may inadvertently "break" later on, and imposing a
> >>>>> burden on QEMU that isn't reasonable.  Perhaps it is enough to just
> >>>>> rely on documentation for this, without enforcing it in the scheme.
> >>>> 
> >>>> If we do nothing, QEMU stays broken. The monitor command device_del
> >>>> and others that need an ID will not work still. Hopefully any changes we
> >>>> make to QEMU will be robust enough stand the test of time.
> >>> 
> >>> That is not correct. It is possible for us to fix object_del / device_del
> >>> to accept the QOM object path. It isn't pretty but it is a solution that
> >>> gives everything a stable unique path ID to use for deletion even if the
> >>> user forgets to give a pretty path-less ID.
> >> 
> >> This QOM path might be better than nothing. Hopefully someone will make this
> >> patch and share it with us.
> > 
> > I sent a patch to support that, since it turned out to be pretty
> > trivial to implement. So that at least solves the immediate blocking
> > issue of deleting devices with an ID. The question of usability and
> > auto-generated IDs can continue in parallel....
> > 
> > Regards,
> > Daniel
> 
> I applied your patch, but saw this error message when I tried to 'make' QEMU:
> 
>   GEN   qmp-commands.txt
> line 344: syntax error: expected EQMP, found SQMP
> make: *** [qmp-commands.txt] Error 1
> make: *** Deleting file `qmp-commands.txt'
> 
> Know what it means?

It means I should have run 'make' again after adding the documentation
to detect the bug :-) I'll send another patch....

Regards,
Daniel
Programmingkid Aug. 27, 2015, 4:08 p.m. UTC | #61
On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote:
>> 
>>> On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
>>>> 
>>>>> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
>>>>>> 
>>>>>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
>>>>>> 
>>>>>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
>>>>>>> 
>>>>>>>>> If we did have auto-generated names, we would need to come up with a
>>>>>>>>> scheme that is not going to clash with any existing naming that users
>>>>>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>>>>>>>> Something as simple as what you suggest has non-trivial chance of
>>>>>>>>> clashing.
>>>>>>>> 
>>>>>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>>>>>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>>>>>>>> taken, just increment the ID until it is detected to be unique. The previous
>>>>>>>> threads on this subject has patches that did just that. This means that a
>>>>>>>> ID scheme that is just a single number would work without clashes. 
>>>>>>> 
>>>>>>> No, because you cannot predict what FUTURE names the user will request.
>>>>>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
>>>>>>> not just one that happens not to clash at the current moment.
>>>>>> 
>>>>>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
>>>>>> taken. I could just increment the ID myself until I find one that is unique. It is a
>>>>>> simple algorithm. Maybe you are talking about some program that has hard coded
>>>>>> ID's it depends on. If that is the case, perhaps this management program could be
>>>>>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
>>>>>> guaranteed to always be unique.
>>>>> 
>>>>> If breaking existing apps was OK, we would just mandate that users always
>>>>> provide an ID which trivially avoids the problem of not having an ID to
>>>>> use when deleting the object. We want to /avoid/ breaking existing apps
>>>>> though, so saying an app should change the way it works in order to cope
>>>>> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
>>>>> IDs, must use a separate namespace to avoid such clashes.
>>>> 
>>>> This is making the assumption that an auto-generated ID system would break any existing
>>>> application. We don't know this. In fact, I predict a future patch would allow existing
>>>> applications to continue running without modification. The patch would be a win-win
>>>> for both users and any management application.
>>>> 
>>> 
>>> Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
>>> that ID is already taken" will break existing applications.
>>> 
>>> But we are all striving to make your prediction true, with this very
>>> conversation.
>> 
>> Ok, it sounds like some are concerned that Libvirt would not be able to work with this
>> feature. Let me ask you this: does Libvirt interact with QEMU before the user has a
>> chance to do so? If the answer is yes, then that means Libvirt will have finished 
>> creating all its devices with their ID's long before the user has a chance to enter
>> his own devices.
> 
> Just to be clear - libvirt will *never* use an auto-generated device
> IDs feature. It is way more complicated to let QEMU assign device IDs
> and then auto-detect them from some 'info device-list' output, than
> to just specify IDs upfront at device/object creation time which
> it already does[1]. There is simply no benefit to auto-generating device
> IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
> IDs will only be of interest to humans talking to the monitor directly
> without a mgmt app involved.

I've haven't used Libvirt but I do believe in the saying "never say never". The
rest of what you said does sound accurate.
Eric Blake Aug. 27, 2015, 4:08 p.m. UTC | #62
On 08/27/2015 10:06 AM, Daniel P. Berrange wrote:

>>
>> I applied your patch, but saw this error message when I tried to 'make' QEMU:
>>
>>   GEN   qmp-commands.txt
>> line 344: syntax error: expected EQMP, found SQMP
>> make: *** [qmp-commands.txt] Error 1
>> make: *** Deleting file `qmp-commands.txt'
>>
>> Know what it means?
> 
> It means I should have run 'make' again after adding the documentation
> to detect the bug :-) I'll send another patch....

Generally, it's nicer to reply about problems with a patch to the patch
itself, rather than in an unrelated thread.  Makes it easier for
maintainers to see that a v2 of the patch will be needed.
Daniel P. Berrangé Aug. 27, 2015, 4:22 p.m. UTC | #63
On Thu, Aug 27, 2015 at 12:08:25PM -0400, Programmingkid wrote:
> 
> On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote:
> 
> > On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote:
> >> 
> >> On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote:
> >> 
> >>> On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
> >>>> 
> >>>> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
> >>>> 
> >>>>> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
> >>>>>> 
> >>>>>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
> >>>>>> 
> >>>>>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
> >>>>>>> 
> >>>>>>>>> If we did have auto-generated names, we would need to come up with a
> >>>>>>>>> scheme that is not going to clash with any existing naming that users
> >>>>>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
> >>>>>>>>> Something as simple as what you suggest has non-trivial chance of
> >>>>>>>>> clashing.
> >>>>>>>> 
> >>>>>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
> >>>>>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
> >>>>>>>> taken, just increment the ID until it is detected to be unique. The previous
> >>>>>>>> threads on this subject has patches that did just that. This means that a
> >>>>>>>> ID scheme that is just a single number would work without clashes. 
> >>>>>>> 
> >>>>>>> No, because you cannot predict what FUTURE names the user will request.
> >>>>>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
> >>>>>>> not just one that happens not to clash at the current moment.
> >>>>>> 
> >>>>>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
> >>>>>> taken. I could just increment the ID myself until I find one that is unique. It is a
> >>>>>> simple algorithm. Maybe you are talking about some program that has hard coded
> >>>>>> ID's it depends on. If that is the case, perhaps this management program could be
> >>>>>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
> >>>>>> guaranteed to always be unique.
> >>>>> 
> >>>>> If breaking existing apps was OK, we would just mandate that users always
> >>>>> provide an ID which trivially avoids the problem of not having an ID to
> >>>>> use when deleting the object. We want to /avoid/ breaking existing apps
> >>>>> though, so saying an app should change the way it works in order to cope
> >>>>> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
> >>>>> IDs, must use a separate namespace to avoid such clashes.
> >>>> 
> >>>> This is making the assumption that an auto-generated ID system would break any existing
> >>>> application. We don't know this. In fact, I predict a future patch would allow existing
> >>>> applications to continue running without modification. The patch would be a win-win
> >>>> for both users and any management application.
> >>>> 
> >>> 
> >>> Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
> >>> that ID is already taken" will break existing applications.
> >>> 
> >>> But we are all striving to make your prediction true, with this very
> >>> conversation.
> >> 
> >> Ok, it sounds like some are concerned that Libvirt would not be able to work with this
> >> feature. Let me ask you this: does Libvirt interact with QEMU before the user has a
> >> chance to do so? If the answer is yes, then that means Libvirt will have finished 
> >> creating all its devices with their ID's long before the user has a chance to enter
> >> his own devices.
> > 
> > Just to be clear - libvirt will *never* use an auto-generated device
> > IDs feature. It is way more complicated to let QEMU assign device IDs
> > and then auto-detect them from some 'info device-list' output, than
> > to just specify IDs upfront at device/object creation time which
> > it already does[1]. There is simply no benefit to auto-generating device
> > IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
> > IDs will only be of interest to humans talking to the monitor directly
> > without a mgmt app involved.
> 
> I've haven't used Libvirt but I do believe in the saying "never say never". The
> rest of what you said does sound accurate. 

I say that based on history of the development of libvirt. Many, many
years ago now, with QEMU 0.8.x, -device / device_add did not exist,
so you had to configure devices using args like -drive, or before
that, -hda, -hdb, etc. With that old syntax, QEMU would in fact
auto-generate a unique ID for each device. Libvirt then had to
figure out what that unique ID would be which was non-trivial to
get right, and risked changing with new QEMU releases. It also
did not cope well when hotplug was combined with migraiton, as
two guest machines with identical guest hardware could have
different assigned IDs depending on the sequence of hotplug/unplug
operations performed. All in all it was a world of hurt  and
we were very happy when -device came along and allowed libvirt
to specify the deivce IDs upfront itself. So yes, I am confident
we will not go back to letting QEMU auto-generate IDs in libvirt.

Regards,
Daniel
Programmingkid Aug. 27, 2015, 4:49 p.m. UTC | #64
On Aug 27, 2015, at 12:22 PM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 12:08:25PM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote:
>> 
>>> On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote:
>>>> 
>>>>> On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
>>>>>> 
>>>>>> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
>>>>>> 
>>>>>>> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
>>>>>>>> 
>>>>>>>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
>>>>>>>> 
>>>>>>>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
>>>>>>>>> 
>>>>>>>>>>> If we did have auto-generated names, we would need to come up with a
>>>>>>>>>>> scheme that is not going to clash with any existing naming that users
>>>>>>>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>>>>>>>>>> Something as simple as what you suggest has non-trivial chance of
>>>>>>>>>>> clashing.
>>>>>>>>>> 
>>>>>>>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>>>>>>>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>>>>>>>>>> taken, just increment the ID until it is detected to be unique. The previous
>>>>>>>>>> threads on this subject has patches that did just that. This means that a
>>>>>>>>>> ID scheme that is just a single number would work without clashes. 
>>>>>>>>> 
>>>>>>>>> No, because you cannot predict what FUTURE names the user will request.
>>>>>>>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
>>>>>>>>> not just one that happens not to clash at the current moment.
>>>>>>>> 
>>>>>>>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
>>>>>>>> taken. I could just increment the ID myself until I find one that is unique. It is a
>>>>>>>> simple algorithm. Maybe you are talking about some program that has hard coded
>>>>>>>> ID's it depends on. If that is the case, perhaps this management program could be
>>>>>>>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
>>>>>>>> guaranteed to always be unique.
>>>>>>> 
>>>>>>> If breaking existing apps was OK, we would just mandate that users always
>>>>>>> provide an ID which trivially avoids the problem of not having an ID to
>>>>>>> use when deleting the object. We want to /avoid/ breaking existing apps
>>>>>>> though, so saying an app should change the way it works in order to cope
>>>>>>> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
>>>>>>> IDs, must use a separate namespace to avoid such clashes.
>>>>>> 
>>>>>> This is making the assumption that an auto-generated ID system would break any existing
>>>>>> application. We don't know this. In fact, I predict a future patch would allow existing
>>>>>> applications to continue running without modification. The patch would be a win-win
>>>>>> for both users and any management application.
>>>>>> 
>>>>> 
>>>>> Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
>>>>> that ID is already taken" will break existing applications.
>>>>> 
>>>>> But we are all striving to make your prediction true, with this very
>>>>> conversation.
>>>> 
>>>> Ok, it sounds like some are concerned that Libvirt would not be able to work with this
>>>> feature. Let me ask you this: does Libvirt interact with QEMU before the user has a
>>>> chance to do so? If the answer is yes, then that means Libvirt will have finished 
>>>> creating all its devices with their ID's long before the user has a chance to enter
>>>> his own devices.
>>> 
>>> Just to be clear - libvirt will *never* use an auto-generated device
>>> IDs feature. It is way more complicated to let QEMU assign device IDs
>>> and then auto-detect them from some 'info device-list' output, than
>>> to just specify IDs upfront at device/object creation time which
>>> it already does[1]. There is simply no benefit to auto-generating device
>>> IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
>>> IDs will only be of interest to humans talking to the monitor directly
>>> without a mgmt app involved.
>> 
>> I've haven't used Libvirt but I do believe in the saying "never say never". The
>> rest of what you said does sound accurate. 
> 
> I say that based on history of the development of libvirt. Many, many
> years ago now, with QEMU 0.8.x, -device / device_add did not exist,
> so you had to configure devices using args like -drive, or before
> that, -hda, -hdb, etc. With that old syntax, QEMU would in fact
> auto-generate a unique ID for each device. Libvirt then had to
> figure out what that unique ID would be which was non-trivial to
> get right, and risked changing with new QEMU releases. It also
> did not cope well when hotplug was combined with migraiton, as
> two guest machines with identical guest hardware could have
> different assigned IDs depending on the sequence of hotplug/unplug
> operations performed. All in all it was a world of hurt  and
> we were very happy when -device came along and allowed libvirt
> to specify the deivce IDs upfront itself. So yes, I am confident
> we will not go back to letting QEMU auto-generate IDs in libvirt.

Thank you very much for this history. I didn't know about this.
John Snow Aug. 27, 2015, 6:59 p.m. UTC | #65
On 08/27/2015 09:00 AM, Eric Blake wrote:
> On 08/27/2015 06:32 AM, Jeff Cody wrote:
>> (Added Eric back in to the CC list.  Looks like he got dropped 
>> somewhere along the way)
> 
> No thanks to mailman's inept behavior that thinks that it is okay
> to rewrite cc's to drop anyone that doesn't want duplicate email.
> But don't worry about it; I have my local mail setup to flag any
> message in-reply-to an earlier one where I was in cc, precisely to
> work around mailman stupidly dropping me from cc. [Ideally, I'd
> filter the duplicate messages on my side, and turn off the broken
> mailman setting server-side, but I haven't yet figured out how to
> get filters working on my side that do that correctly.]  I'm hoping
> that mailman3 is not so inept, and that this list archives can
> migrate to hyperkitty/mailman3 in the not-too-distant future.
> 
> 
>> 
>> Do you disagree with the requirements I listed above?  If so, it
>> would be useful to begin the discussion around that.  For ease
>> of discussion, I'll list them again:
>> 
>> * Reserved namespaces * Uniqueness * Non-predictable (to avoid
>> inadvertently creating a de facto ABI)
> 
> Dan made the point that if a name is unpredictable, then we have
> to query to learn what name was assigned.  But if you add two or
> more devices before querying, then you don't know which device has
> which name.  Predictable might actually be better than
> non-predictable.
> 

Can we add the information into the QMP response?

> Better still might be fixing things to where we add a global
> command line option that outright fails any attempt to create an
> unnamed object. The option would be off by default for back-compat.
> But management apps like libvirt can turn it on once they are
> prepared to name every object they create (which in turn may imply
> fixing any remaining interfaces that cannot name an object to add
> in that ability for management to pass in a name).  Then there
> would be no unnamed objects, no ambiguity, and no need to generate
> names.
>
Jeff Cody Aug. 27, 2015, 7:08 p.m. UTC | #66
On Thu, Aug 27, 2015 at 05:02:17PM +0100, Daniel P. Berrange wrote:
> 
> Just to be clear - libvirt will *never* use an auto-generated device
> IDs feature. It is way more complicated to let QEMU assign device IDs
> and then auto-detect them from some 'info device-list' output, than
> to just specify IDs upfront at device/object creation time which
> it already does[1]. There is simply no benefit to auto-generating device
> IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
> IDs will only be of interest to humans talking to the monitor directly
> without a mgmt app involved.
> 
> Regards,
> Daniel
> 
> [1] we don't provide IDs for qcow2 image backing file chain, but that's
>     part of a bigger story that's being dealt with in this area.

This is the part that interests me the most :)

Do you think in dealing with image backing file chains, libvirt would
ever make use of QEMU auto-generated node-names (either in the
current feature set, or with future features)?  I'm not sure if your
above statement is specific to device ID, or extends to node-names as
well.


Thanks,
Jeff
Eric Blake Aug. 27, 2015, 7:20 p.m. UTC | #67
On 08/27/2015 12:59 PM, John Snow wrote:

>> Dan made the point that if a name is unpredictable, then we have
>> to query to learn what name was assigned.  But if you add two or
>> more devices before querying, then you don't know which device has
>> which name.  Predictable might actually be better than
>> non-predictable.
>>
> 
> Can we add the information into the QMP response?

Yes, any command that currently returns nothing (aka {}) can be expanded
to return something useful, such as the name of the device just
allocated, without breaking back-compat.  That would be one less round
trip (create a device, and the response is the devices name; rather than
create a device and pass a second command to query its name).  But
remember that even things as trivial as creating a disk device may open
several BDS and therefore create several named items at once, so we have
to be careful that all the information can be parsed.

On the bright side, Markus' introspection work would let us know if a
command's return struct supplies the additional information.  On the
downside, management apps that want to talk to older qemu probably
wouldn't use it anyways, as they have to have the fallback of supplying
a name in the first place for older qemu, at which point supplying the
name always is easier than making code conditional on whether qemu is
new enough to introspect.
Eric Blake Aug. 27, 2015, 7:27 p.m. UTC | #68
On 08/27/2015 01:08 PM, Jeff Cody wrote:
> On Thu, Aug 27, 2015 at 05:02:17PM +0100, Daniel P. Berrange wrote:
>>
>> Just to be clear - libvirt will *never* use an auto-generated device
>> IDs feature. It is way more complicated to let QEMU assign device IDs
>> and then auto-detect them from some 'info device-list' output, than
>> to just specify IDs upfront at device/object creation time which
>> it already does[1]. There is simply no benefit to auto-generating device
>> IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
>> IDs will only be of interest to humans talking to the monitor directly
>> without a mgmt app involved.
>>
>> Regards,
>> Daniel
>>
>> [1] we don't provide IDs for qcow2 image backing file chain, but that's
>>     part of a bigger story that's being dealt with in this area.
> 
> This is the part that interests me the most :)
> 
> Do you think in dealing with image backing file chains, libvirt would
> ever make use of QEMU auto-generated node-names (either in the
> current feature set, or with future features)?  I'm not sure if your
> above statement is specific to device ID, or extends to node-names as
> well.

I have a patch series that I posted for libvirt that used a hack to try
and take advantage of auto-generated names (basically, you can't use
allocation watermark events on qcow2-over-block-devices without proper
node names).  Had we turned on auto node names in 2.4 (the release that
added the event), then it might be in libvirt now.  But now that we have
to support 2.4 without auto node names, it's in libvirt's long-term
interest to avoid technical debt and directly supply node names for all
BDS, rather than relying on node names, and rather than cheating by
waiting for qemu 2.5.

So my current task in libvirt is to try and fix things to supply node
names everywhere, then rewrite my allocation watermark event series atop
that change, at which point relying on auto names will not be necessary
for libvirt.
Programmingkid Aug. 27, 2015, 8:15 p.m. UTC | #69
On Aug 27, 2015, at 12:22 PM, Daniel P. Berrange wrote:

> On Thu, Aug 27, 2015 at 12:08:25PM -0400, Programmingkid wrote:
>> 
>> On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote:
>> 
>>> On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote:
>>>> 
>>>> On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote:
>>>> 
>>>>> On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote:
>>>>>> 
>>>>>> On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote:
>>>>>> 
>>>>>>> On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote:
>>>>>>>> 
>>>>>>>> On Aug 27, 2015, at 10:02 AM, Eric Blake wrote:
>>>>>>>> 
>>>>>>>>> On 08/27/2015 07:56 AM, Programmingkid wrote:
>>>>>>>>> 
>>>>>>>>>>> If we did have auto-generated names, we would need to come up with a
>>>>>>>>>>> scheme that is not going to clash with any existing naming that users
>>>>>>>>>>> of QEMU may already be doing, otherwise we risk causing a regression.
>>>>>>>>>>> Something as simple as what you suggest has non-trivial chance of
>>>>>>>>>>> clashing.
>>>>>>>>>> 
>>>>>>>>>> Actually there is a way to prevent clashing. When QEMU auto-generates a
>>>>>>>>>> name, it could scan all the ID's to see if there is a clash. If the ID is already
>>>>>>>>>> taken, just increment the ID until it is detected to be unique. The previous
>>>>>>>>>> threads on this subject has patches that did just that. This means that a
>>>>>>>>>> ID scheme that is just a single number would work without clashes. 
>>>>>>>>> 
>>>>>>>>> No, because you cannot predict what FUTURE names the user will request.
>>>>>>>>> The name generated by qemu must be IMPOSSIBLE to request manually, and
>>>>>>>>> not just one that happens not to clash at the current moment.
>>>>>>>> 
>>>>>>>> If I add a device with an ID that is taken, QEMU can just say sorry that ID is already
>>>>>>>> taken. I could just increment the ID myself until I find one that is unique. It is a
>>>>>>>> simple algorithm. Maybe you are talking about some program that has hard coded
>>>>>>>> ID's it depends on. If that is the case, perhaps this management program could be
>>>>>>>> made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually
>>>>>>>> guaranteed to always be unique.
>>>>>>> 
>>>>>>> If breaking existing apps was OK, we would just mandate that users always
>>>>>>> provide an ID which trivially avoids the problem of not having an ID to
>>>>>>> use when deleting the object. We want to /avoid/ breaking existing apps
>>>>>>> though, so saying an app should change the way it works in order to cope
>>>>>>> with QEMU's ID generation is not appropriate. Hence any use of auto-generated
>>>>>>> IDs, must use a separate namespace to avoid such clashes.
>>>>>> 
>>>>>> This is making the assumption that an auto-generated ID system would break any existing
>>>>>> application. We don't know this. In fact, I predict a future patch would allow existing
>>>>>> applications to continue running without modification. The patch would be a win-win
>>>>>> for both users and any management application.
>>>>>> 
>>>>> 
>>>>> Daniel's assumption is spot on.  The idea of "QEMU can just say sorry
>>>>> that ID is already taken" will break existing applications.
>>>>> 
>>>>> But we are all striving to make your prediction true, with this very
>>>>> conversation.
>>>> 
>>>> Ok, it sounds like some are concerned that Libvirt would not be able to work with this
>>>> feature. Let me ask you this: does Libvirt interact with QEMU before the user has a
>>>> chance to do so? If the answer is yes, then that means Libvirt will have finished 
>>>> creating all its devices with their ID's long before the user has a chance to enter
>>>> his own devices.
>>> 
>>> Just to be clear - libvirt will *never* use an auto-generated device
>>> IDs feature. It is way more complicated to let QEMU assign device IDs
>>> and then auto-detect them from some 'info device-list' output, than
>>> to just specify IDs upfront at device/object creation time which
>>> it already does[1]. There is simply no benefit to auto-generating device
>>> IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
>>> IDs will only be of interest to humans talking to the monitor directly
>>> without a mgmt app involved.
>> 
>> I've haven't used Libvirt but I do believe in the saying "never say never". The
>> rest of what you said does sound accurate. 
> 
> I say that based on history of the development of libvirt. Many, many
> years ago now, with QEMU 0.8.x, -device / device_add did not exist,
> so you had to configure devices using args like -drive, or before
> that, -hda, -hdb, etc. With that old syntax, QEMU would in fact
> auto-generate a unique ID for each device. Libvirt then had to
> figure out what that unique ID would be which was non-trivial to
> get right, and risked changing with new QEMU releases. It also
> did not cope well when hotplug was combined with migraiton, as
> two guest machines with identical guest hardware could have
> different assigned IDs depending on the sequence of hotplug/unplug
> operations performed. All in all it was a world of hurt  and
> we were very happy when -device came along and allowed libvirt
> to specify the deivce IDs upfront itself. So yes, I am confident
> we will not go back to letting QEMU auto-generate IDs in libvirt.

Your patch does make device_del work, but still doesn't solve the 
auto-generated ID's issue. 

I have the feeling you think an auto-generated ID system will be in
Libvirt's way. It won't. Libvirt can specify its own ID's as much as it
wants. The auto-generated ID system would only work when no ID is
specified. So you have nothing to worry about. When Libvirt starts up
QEMU, it can specify all the ID's it wants without problem. This feature
only works if the user forgot to give an ID. Libvirt and the user can be
happy with this feature.
Jeff Cody Aug. 27, 2015, 8:37 p.m. UTC | #70
On Thu, Aug 27, 2015 at 01:27:40PM -0600, Eric Blake wrote:
> On 08/27/2015 01:08 PM, Jeff Cody wrote:
> > On Thu, Aug 27, 2015 at 05:02:17PM +0100, Daniel P. Berrange wrote:
> >>
> >> Just to be clear - libvirt will *never* use an auto-generated device
> >> IDs feature. It is way more complicated to let QEMU assign device IDs
> >> and then auto-detect them from some 'info device-list' output, than
> >> to just specify IDs upfront at device/object creation time which
> >> it already does[1]. There is simply no benefit to auto-generating device
> >> IDs for a mgmt app like libvirt, and plenty of downside.  Auto-generated
> >> IDs will only be of interest to humans talking to the monitor directly
> >> without a mgmt app involved.
> >>
> >> Regards,
> >> Daniel
> >>
> >> [1] we don't provide IDs for qcow2 image backing file chain, but that's
> >>     part of a bigger story that's being dealt with in this area.
> > 
> > This is the part that interests me the most :)
> > 
> > Do you think in dealing with image backing file chains, libvirt would
> > ever make use of QEMU auto-generated node-names (either in the
> > current feature set, or with future features)?  I'm not sure if your
> > above statement is specific to device ID, or extends to node-names as
> > well.
> 
> I have a patch series that I posted for libvirt that used a hack to try
> and take advantage of auto-generated names (basically, you can't use
> allocation watermark events on qcow2-over-block-devices without proper
> node names).  Had we turned on auto node names in 2.4 (the release that
> added the event), then it might be in libvirt now.  But now that we have
> to support 2.4 without auto node names, it's in libvirt's long-term
> interest to avoid technical debt and directly supply node names for all
> BDS, rather than relying on node names, and rather than cheating by
> waiting for qemu 2.5.
> 
> So my current task in libvirt is to try and fix things to supply node
> names everywhere, then rewrite my allocation watermark event series atop
> that change, at which point relying on auto names will not be necessary
> for libvirt.
> 

OK, thanks. That certainly makes it less urgent, at least on my end.
Seems like the only time we'd need it for libvirt is if in the future
we created BDSs as a byproduct of an operation, and wanted to have a
node-name so that we could present it to libvirt afterwards.

Thanks,
Jeff
Kevin Wolf Sept. 1, 2015, 12:34 p.m. UTC | #71
Am 27.08.2015 um 14:32 hat Jeff Cody geschrieben:
> I'm not married to the ID generation scheme I proposed.  
> 
> What I am trying to do, however, is have a technical discussion on
> generating an ID in a well-formed manner.  And hopefully, in a way
> that is useful to all interested subsystems, if possible.
> 
> Do you disagree with the requirements I listed above?  If so, it would
> be useful to begin the discussion around that.  For ease of
> discussion, I'll list them again:
> 
> * Reserved namespaces
> * Uniqueness
> * Non-predictable (to avoid inadvertently creating a de facto ABI)
> 
> 
> . . .
> 
> On the generation scheme proposed above:
> 
> I understand that something you desire is an ID that is easier to
> type.
> 
> If we wanted to make it shorter, perhaps we could have the number
> counter be variable length:
> 
>             qemu#ss#D#XY
>               |   | | |
> qemu reserved -   | | |
>                   | | |
> subsystem name ---| | |
>                     | |
>     counter --------| |
>                       |
>     2-digit random ---|

Even with keeping all of the information in there we can shorten the ID
a bit more: # at the start is enough to mark it as autogenerated, the
subsystem seems nice to have in there anyway, and the # separators can
be removed without making the ID less unique (assuming that subsystems
never end in a digit). This results in an ID that looks like a three (or
more) digit number for the subsystem, where the last two digits are
random, like this:

    #block150
    #block219
    #block344
    ...

That seems easy to type and still fulfills all of the criteria.

Kevin
Programmingkid Sept. 1, 2015, 2:18 p.m. UTC | #72
On Sep 1, 2015, at 8:34 AM, Kevin Wolf wrote:

> Am 27.08.2015 um 14:32 hat Jeff Cody geschrieben:
>> I'm not married to the ID generation scheme I proposed.  
>> 
>> What I am trying to do, however, is have a technical discussion on
>> generating an ID in a well-formed manner.  And hopefully, in a way
>> that is useful to all interested subsystems, if possible.
>> 
>> Do you disagree with the requirements I listed above?  If so, it would
>> be useful to begin the discussion around that.  For ease of
>> discussion, I'll list them again:
>> 
>> * Reserved namespaces
>> * Uniqueness
>> * Non-predictable (to avoid inadvertently creating a de facto ABI)
>> 
>> 
>> . . .
>> 
>> On the generation scheme proposed above:
>> 
>> I understand that something you desire is an ID that is easier to
>> type.
>> 
>> If we wanted to make it shorter, perhaps we could have the number
>> counter be variable length:
>> 
>>            qemu#ss#D#XY
>>              |   | | |
>> qemu reserved -   | | |
>>                  | | |
>> subsystem name ---| | |
>>                    | |
>>    counter --------| |
>>                      |
>>    2-digit random ---|
> 
> Even with keeping all of the information in there we can shorten the ID
> a bit more: # at the start is enough to mark it as autogenerated, the
> subsystem seems nice to have in there anyway, and the # separators can
> be removed without making the ID less unique (assuming that subsystems
> never end in a digit). This results in an ID that looks like a three (or
> more) digit number for the subsystem, where the last two digits are
> random, like this:
> 
>    #block150
>    #block219
>    #block344
>    ...
> 
> That seems easy to type and still fulfills all of the criteria.
> 
> Kevin

I do know that some really want an indicator that shows that an ID is auto-generated. But we could still do this and keep the ID short. What if the auto-generated ID just started with a character the user could never use at the beginning of the ID. I suggest we use an underscore to indicate machine-generated ID's. Something like this _1. It is very simple and effective.
Kevin Wolf Sept. 1, 2015, 2:43 p.m. UTC | #73
Am 01.09.2015 um 16:18 hat Programmingkid geschrieben:
> 
> On Sep 1, 2015, at 8:34 AM, Kevin Wolf wrote:
> 
> > Am 27.08.2015 um 14:32 hat Jeff Cody geschrieben:
> >> I'm not married to the ID generation scheme I proposed.  
> >> 
> >> What I am trying to do, however, is have a technical discussion on
> >> generating an ID in a well-formed manner.  And hopefully, in a way
> >> that is useful to all interested subsystems, if possible.
> >> 
> >> Do you disagree with the requirements I listed above?  If so, it would
> >> be useful to begin the discussion around that.  For ease of
> >> discussion, I'll list them again:
> >> 
> >> * Reserved namespaces
> >> * Uniqueness
> >> * Non-predictable (to avoid inadvertently creating a de facto ABI)
> >> 
> >> 
> >> . . .
> >> 
> >> On the generation scheme proposed above:
> >> 
> >> I understand that something you desire is an ID that is easier to
> >> type.
> >> 
> >> If we wanted to make it shorter, perhaps we could have the number
> >> counter be variable length:
> >> 
> >>            qemu#ss#D#XY
> >>              |   | | |
> >> qemu reserved -   | | |
> >>                  | | |
> >> subsystem name ---| | |
> >>                    | |
> >>    counter --------| |
> >>                      |
> >>    2-digit random ---|
> > 
> > Even with keeping all of the information in there we can shorten the ID
> > a bit more: # at the start is enough to mark it as autogenerated, the
> > subsystem seems nice to have in there anyway, and the # separators can
> > be removed without making the ID less unique (assuming that subsystems
> > never end in a digit). This results in an ID that looks like a three (or
> > more) digit number for the subsystem, where the last two digits are
> > random, like this:
> > 
> >    #block150
> >    #block219
> >    #block344
> >    ...
> > 
> > That seems easy to type and still fulfills all of the criteria.
> > 
> > Kevin
> 
> I do know that some really want an indicator that shows that an ID is auto-generated. But we could still do this and keep the ID short. What if the auto-generated ID just started with a character the user could never use at the beginning of the ID. I suggest we use an underscore to indicate machine-generated ID's. Something like this _1. It is very simple and effective. 

That's what I already did. # is a reserved character. I don't think
something like #block150 is unreasonably long, it's a bit more
descriptive than _1, and it fulfills all of Jeff's criteria, which _1
clearly doesn't.

Kevin
Programmingkid Sept. 1, 2015, 3:55 p.m. UTC | #74
On Sep 1, 2015, at 10:43 AM, Kevin Wolf wrote:

> Am 01.09.2015 um 16:18 hat Programmingkid geschrieben:
>> 
>> On Sep 1, 2015, at 8:34 AM, Kevin Wolf wrote:
>> 
>>> Am 27.08.2015 um 14:32 hat Jeff Cody geschrieben:
>>>> I'm not married to the ID generation scheme I proposed.  
>>>> 
>>>> What I am trying to do, however, is have a technical discussion on
>>>> generating an ID in a well-formed manner.  And hopefully, in a way
>>>> that is useful to all interested subsystems, if possible.
>>>> 
>>>> Do you disagree with the requirements I listed above?  If so, it would
>>>> be useful to begin the discussion around that.  For ease of
>>>> discussion, I'll list them again:
>>>> 
>>>> * Reserved namespaces
>>>> * Uniqueness
>>>> * Non-predictable (to avoid inadvertently creating a de facto ABI)
>>>> 
>>>> 
>>>> . . .
>>>> 
>>>> On the generation scheme proposed above:
>>>> 
>>>> I understand that something you desire is an ID that is easier to
>>>> type.
>>>> 
>>>> If we wanted to make it shorter, perhaps we could have the number
>>>> counter be variable length:
>>>> 
>>>>           qemu#ss#D#XY
>>>>             |   | | |
>>>> qemu reserved -   | | |
>>>>                 | | |
>>>> subsystem name ---| | |
>>>>                   | |
>>>>   counter --------| |
>>>>                     |
>>>>   2-digit random ---|
>>> 
>>> Even with keeping all of the information in there we can shorten the ID
>>> a bit more: # at the start is enough to mark it as autogenerated, the
>>> subsystem seems nice to have in there anyway, and the # separators can
>>> be removed without making the ID less unique (assuming that subsystems
>>> never end in a digit). This results in an ID that looks like a three (or
>>> more) digit number for the subsystem, where the last two digits are
>>> random, like this:
>>> 
>>>   #block150
>>>   #block219
>>>   #block344
>>>   ...
>>> 
>>> That seems easy to type and still fulfills all of the criteria.
>>> 
>>> Kevin
>> 
>> I do know that some really want an indicator that shows that an ID is auto-generated. But we could still do this and keep the ID short. What if the auto-generated ID just started with a character the user could never use at the beginning of the ID. I suggest we use an underscore to indicate machine-generated ID's. Something like this _1. It is very simple and effective. 
> 
> That's what I already did. # is a reserved character. I don't think
> something like #block150 is unreasonably long, it's a bit more
> descriptive than _1, and it fulfills all of Jeff's criteria, which _1
> clearly doesn't.

The rules aren't all necessary.
Programmingkid Sept. 3, 2015, 2:34 p.m. UTC | #75
It has been over a week since we first started talking about this subject. A lot of opinions have been flying around. Does this issue look like it is starting to be fixed? I just have to say I don't think it has been solved yet. Having device_del use a QOM path does not sound very good. It is actually easier and faster for the user to restart QEMU with a new ID for some device than to have to look up some QOM path. Having a user-friendly system for creating ID's should be a priority.
Jeff Cody Sept. 3, 2015, 2:43 p.m. UTC | #76
On Thu, Sep 03, 2015 at 10:34:04AM -0400, Programmingkid wrote:
> It has been over a week since we first started talking about this
> subject. A lot of opinions have been flying around. Does this issue
> look like it is starting to be fixed? I just have to say I don't
> think it has been solved yet. Having device_del use a QOM path does
> not sound very good. It is actually easier and faster for the user
> to restart QEMU with a new ID for some device than to have to look
> up some QOM path. Having a user-friendly system for creating ID's
> should be a priority. 

Please see my v2 patch series (you were CC'ed on it):


    [PATCH v2 1/2] util - add automated ID generation utility
    [PATCH v2 2/2] block: auto-generated node-names

The first patch is a qemu-wide utility to generate IDs.

The scheme seems to have some consensus, and a few r-b's (it is
also derived from this conversation thread).

The second patch is using it in the block layer, for node-names; it
should be trivial to see how to use it for qdev.

Would you like me to roll a v3, with a qdev patch added in?

Thanks,
Jeff
Programmingkid Sept. 3, 2015, 3:55 p.m. UTC | #77
On Sep 3, 2015, at 10:43 AM, Jeff Cody wrote:

> On Thu, Sep 03, 2015 at 10:34:04AM -0400, Programmingkid wrote:
>> It has been over a week since we first started talking about this
>> subject. A lot of opinions have been flying around. Does this issue
>> look like it is starting to be fixed? I just have to say I don't
>> think it has been solved yet. Having device_del use a QOM path does
>> not sound very good. It is actually easier and faster for the user
>> to restart QEMU with a new ID for some device than to have to look
>> up some QOM path. Having a user-friendly system for creating ID's
>> should be a priority. 
> 
> Please see my v2 patch series (you were CC'ed on it):
> 
> 
>    [PATCH v2 1/2] util - add automated ID generation utility
>    [PATCH v2 2/2] block: auto-generated node-names
> 
> The first patch is a qemu-wide utility to generate IDs.
> 
> The scheme seems to have some consensus, and a few r-b's (it is
> also derived from this conversation thread).
> 
> The second patch is using it in the block layer, for node-names; it
> should be trivial to see how to use it for qdev.
> 
> Would you like me to roll a v3, with a qdev patch added in?
> 
> Thanks,
> Jeff

Yes, thank you for them. The first patch does provide a function that generates an ID, 
but it doesn't actually give the ID to anything. A third patch might be needed
that actually puts the id_generate() function to use. I use USB devices that 
I would like to be able to remove during QEMU's usage. Any ID generating
system would be very useful. In qdev-monitor.c there is a function called
qdev_device_add(). That is where I would use your id_generate() function.
diff mbox

Patch

diff --git a/qdev-monitor.c b/qdev-monitor.c
index f9e2d62..98267c4 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -26,6 +26,10 @@ 
 #include "qapi/qmp/qerror.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include <math.h>
+
+/* USB's max number of devices is 127. This number is 3 digits long. */
+#define MAX_NUM_DIGITS_FOR_USB_ID 3
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -574,17 +578,25 @@  DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
     id = qemu_opts_id(opts);
     if (id) {
         dev->id = id;
+    } else { /* create an id for a device if none is provided */
+        static int device_id_count;
+
+        /* Add one for '\0' character */
+        char *device_id = (char *) malloc(sizeof(char) *
+                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
+        sprintf(device_id, "%d", device_id_count++);
+        dev->id = (const char *) device_id;
+
+        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
+        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
+            printf("Warning: Maximum number of device ID's generated!\n\a");
+            printf("Time for you to make your own device ID's.\n");
+        }
     }
 
     if (dev->id) {
         object_property_add_child(qdev_get_peripheral(), dev->id,
                                   OBJECT(dev), NULL);
-    } else {
-        static int anon_count;
-        gchar *name = g_strdup_printf("device[%d]", anon_count++);
-        object_property_add_child(qdev_get_peripheral_anon(), name,
-                                  OBJECT(dev), NULL);
-        g_free(name);
     }
 
     /* set properties */