Michal Privoznik <mprivozn@xxxxxxxxxx> [2017-09-21, 04:47PM +0200]:
> 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)) {
Can you make this conditional consistent to the others?
> if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0)
> return -1;
> }
> - if (def->memballoon) {
> + if (def->memballoon &&
> + (!def->memballoon->info.alias || regenerate)) {
And this.
> 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;
> }
> - if (def->tpm) {
> + if (def->tpm &&
> + (!def->tpm->info.alias || regenerate)) {
This too.
> if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0)
> return -1;
> }
> for (i = 0; i < def->nmems; i++) {
> + if (def->mems[i]->info.alias && !regenerate)
> + continue;
> if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0)
> return -1;
> }
> diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
> index 860ab6c0c..e33a05580 100644
> --- a/src/qemu/qemu_alias.h
> +++ b/src/qemu/qemu_alias.h
> @@ -66,7 +66,9 @@ int qemuAssignDeviceShmemAlias(virDomainDefPtr def,
> virDomainShmemDefPtr shmem,
> int idx);
>
> -int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
> +int qemuAssignDeviceAliases(virDomainDefPtr def,
> + virQEMUCapsPtr qemuCaps,
> + bool regenerate);
>
> int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info,
> const char *prefix);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 1b271792d..64ba7d454 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -16145,7 +16145,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
> def->emulator)))
> goto cleanup;
>
> - if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
> + if (qemuAssignDeviceAliases(def, qemuCaps, true) < 0)
> goto cleanup;
>
> if (!(vm = virDomainObjListAdd(driver->domains, def,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e6cc41e13..448de679c 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5340,7 +5340,7 @@ qemuProcessPrepareDomain(virConnectPtr conn,
> goto cleanup;
> }
>
> - if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0)
> + if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps, false) < 0)
> goto cleanup;
>
> VIR_DEBUG("Setting graphics devices");
> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
> index 23a498617..0b1b00176 100644
> --- a/tests/qemuhotplugtest.c
> +++ b/tests/qemuhotplugtest.c
> @@ -95,7 +95,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
> goto cleanup;
> }
>
> - if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0)
> + if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps, true) < 0)
Instead of passing a state flag into the function, would it be possible
to manually clear the aliases before calling qemuAssignDeviceAliases and
dropping the regenerate flag? I find boolean parameters that
fundamentally change the behaviour of a function hard to follow at a
certain time.
> goto cleanup;
>
> (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
> --
> 2.13.5
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
IBM Systems
Linux on z Systems & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bwalk@xxxxxxxxxx
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
