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@xxxxxxxxxx>
>>> ---
>>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
