11:45 a.m.
Pavel Hrdina (2):
qemu_migration: don't allow ABI changes for persistent migration
qemu_domain: use correct default USB controller on ppc64
src/qemu/qemu_domain.c | 16 ++++++++++++----
src/qemu/qemu_migration.c | 1 -
2 files changed, 12 insertions(+), 5 deletions(-)
--
2.12.0
11:45 a.m.
New subject: [libvirt] [RFC v2 1/2] qemu_migration: don't allow ABI changes for persistent migration
So far there is probably no change that is allowed to be done
by the VIR_DOMAIN_DEF_PARSE_ABI_UPDATE flag that would break
guest ABI but this may change in the future.
The other cases where this flag is used is only when we are
defining new domain or adding new device into a domain.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
This patch is a product of a discussion about the last patch in v1 [1].
Currently we allow ABI changes for persistent migration however it might
be something that user don't expect to be done.
Technically it defines new domain on the destination which would fall
into the same category as defining new domain from scratch without
migration but it may be unexpected behavior because for live migration
we don't allow ABI changes (for obvious reasons).
At first I though that this is correct and we are doing the right thing,
but now I'm not so sure about that and IMHO it would be probably better
to not do ABI updates in this case like we don't do if libvirtd is
restarted (for example because of an update) and also it would be
consistent with the live migration.
[1] <https://www.redhat.com/archives/libvir-list/2017-March/msg00057.html>
src/qemu/qemu_migration.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f5711bcf74..e45bb45670 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1314,7 +1314,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
mig->persistent = virDomainDefParseNode(doc, nodes[0],
caps, driver->xmlopt, NULL,
VIR_DOMAIN_DEF_PARSE_INACTIVE |
- VIR_DOMAIN_DEF_PARSE_ABI_UPDATE |
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
if (!mig->persistent) {
/* virDomainDefParseNode already reported
--
2.12.0
2:22 a.m.
New subject: [libvirt] [RFC v2 1/2] qemu_migration: don't allow ABI changes for persistent migration
On Tue, Mar 07, 2017 at 18:45:23 +0100, Pavel Hrdina wrote:
So far there is probably no change that is allowed to be done
by the VIR_DOMAIN_DEF_PARSE_ABI_UPDATE flag that would break
guest ABI but this may change in the future.
The other cases where this flag is used is only when we are
defining new domain or adding new device into a domain.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
This patch is a product of a discussion about the last patch in v1 [1].
Currently we allow ABI changes for persistent migration however it might
be something that user don't expect to be done.
Technically it defines new domain on the destination which would fall
into the same category as defining new domain from scratch without
migration but it may be unexpected behavior because for live migration
we don't allow ABI changes (for obvious reasons).
At first I though that this is correct and we are doing the right thing,
but now I'm not so sure about that and IMHO it would be probably better
to not do ABI updates in this case like we don't do if libvirtd is
restarted (for example because of an update) and also it would be
consistent with the live migration.
This flag was invented so that libvirt could finally properly track the
total memory size of the VM when NUMA is used. Prior to that the
individual node sizes would not have to total up to the size declared in
the <memory> element. While migrating a live VM we can't change this but
other code expects that the size is correct.
This patch would break this particular case. In general I'm not against
a change, but the recalculation of the memory sizes should not be
broken.
4:36 a.m.
New subject: [libvirt] [RFC v2 1/2] qemu_migration: don't allow ABI changes for persistent migration
On Wed, 2017-03-08 at 09:22 +0100, Peter Krempa wrote:
On Tue, Mar 07, 2017 at 18:45:23 +0100, Pavel Hrdina wrote:
>
> So far there is probably no change that is allowed to be done
> by the VIR_DOMAIN_DEF_PARSE_ABI_UPDATE flag that would break
> guest ABI but this may change in the future.
>
> The other cases where this flag is used is only when we are
> defining new domain or adding new device into a domain.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
>
> This patch is a product of a discussion about the last patch in v1 [1].
> Currently we allow ABI changes for persistent migration however it might
> be something that user don't expect to be done.
>
> Technically it defines new domain on the destination which would fall
> into the same category as defining new domain from scratch without
> migration but it may be unexpected behavior because for live migration
> we don't allow ABI changes (for obvious reasons).
>
> At first I though that this is correct and we are doing the right thing,
> but now I'm not so sure about that and IMHO it would be probably better
> to not do ABI updates in this case like we don't do if libvirtd is
> restarted (for example because of an update) and also it would be
> consistent with the live migration.
This flag was invented so that libvirt could finally properly track the
total memory size of the VM when NUMA is used. Prior to that the
individual node sizes would not have to total up to the size declared in
the <memory> element. While migrating a live VM we can't change this but
other code expects that the size is correct.
This patch would break this particular case. In general I'm not against
a change, but the recalculation of the memory sizes should not be
broken.
I see.
However, as it is now, we're granting blanket permission
to update the guest ABI in whatever random way during a
persistent migration, which at least to me feels like it's
way too broad.
Can we maybe introduce a less coarse-grained flag just for
the memory update, and leave the broader ABI_UPDATE for
cases such as defining an entirely new guest or attaching
an entirely new device, eg. situations where any change we
perform won't affect the existing guest? Or is it too late
for that?
--
Andrea Bolognani / Red Hat / Virtualization
11:45 a.m.
New subject: [libvirt] [PATCH v2 2/2] qemu_domain: use correct default USB controller on ppc64
The history of USB controller for ppc64 guest is complex and goes
back to libvirt 1.3.1 where the fun started.
Prior Libvirt 1.3.1 if no model for USB controller was specified
we've simply passed "-usb" on QEMU command line.
Since Libvirt 1.3.1 there is a patch (8156493d8db) that fixes this
issue by using "-device pci-ohci,..." but it breaks migration with
older Libvirts which was agreed that's acceptable. However this
patch didn't reflect this change in the domain XML and the model
was still missing.
Since Libvirt 2.2.0 there is a patch (f55eaccb0c5) that fixes the
issue with not setting the USB model into domain XML which we need
to know about to not break the migration and since the default
model was *pci-ohci* it was used as default in this patch as well.
This patch tries to take all the previous changes into account and
also change the default for newly defined domains that don't specify
any model for USB controller.
The VIR_DOMAIN_DEF_PARSE_ABI_UPDATE is set only if new domain is
defined or new device is added into a domain which means that in
all other cases we will use the old *pci-ohci* model instead of the
better and not broken *nec-usb-xhci* model.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1373184
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
No changes from v1, the current default pci-ohci is broken and
if user don't care about the model we should default to nec-xhci.
src/qemu/qemu_domain.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9be78bdbd4..12af0651a5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3012,7 +3012,8 @@ qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
static int
qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
const virDomainDef *def,
- virQEMUCapsPtr qemuCaps)
+ virQEMUCapsPtr qemuCaps,
+ unsigned int parseFlags)
{
switch ((virDomainControllerType)cont->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
@@ -3040,9 +3041,16 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
* address is found */
cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
} else if (ARCH_IS_PPC64(def->os.arch)) {
- /* Default USB controller for ppc64 is pci-ohci */
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
+ /* To not break migration we need to set default USB controller
+ * for ppc64 to pci-ohci if we cannot change ABI of the VM.
+ * The nec-usb-xhci controller is used as default only for
+ * newly defined domains or devices. */
+ if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) {
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
+ } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) {
cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
+ }
} else {
/* Default USB controller for anything else is piix3-uhci */
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
@@ -3241,7 +3249,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
qemuDomainControllerDefPostParse(dev->data.controller, def,
- qemuCaps) < 0)
+ qemuCaps, parseFlags) < 0)
goto cleanup;
if (dev->type == VIR_DOMAIN_DEVICE_SHMEM &&
--
2.12.0
2816
days inactive
2817
days old
4 comments
3 participants
participants (3)
-
Andrea Bolognani -
Pavel Hrdina -
Peter Krempa