[libvirt] [PATCH v2 0/2] Fix backwards migration of pSeries guests

Changes from [v1]: * don't hide the controller model check inside a function and propely explain the reason not to format it; * rename a confusingly named function. [v1] https://www.redhat.com/archives/libvir-list/2017-July/msg00705.html Andrea Bolognani (2): conf: Fix backwards migration of pSeries guests conf: Rename virDomainControllerIsPCIHostBridge() to IsPSeriesPHB() src/conf/domain_conf.c | 29 ++++++++++++++++++++++++++--- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 2 +- 6 files changed, 31 insertions(+), 8 deletions(-) -- 2.7.5

Recent commits made it so that pci-root controllers for pSeries guests are automatically assigned the spapr-pci-host-bridge model name; however, that prevents guests to migrate to older versions of libvirt which don't know about that model name at all, which at the moment is all of them :) To avoid the issue, just strip the model name from PHBs when formatting the migratable XML; guests that use more than one PHB are not going to be migratable anyway. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d88a88e..b5ab97a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21813,7 +21813,30 @@ virDomainControllerDefFormat(virBufferPtr buf, } if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { - if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + bool formatModelName = true; + + if (def->opts.pciopts.modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) + formatModelName = false; + + /* Historically, libvirt didn't support specifying a model name for + * pci-root controllers; starting from 3.6.0, however, pSeries guests + * use pci-root controllers with model name spapr-pci-host-bridge to + * represent all PHBs, including the default one. + * + * In order to allow migration of pSeries guests from older libvirt + * versions and back, we don't format the model name in the migratable + * XML if it's spapr-pci-host-bridge, thus making "no model name" and + * "spapr-pci-host-bridge model name" basically equivalent. + * + * The spapr-pci-host-bridge device is specific to pSeries. + */ + if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT && + def->opts.pciopts.modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE && + flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) { + formatModelName = false; + } + + if (formatModelName) { modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.7.5

On Mon, Jul 24, 2017 at 13:40:57 +0200, Andrea Bolognani wrote:
Recent commits made it so that pci-root controllers for pSeries guests are automatically assigned the spapr-pci-host-bridge model name; however, that prevents guests to migrate to older versions of libvirt which don't know about that model name at all, which at the moment is all of them :)
To avoid the issue, just strip the model name from PHBs when formatting the migratable XML; guests that use more than one PHB are not going to be migratable anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
ACK

The original name didn't hint at the fact that PHBs are a pSeries-specific concept. Suggested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b5ab97a..45693f4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1892,7 +1892,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def) /** - * virDomainControllerIsPCIHostBridge: + * virDomainControllerIsPSeriesPHB: * @cont: controller * * Checks whether @cont is a PCI Host Bridge (PHB), a specific type @@ -1901,7 +1901,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def) * Returns: true if @cont is a PHB, false otherwise. */ bool -virDomainControllerIsPCIHostBridge(const virDomainControllerDef *cont) +virDomainControllerIsPSeriesPHB(const virDomainControllerDef *cont) { virDomainControllerPCIModelName name; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4fef773..a2e8f67 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2681,7 +2681,7 @@ virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def, virDomainControllerDefPtr virDomainControllerDefNew(virDomainControllerType type); void virDomainControllerDefFree(virDomainControllerDefPtr def); -bool virDomainControllerIsPCIHostBridge(const virDomainControllerDef *cont); +bool virDomainControllerIsPSeriesPHB(const virDomainControllerDef *cont); virDomainFSDefPtr virDomainFSDefNew(void); void virDomainFSDefFree(virDomainFSDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eda4269..6f3549a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -228,7 +228,7 @@ virDomainControllerFindByType; virDomainControllerFindUnusedIndex; virDomainControllerInsert; virDomainControllerInsertPreAlloced; -virDomainControllerIsPCIHostBridge; +virDomainControllerIsPSeriesPHB; virDomainControllerModelPCITypeToString; virDomainControllerModelSCSITypeFromString; virDomainControllerModelSCSITypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0ce5aa5..90f6883 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -317,7 +317,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && cont->idx == info->addr.pci.bus) { contAlias = cont->info.alias; - contIsPHB = virDomainControllerIsPCIHostBridge(cont); + contIsPHB = virDomainControllerIsPSeriesPHB(cont); contTargetIndex = cont->opts.pciopts.targetIndex; if (!contAlias) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c8c9a7..3d79765 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6735,7 +6735,7 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def) for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; - if (!virDomainControllerIsPCIHostBridge(cont)) + if (!virDomainControllerIsPSeriesPHB(cont)) continue; nPCIHostBridges++; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d943c8b..8066ed1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2167,7 +2167,7 @@ qemuDomainAddressFindNewTargetIndex(virDomainDefPtr def) virDomainControllerDefPtr cont = def->controllers[i]; /* Skip everything but PHBs */ - if (!virDomainControllerIsPCIHostBridge(cont)) + if (!virDomainControllerIsPSeriesPHB(cont)) continue; /* Stop looking as soon as we find a PHB that's -- 2.7.5

On Mon, Jul 24, 2017 at 13:40:58 +0200, Andrea Bolognani wrote:
The original name didn't hint at the fact that PHBs are a pSeries-specific concept.
Suggested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-)
ACK
participants (2)
-
Andrea Bolognani
-
Peter Krempa