[libvirt] [PATCH v1 4/7] qemu: Allow regeneration of aliases

[Michal Privoznik]

On 09/21/2017 07:18 PM, Daniel P. Berrange wrote:
> On Thu, Sep 21, 2017 at 06:05:19PM +0200, Peter Krempa wrote:
>> On Thu, Sep 21, 2017 at 16:47:08 +0200, Michal Privoznik wrote:
>>> In the near future the qemuAssignDeviceAliases() function is
>>> going to be called multiple times: once at the domain define
>>> time, then in domain prepare phase. To avoid regenerating the
>>> same aliases the second time we need to be able to tell the
>>> function which time it is being called.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/qemu/qemu_alias.c   | 113 ++++++++++++++++++++++++++++++++++++++++++++++--
>>>  src/qemu/qemu_alias.h   |   4 +-
>>>  src/qemu/qemu_driver.c  |   2 +-
>>>  src/qemu/qemu_process.c |   2 +-
>>>  tests/qemuhotplugtest.c |   2 +-
>>>  5 files changed, 115 insertions(+), 8 deletions(-)
>>>

>> So if you want to make aliases persistent, at least this will not work
>> properly with device coldplug.

Ah, good point.

>>
>> If you have two devices, detach the first one, then the second one moves
>> to index 0 but will still have alias ending with 1. Then if you cold-add
>> another device that will be put into index 1, and when starting the VM
>> it will get assigned the same alias as the one which has the old one.
>>
>> This applies to all devices where the alias depends on the ordering.
> 
> Yep, we would need to maintain a  hash table remembering all currently
> assigned aliases, and then increment the counter until we find a free
> one for that dev type.

Alternatively, every time we want to assign an alias for a device we
traverse its siblings and see if it's taken.

for (i = 0; ; i++) {
    alias = "device$i";
    for (j = 0; j < def->ndevice; j++) {
        if (STREQ(def->device[j]->info.alias, alias))
            continue;
        break
    }
    if (j != def->ndevice) {
        /* alias is free */
        device->alias = alias;
        break;
    }
}

This is roughly the same as your approach except the hash table is taken
out. Which I find better because:

1) the hash table would have to live under qemuDomainObjPrivatePtr (or
under virDomainObjPtr) and I'd have to pass pointer to it all the way
down to virDomainDeviceInfoClear()  -> huge change

2) we would have two copies of aliases in memory.

IOW, my approach is more time complex but less memory complex.

Michal