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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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(-)
> > 
> > diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> > index e1431e2a2..00868c50f 100644
> > --- a/src/qemu/qemu_alias.c
> > +++ b/src/qemu/qemu_alias.c
> > @@ -481,25 +481,99 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def,
> >  }
> >  
> >  
> > +/*
> > + * qemuAssignDeviceAliases:
> > + * @def: domain definition
> > + * @qemuCaps: qemu capabilities
> > + * @regenerate: forcibly regenerate aliases
> > + *
> > + * Assign aliases to domain devices. If @regenerate is false only
> > + * the missing aliases are generated leaving already assigned
> > + * aliases untouched. If @regenerate is true any preexisting
> > + * aliases are overwritten.
> > + *
> > + * Returns 0 on success, -1 otherwise.
> > + */
> >  int
> > -qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> > +qemuAssignDeviceAliases(virDomainDefPtr def,
> > +                        virQEMUCapsPtr qemuCaps,
> > +                        bool regenerate)
> >  {
> >      size_t i;
> >  
> > +    if (regenerate) {
> > +        /* If we need to regenerate the aliases, we have to free
> > +         * them all upfront because there are some dependencies,
> > +         * e.g. an interface can be a hostdev and so on.
> > +         * Therefore, if we'd do the freeing in the loop below we
> > +         * might not start from zero. */
> > +        for (i = 0; i < def->ndisks; i++)
> > +            VIR_FREE(def->disks[i]->info.alias);
> > +        for (i = 0; i < def->nnets; i++)
> > +            VIR_FREE(def->nets[i]->info.alias);
> > +        for (i = 0; i < def->nfss; i++)
> > +            VIR_FREE(def->fss[i]->info.alias);
> > +        for (i = 0; i < def->nsounds; i++)
> > +            VIR_FREE(def->sounds[i]->info.alias);
> > +        for (i = 0; i < def->nhostdevs; i++)
> > +            VIR_FREE(def->hostdevs[i]->info->alias);
> > +        for (i = 0; i < def->nredirdevs; i++)
> > +            VIR_FREE(def->redirdevs[i]->info.alias);
> > +        for (i = 0; i < def->nvideos; i++)
> > +            VIR_FREE(def->videos[i]->info.alias);
> > +        for (i = 0; i < def->ncontrollers; i++)
> > +            VIR_FREE(def->controllers[i]->info.alias);
> > +        for (i = 0; i < def->ninputs; i++)
> > +            VIR_FREE(def->inputs[i]->info.alias);
> > +        for (i = 0; i < def->nparallels; i++)
> > +            VIR_FREE(def->parallels[i]->info.alias);
> > +        for (i = 0; i < def->nserials; i++)
> > +            VIR_FREE(def->serials[i]->info.alias);
> > +        for (i = 0; i < def->nchannels; i++)
> > +            VIR_FREE(def->channels[i]->info.alias);
> > +        for (i = 0; i < def->nconsoles; i++)
> > +            VIR_FREE(def->consoles[i]->info.alias);
> > +        for (i = 0; i < def->nhubs; i++)
> > +            VIR_FREE(def->hubs[i]->info.alias);
> > +        for (i = 0; i < def->nshmems; i++)
> > +            VIR_FREE(def->shmems[i]->info.alias);
> > +        for (i = 0; i < def->nsmartcards; i++)
> > +            VIR_FREE(def->smartcards[i]->info.alias);
> > +        if (def->watchdog)
> > +            VIR_FREE(def->watchdog->info.alias);
> > +        if (def->memballoon)
> > +            VIR_FREE(def->memballoon->info.alias);
> > +        for (i = 0; i < def->nrngs; i++)
> > +            VIR_FREE(def->rngs[i]->info.alias);
> > +        if (def->tpm)
> > +            VIR_FREE(def->tpm->info.alias);
> > +        for (i = 0; i < def->nmems; i++)
> > +            VIR_FREE(def->mems[i]->info.alias);
> > +    }
> > +
> > +
> >      for (i = 0; i < def->ndisks; i++) {
> > +        if (def->disks[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nnets; i++) {
> > +        if (def->nets[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceNetAlias(def, def->nets[i], -1) < 0)
> >              return -1;
> >      }
> >  
> >      for (i = 0; i < def->nfss; i++) {
> > +        if (def->fss[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nsounds; i++) {
> > +        if (def->sounds[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0)
> >              return -1;
> >      }
> > @@ -509,71 +583,102 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> >           * linked to a NetDef, they will share an info and the alias
> >           * will already be set, so don't try to set it again.
> >           */
> > +        if (def->hostdevs[i]->info->alias && !regenerate)
> > +            continue;
> >          if (!def->hostdevs[i]->info->alias &&
> >              qemuAssignDeviceHostdevAlias(def, &def->hostdevs[i]->info->alias, -1) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nredirdevs; i++) {
> > +        if (def->redirdevs[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nvideos; i++) {
> > +        if (def->videos[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->ncontrollers; i++) {
> > +        if (def->controllers[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceControllerAlias(def, qemuCaps, def->controllers[i]) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->ninputs; i++) {
> > +        if (def->inputs[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nparallels; i++) {
> > +        if (def->parallels[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nserials; i++) {
> > +        if (def->serials[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nchannels; i++) {
> > +        if (def->channels[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nconsoles; i++) {
> > +        if (def->consoles[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nhubs; i++) {
> > +        if (def->hubs[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nshmems; i++) {
> > +        if (def->shmems[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nsmartcards; i++) {
> > +        if (def->smartcards[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0)
> >              return -1;
> >      }
> > -    if (def->watchdog) {
> > +    if (def->watchdog &&
> > +        (!def->watchdog->info.alias || regenerate)) {
> >          if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0)
> >              return -1;
> >      }
> > -    if (def->memballoon) {
> > +    if (def->memballoon &&
> > +        (!def->memballoon->info.alias || regenerate)) {
> >          if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0)
> >              return -1;
> >      }
> >      for (i = 0; i < def->nrngs; i++) {
> > +        if (def->rngs[i]->info.alias && !regenerate)
> > +            continue;
> >          if (qemuAssignDeviceRNGAlias(def, def->rngs[i], i) < 0)
> >              return -1;
> >      }
> 
> So if you want to make aliases persistent, at least this will not work
> properly with device coldplug.
> 
> 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.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux