Re: [libvirt] [PATCH v1 4/7] qemu: Allow regeneration of aliases
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(a)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