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&gt; >>> --- >>> 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.