2:48 a.m.
Pavel Hrdina (3):
qemu_domain: move controller post parse code into its own function
qemu_domain: cleanup the controller post parse code
qemu_domain: use correct default USB controller on ppc64
src/qemu/qemu_domain.c | 218 +++++++++++++++++++++++++++----------------------
1 file changed, 120 insertions(+), 98 deletions(-)
--
2.12.0
2:48 a.m.
New subject: [libvirt] [PATCH 1/3] qemu_domain: move controller post parse code into its own function
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_domain.c | 202 +++++++++++++++++++++++++------------------------
1 file changed, 104 insertions(+), 98 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c187214dc3..5c5a055354 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3010,6 +3010,106 @@ qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
static int
+qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ /* set the default USB model to none for s390 unless an address is found */
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ cont->model == -1 &&
+ cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ ARCH_IS_S390(def->os.arch))
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
+
+ /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr
*/
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("USB controller model type 'qusb1' or
'qusb2' "
+ "is not supported in %s"),
+ virDomainVirtTypeToString(def->virtType));
+ return -1;
+ }
+
+
+ /* set the default SCSI controller model for S390 arches */
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
+ cont->model == -1 &&
+ ARCH_IS_S390(def->os.arch))
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI &&
+ cont->opts.usbopts.ports > QEMU_USB_NEC_XHCI_MAXPORTS) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("nec-xhci controller only supports up to %u ports"),
+ QEMU_USB_NEC_XHCI_MAXPORTS);
+ return -1;
+ }
+
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS &&
+ !qemuDomainMachineIsI440FX(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("pci-expander-bus controllers are only supported
"
+ "on 440fx-based machinetypes"));
+ return -1;
+ }
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS &&
+ !qemuDomainMachineIsQ35(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("pcie-expander-bus controllers are only supported
"
+ "on q35-based machinetypes"));
+ return -1;
+ }
+
+ /* if a PCI expander bus has a NUMA node set, make sure
+ * that NUMA node is configured in the guest <cpu><numa>
+ * array. NUMA cell id's in this array are numbered
+ * from 0 .. size-1.
+ */
+ if ((cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS) &&
+ (int) virDomainNumaGetNodeCount(def->numa)
+ <= cont->opts.pciopts.numaNode) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("%s with index %d is "
+ "configured for a NUMA node (%d) "
+ "not present in the domain's "
+ "<cpu><numa> array (%zu)"),
+ virDomainControllerModelPCITypeToString(cont->model),
+ cont->idx, cont->opts.pciopts.numaNode,
+ virDomainNumaGetNodeCount(def->numa));
+ return -1;
+ }
+ } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ cont->model == -1) {
+ /* Pick a suitable default model for the USB controller if none
+ * has been selected by the user.
+ *
+ * We rely on device availability instead of setting the model
+ * unconditionally because, for some machine types, there's a
+ * chance we will get away with using the legacy USB controller
+ * when the relevant device is not available.
+ *
+ * See qemuBuildControllerDevCommandLine() */
+ if (ARCH_IS_PPC64(def->os.arch)) {
+ /* Default USB controller for ppc64 is pci-ohci */
+ 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))
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+ }
+ }
+
+ return 0;
+}
+
+
+static int
qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
const virDomainDef *def,
virCapsPtr caps,
@@ -3087,34 +3187,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
ARCH_IS_S390(def->os.arch))
dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO;
- /* set the default USB model to none for s390 unless an address is found */
- if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
- dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- dev->data.controller->model == -1 &&
- dev->data.controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
&&
- ARCH_IS_S390(def->os.arch))
- dev->data.controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
-
- /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr
*/
- if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
- dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- (dev->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
- dev->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("USB controller model type 'qusb1' or
'qusb2' "
- "is not supported in %s"),
- virDomainVirtTypeToString(def->virtType));
- goto cleanup;
- }
-
-
- /* set the default SCSI controller model for S390 arches */
- if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
- dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- dev->data.controller->model == -1 &&
- ARCH_IS_S390(def->os.arch))
- dev->data.controller->model =
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
-
/* clear auto generated unix socket path for inactive definitions */
if ((parseFlags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
dev->type == VIR_DOMAIN_DEVICE_CHR)
@@ -3159,76 +3231,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_ISA;
}
-
- if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
- virDomainControllerDefPtr cont = dev->data.controller;
-
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI &&
- cont->opts.usbopts.ports > QEMU_USB_NEC_XHCI_MAXPORTS) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("nec-xhci controller only supports up to %u
ports"),
- QEMU_USB_NEC_XHCI_MAXPORTS);
- goto cleanup;
- }
-
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
- if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS
&&
- !qemuDomainMachineIsI440FX(def)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("pci-expander-bus controllers are only supported
"
- "on 440fx-based machinetypes"));
- goto cleanup;
- }
- if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS
&&
- !qemuDomainMachineIsQ35(def)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("pcie-expander-bus controllers are only supported
"
- "on q35-based machinetypes"));
- goto cleanup;
- }
-
- /* if a PCI expander bus has a NUMA node set, make sure
- * that NUMA node is configured in the guest <cpu><numa>
- * array. NUMA cell id's in this array are numbered
- * from 0 .. size-1.
- */
- if ((cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS ||
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS)
&&
- (int) virDomainNumaGetNodeCount(def->numa)
- <= cont->opts.pciopts.numaNode) {
- virReportError(VIR_ERR_XML_ERROR,
- _("%s with index %d is "
- "configured for a NUMA node (%d) "
- "not present in the domain's "
- "<cpu><numa> array (%zu)"),
- virDomainControllerModelPCITypeToString(cont->model),
- cont->idx, cont->opts.pciopts.numaNode,
- virDomainNumaGetNodeCount(def->numa));
- goto cleanup;
- }
- } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == -1) {
- /* Pick a suitable default model for the USB controller if none
- * has been selected by the user.
- *
- * We rely on device availability instead of setting the model
- * unconditionally because, for some machine types, there's a
- * chance we will get away with using the legacy USB controller
- * when the relevant device is not available.
- *
- * See qemuBuildControllerDevCommandLine() */
- if (ARCH_IS_PPC64(def->os.arch)) {
- /* Default USB controller for ppc64 is pci-ohci */
- 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))
- cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
- }
- }
- }
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
+ qemuDomainControllerDefPostParse(dev->data.controller, def,
+ qemuCaps) < 0)
+ goto cleanup;
if (dev->type == VIR_DOMAIN_DEVICE_SHMEM &&
qemuDomainShmemDefPostParse(dev->data.shmem) < 0)
--
2.12.0
9:20 a.m.
New subject: [libvirt] [PATCH 1/3] qemu_domain: move controller post parse code into its own function
On Thu, 2017-03-02 at 09:48 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_domain.c | 202 +++++++++++++++++++++++++------------------------
1 file changed, 104 insertions(+), 98 deletions(-)
ACK
--
Andrea Bolognani / Red Hat / Virtualization
2:48 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_domain: cleanup the controller post parse code
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_domain.c | 114 ++++++++++++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 53 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5c5a055354..10e831b779 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3014,41 +3014,60 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
- /* set the default USB model to none for s390 unless an address is found */
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == -1 &&
- cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
- ARCH_IS_S390(def->os.arch))
- cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
+ switch ((virDomainControllerType)cont->type) {
+ case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
+ /* set the default SCSI controller model for S390 arches */
+ if (cont->model == -1 &&
+ ARCH_IS_S390(def->os.arch)) {
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+ }
+ break;
- /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr
*/
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("USB controller model type 'qusb1' or
'qusb2' "
- "is not supported in %s"),
- virDomainVirtTypeToString(def->virtType));
- return -1;
- }
+ case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+ if (cont->model == -1) {
+ /* Pick a suitable default model for the USB controller if none
+ * has been selected by the user.
+ *
+ * We rely on device availability instead of setting the model
+ * unconditionally because, for some machine types, there's a
+ * chance we will get away with using the legacy USB controller
+ * when the relevant device is not available.
+ *
+ * See qemuBuildControllerDevCommandLine() */
+ if (ARCH_IS_S390(def->os.arch) &&
+ cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ /* set the default USB model to none for s390 unless an
+ * 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))
+ 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))
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+ }
+ }
+ /* forbid usb model 'qusb1' and 'qusb2' in this kind of
hyperviosr */
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("USB controller model type 'qusb1' or
'qusb2' "
+ "is not supported in %s"),
+ virDomainVirtTypeToString(def->virtType));
+ return -1;
+ }
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI &&
+ cont->opts.usbopts.ports > QEMU_USB_NEC_XHCI_MAXPORTS) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("nec-xhci controller only supports up to %u
ports"),
+ QEMU_USB_NEC_XHCI_MAXPORTS);
+ return -1;
+ }
+ break;
-
- /* set the default SCSI controller model for S390 arches */
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- cont->model == -1 &&
- ARCH_IS_S390(def->os.arch))
- cont->model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
-
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI &&
- cont->opts.usbopts.ports > QEMU_USB_NEC_XHCI_MAXPORTS) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("nec-xhci controller only supports up to %u ports"),
- QEMU_USB_NEC_XHCI_MAXPORTS);
- return -1;
- }
-
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+ case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS &&
!qemuDomainMachineIsI440FX(def)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -3083,26 +3102,15 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
virDomainNumaGetNodeCount(def->numa));
return -1;
}
- } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == -1) {
- /* Pick a suitable default model for the USB controller if none
- * has been selected by the user.
- *
- * We rely on device availability instead of setting the model
- * unconditionally because, for some machine types, there's a
- * chance we will get away with using the legacy USB controller
- * when the relevant device is not available.
- *
- * See qemuBuildControllerDevCommandLine() */
- if (ARCH_IS_PPC64(def->os.arch)) {
- /* Default USB controller for ppc64 is pci-ohci */
- 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))
- cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
- }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+ case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
+ case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
+ case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
+ case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
+ break;
}
return 0;
--
2.12.0
9:22 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_domain: cleanup the controller post parse code
On Thu, 2017-03-02 at 09:48 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_domain.c | 114 ++++++++++++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 53 deletions(-)
Nice cleanup!
ACK
--
Andrea Bolognani / Red Hat / Virtualization
2:48 a.m.
New subject: [libvirt] [PATCH 3/3] 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>
---
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 10e831b779..64dbd0a491 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
1:11 p.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Thu, 2017-03-02 at 09:48 +0100, Pavel Hrdina wrote:
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.
Unfortunately, migrating a guest with
<controller type='usb' index='0'>
<address type='pci' .../>
</controller>
from libvirt 2.1.0 (which still allows not specifying the
controller model) to a build of libvirt including this patch
doesn't work as expected.
On the source host, I run
# virsh migrate \
mig-usb \
qemu+ssh://root@destination/system \
--live --persistent --copy-storage-all --compressed
While migration is still running, I can check on the
destination host and verify that:
* the <controller> element has model='pci-ohci';
* the QEMU command line contains -device pci-ohci.
However, after migration is complete, the <controller>
element has model='nec-xhci' instead of model='pci-ohci',
which means that power cycling the guest results in
breaking the guest ABI.
--
Andrea Bolognani / Red Hat / Virtualization
2:19 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Mon, Mar 06, 2017 at 08:11:40PM +0100, Andrea Bolognani wrote:
On Thu, 2017-03-02 at 09:48 +0100, Pavel Hrdina wrote:
> 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.
Unfortunately, migrating a guest with
<controller type='usb' index='0'>
<address type='pci' .../>
</controller>
from libvirt 2.1.0 (which still allows not specifying the
controller model) to a build of libvirt including this patch
doesn't work as expected.
On the source host, I run
# virsh migrate \
mig-usb \
qemu+ssh://root@destination/system \
--live --persistent --copy-storage-all --compressed
While migration is still running, I can check on the
destination host and verify that:
* the <controller> element has model='pci-ohci';
* the QEMU command line contains -device pci-ohci.
However, after migration is complete, the <controller>
element has model='nec-xhci' instead of model='pci-ohci',
which means that power cycling the guest results in
breaking the guest ABI.
I'm not so sure that this is an ABI change. The guest ABI is to ensure
that the same guest XML will always start the same QEMU guest. However
the PERSISTENT migration can make ABI changes because it is the same as
virsh dumpxml $domain > $domain.xml && copy the XML onto remote host
and virsh define $domain.xml. This would also change the *model*.
If this would be considered to be guest ABI stable it would mean that
other changes done by using this flag would be wrong because they also
modifies the persistent XML during migration.
Pavel
7:55 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, 2017-03-07 at 09:19 +0100, Pavel Hrdina wrote:
> However, after migration is complete, the <controller>
> element has model='nec-xhci' instead of model='pci-ohci',
> which means that power cycling the guest results in
> breaking the guest ABI.
I'm not so sure that this is an ABI change. The guest ABI is to ensure
that the same guest XML will always start the same QEMU guest. However
the PERSISTENT migration can make ABI changes because it is the same as
virsh dumpxml $domain > $domain.xml && copy the XML onto remote host
and virsh define $domain.xml. This would also change the *model*.
If this would be considered to be guest ABI stable it would mean that
other changes done by using this flag would be wrong because they also
modifies the persistent XML during migration.
I assume there are very good reasons for persistent
migration to behave differently, but as a user I find
it extremely surprising. Do you have any insight on the
rationale behind allowing ABI changes when performing
persistent migration?
The only other example I could find of ABI update being
used is in virDomainDefPostParseMemory(), and AFAIU adding
or removing memory from a guest is not an ABI change. Did
I miss other uses?
--
Andrea Bolognani / Red Hat / Virtualization
8:32 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, Mar 07, 2017 at 02:55:43PM +0100, Andrea Bolognani wrote:
On Tue, 2017-03-07 at 09:19 +0100, Pavel Hrdina wrote:
> > However, after migration is complete, the <controller>
> > element has model='nec-xhci' instead of model='pci-ohci',
> > which means that power cycling the guest results in
> > breaking the guest ABI.
>
> I'm not so sure that this is an ABI change. The guest ABI is to ensure
> that the same guest XML will always start the same QEMU guest. However
> the PERSISTENT migration can make ABI changes because it is the same as
> virsh dumpxml $domain > $domain.xml && copy the XML onto remote host
> and virsh define $domain.xml. This would also change the *model*.
>
> If this would be considered to be guest ABI stable it would mean that
> other changes done by using this flag would be wrong because they also
> modifies the persistent XML during migration.
I assume there are very good reasons for persistent
migration to behave differently, but as a user I find
it extremely surprising. Do you have any insight on the
rationale behind allowing ABI changes when performing
persistent migration?
Well it's a different kind of migration which is almost similar to the
virsh dumpxml --inactive && virsh define, with one more feature, migration
hook that can update the XML before it's defined on the destination.
I agree that it may seem that we should handle this type of migration to
be guest ABI stable, on the other hand this basically defines new domain
on the destination where we allow to do the ABI updates.
I'm adding Dan to CC to get his opinion about the guest ABI.
The only other example I could find of ABI update being
used is in virDomainDefPostParseMemory(), and AFAIU adding
or removing memory from a guest is not an ABI change. Did
I miss other uses?
There is also an update of USB addresses in qemuDomainAssignUSBAddresses.
Pavel
8:40 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, Mar 07, 2017 at 03:32:37PM +0100, Pavel Hrdina wrote:
On Tue, Mar 07, 2017 at 02:55:43PM +0100, Andrea Bolognani wrote:
> On Tue, 2017-03-07 at 09:19 +0100, Pavel Hrdina wrote:
> > > However, after migration is complete, the <controller>
> > > element has model='nec-xhci' instead of model='pci-ohci',
> > > which means that power cycling the guest results in
> > > breaking the guest ABI.
> >
> > I'm not so sure that this is an ABI change. The guest ABI is to ensure
> > that the same guest XML will always start the same QEMU guest. However
> > the PERSISTENT migration can make ABI changes because it is the same as
> > virsh dumpxml $domain > $domain.xml && copy the XML onto remote
host
> > and virsh define $domain.xml. This would also change the *model*.
> >
> > If this would be considered to be guest ABI stable it would mean that
> > other changes done by using this flag would be wrong because they also
> > modifies the persistent XML during migration.
>
> I assume there are very good reasons for persistent
> migration to behave differently, but as a user I find
> it extremely surprising. Do you have any insight on the
> rationale behind allowing ABI changes when performing
> persistent migration?
Well it's a different kind of migration which is almost similar to the
virsh dumpxml --inactive && virsh define, with one more feature, migration
hook that can update the XML before it's defined on the destination.
I agree that it may seem that we should handle this type of migration to
be guest ABI stable, on the other hand this basically defines new domain
on the destination where we allow to do the ABI updates.
I'm adding Dan to CC to get his opinion about the guest ABI.
So, there's no particular reason why things must be different for
live vs persistent migration. The only fundamental requirement is
that we must never change ABI underneath a running guest, which
mandates the strong ABI guarantee for live migration & save/restore.
For persistent/offline migration, there's no running guest, so from
a technical POV we have more freedom in some sense. That said, a
change in guest hardaware that can cause a guest to become unbootable
is not a nice thing todo.
For this reason, when you use 'defineXML' libvirt aims to immediately
fill in (most) XML elements that would affect guest ABI. ie we assign
addresses at define time, not start time, and we expand non-versioned
machine types into versioned machine types, etc.
So, yes, we should try to *not* change ABI across a dumpxml & define
operational.
When we first introduced PCI device addressing in libvirt, we had an
old state where XML contained no addresses, and upon starting in new
libvirt we have to assign addresses which might not be the same as
those seen with previous libvirt before addressing was recorded. Once
the addresses are assigned though, we must never change them.
So...
- if we recorded 'pci-ohci' in the XML, we must never change that
to 'nec-xhci'.
- if we didn't record 'pci-ochi' in the XML, we should fill in the
model that is most likely to match what the previous default
behaviour was, which would be 'pci-ochi' not 'nec-xhci' IIUC.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
9:15 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, Mar 07, 2017 at 02:40:55PM +0000, Daniel P. Berrange wrote:
On Tue, Mar 07, 2017 at 03:32:37PM +0100, Pavel Hrdina wrote:
> On Tue, Mar 07, 2017 at 02:55:43PM +0100, Andrea Bolognani wrote:
> > On Tue, 2017-03-07 at 09:19 +0100, Pavel Hrdina wrote:
> > > > However, after migration is complete, the <controller>
> > > > element has model='nec-xhci' instead of
model='pci-ohci',
> > > > which means that power cycling the guest results in
> > > > breaking the guest ABI.
> > >
> > > I'm not so sure that this is an ABI change. The guest ABI is to
ensure
> > > that the same guest XML will always start the same QEMU guest. However
> > > the PERSISTENT migration can make ABI changes because it is the same as
> > > virsh dumpxml $domain > $domain.xml && copy the XML onto remote
host
> > > and virsh define $domain.xml. This would also change the *model*.
> > >
> > > If this would be considered to be guest ABI stable it would mean that
> > > other changes done by using this flag would be wrong because they also
> > > modifies the persistent XML during migration.
> >
> > I assume there are very good reasons for persistent
> > migration to behave differently, but as a user I find
> > it extremely surprising. Do you have any insight on the
> > rationale behind allowing ABI changes when performing
> > persistent migration?
>
> Well it's a different kind of migration which is almost similar to the
> virsh dumpxml --inactive && virsh define, with one more feature, migration
> hook that can update the XML before it's defined on the destination.
>
> I agree that it may seem that we should handle this type of migration to
> be guest ABI stable, on the other hand this basically defines new domain
> on the destination where we allow to do the ABI updates.
>
> I'm adding Dan to CC to get his opinion about the guest ABI.
So, there's no particular reason why things must be different for
live vs persistent migration. The only fundamental requirement is
that we must never change ABI underneath a running guest, which
mandates the strong ABI guarantee for live migration & save/restore.
For persistent/offline migration, there's no running guest, so from
a technical POV we have more freedom in some sense. That said, a
change in guest hardaware that can cause a guest to become unbootable
is not a nice thing todo.
For this reason, when you use 'defineXML' libvirt aims to immediately
fill in (most) XML elements that would affect guest ABI. ie we assign
addresses at define time, not start time, and we expand non-versioned
machine types into versioned machine types, etc.
So, yes, we should try to *not* change ABI across a dumpxml & define
operational.
When we first introduced PCI device addressing in libvirt, we had an
old state where XML contained no addresses, and upon starting in new
libvirt we have to assign addresses which might not be the same as
those seen with previous libvirt before addressing was recorded. Once
the addresses are assigned though, we must never change them.
This is basically the same issue like with the PCI addresses. We used "-usb"
instead of "-device ...", but when we switched to using the "-device
..."
way we didn't record the default model in the XML. The default model
is recorded in the XML since Libvirt 2.2.0.
So...
- if we recorded 'pci-ohci' in the XML, we must never change that
to 'nec-xhci'.
This will never happen.
- if we didn't record 'pci-ochi' in the XML, we should
fill in the
model that is most likely to match what the previous default
behaviour was, which would be 'pci-ochi' not 'nec-xhci' IIUC.
This patch tries to move from 'pci-ohci' to 'nec-xhci' as a default USB
controller because the current default model is probably broken, see [1],
and will never be fixed. In addition I would like to do this change in
Libvirt so all other tools that uses Libvirt don't have to implement
this change by themselves.
In this case I think it's better to change the default from 'pci-ohci' to
'nec-xhci' if you define a new domain where the model is not specified.
There is a concern whether we should allow ABI change for persistent
migration which is more general issue that will affect some other changes
that are already "allowed" for persistent migration. I'm starting to feel
that we should remove the "VIR_DOMAIN_DEF_PARSE_ABI_UPDATE" from
"virDomainDefParseNode" call in "qemuMigrationCookieXMLParse"
function
which currently allows to do ABI changes for persistent migration.
Pavel
[1] <https://bugzilla.redhat.com/show_bug.cgi?id=1212275>
9:26 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, Mar 07, 2017 at 04:15:03PM +0100, Pavel Hrdina wrote:
On Tue, Mar 07, 2017 at 02:40:55PM +0000, Daniel P. Berrange wrote:
> On Tue, Mar 07, 2017 at 03:32:37PM +0100, Pavel Hrdina wrote:
> > On Tue, Mar 07, 2017 at 02:55:43PM +0100, Andrea Bolognani wrote:
> > > On Tue, 2017-03-07 at 09:19 +0100, Pavel Hrdina wrote:
> > > > > However, after migration is complete, the <controller>
> > > > > element has model='nec-xhci' instead of
model='pci-ohci',
> > > > > which means that power cycling the guest results in
> > > > > breaking the guest ABI.
> > > >
> > > > I'm not so sure that this is an ABI change. The guest ABI is to
ensure
> > > > that the same guest XML will always start the same QEMU
guest. However
> > > > the PERSISTENT migration can make ABI changes because it is the same
as
> > > > virsh dumpxml $domain > $domain.xml && copy the XML onto
remote host
> > > > and virsh define $domain.xml. This would also change the *model*.
> > > >
> > > > If this would be considered to be guest ABI stable it would mean
that
> > > > other changes done by using this flag would be wrong because they
also
> > > > modifies the persistent XML during migration.
> > >
> > > I assume there are very good reasons for persistent
> > > migration to behave differently, but as a user I find
> > > it extremely surprising. Do you have any insight on the
> > > rationale behind allowing ABI changes when performing
> > > persistent migration?
> >
> > Well it's a different kind of migration which is almost similar to the
> > virsh dumpxml --inactive && virsh define, with one more feature,
migration
> > hook that can update the XML before it's defined on the destination.
> >
> > I agree that it may seem that we should handle this type of migration to
> > be guest ABI stable, on the other hand this basically defines new domain
> > on the destination where we allow to do the ABI updates.
> >
> > I'm adding Dan to CC to get his opinion about the guest ABI.
>
> So, there's no particular reason why things must be different for
> live vs persistent migration. The only fundamental requirement is
> that we must never change ABI underneath a running guest, which
> mandates the strong ABI guarantee for live migration & save/restore.
>
> For persistent/offline migration, there's no running guest, so from
> a technical POV we have more freedom in some sense. That said, a
> change in guest hardaware that can cause a guest to become unbootable
> is not a nice thing todo.
>
> For this reason, when you use 'defineXML' libvirt aims to immediately
> fill in (most) XML elements that would affect guest ABI. ie we assign
> addresses at define time, not start time, and we expand non-versioned
> machine types into versioned machine types, etc.
>
> So, yes, we should try to *not* change ABI across a dumpxml & define
> operational.
>
> When we first introduced PCI device addressing in libvirt, we had an
> old state where XML contained no addresses, and upon starting in new
> libvirt we have to assign addresses which might not be the same as
> those seen with previous libvirt before addressing was recorded. Once
> the addresses are assigned though, we must never change them.
This is basically the same issue like with the PCI addresses. We used "-usb"
instead of "-device ...", but when we switched to using the "-device
..."
way we didn't record the default model in the XML. The default model
is recorded in the XML since Libvirt 2.2.0.
Ok, that makes sense then. Until we are able to use -device we can't
gaurantee stability for an arch.
> - if we recorded 'pci-ohci' in the XML, we must never
change that
> to 'nec-xhci'.
This will never happen.
> - if we didn't record 'pci-ochi' in the XML, we should fill in the
> model that is most likely to match what the previous default
> behaviour was, which would be 'pci-ochi' not 'nec-xhci' IIUC.
This patch tries to move from 'pci-ohci' to 'nec-xhci' as a default USB
controller because the current default model is probably broken, see [1],
and will never be fixed. In addition I would like to do this change in
Libvirt so all other tools that uses Libvirt don't have to implement
this change by themselves.
In this case I think it's better to change the default from 'pci-ohci' to
'nec-xhci' if you define a new domain where the model is not specified.
If I look at qemu-system-ppc64 -usb, that provides an 'nec-xhci' device.
I'm curious why libvirt picked pci-ohci ? Was -usb previously providing
a pci-ohci device instead of nec-xhci ?
I guess the thing that could justify the switch is that we don't guarantee
what default devices you'll get for any given guest. It is dependent on
the particular version of the machine type used. So in that sense if you
have an XML doc which is not fully populated with device info, you are
already liable to see changes in default devices based on which QEMU
you happen to define the XML against first.
There is a concern whether we should allow ABI change for persistent
migration which is more general issue that will affect some other changes
that are already "allowed" for persistent migration. I'm starting to feel
that we should remove the "VIR_DOMAIN_DEF_PARSE_ABI_UPDATE" from
"virDomainDefParseNode" call in "qemuMigrationCookieXMLParse"
function
which currently allows to do ABI changes for persistent migration.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
9:35 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, Mar 07, 2017 at 03:26:55PM +0000, Daniel P. Berrange wrote:
On Tue, Mar 07, 2017 at 04:15:03PM +0100, Pavel Hrdina wrote:
> On Tue, Mar 07, 2017 at 02:40:55PM +0000, Daniel P. Berrange wrote:
> > On Tue, Mar 07, 2017 at 03:32:37PM +0100, Pavel Hrdina wrote:
> > > On Tue, Mar 07, 2017 at 02:55:43PM +0100, Andrea Bolognani wrote:
> > > > On Tue, 2017-03-07 at 09:19 +0100, Pavel Hrdina wrote:
> > > > > > However, after migration is complete, the
<controller>
> > > > > > element has model='nec-xhci' instead of
model='pci-ohci',
> > > > > > which means that power cycling the guest results in
> > > > > > breaking the guest ABI.
> > > > >
> > > > > I'm not so sure that this is an ABI change. The guest ABI
is to ensure
> > > > > that the same guest XML will always start the same QEMU
guest. However
> > > > > the PERSISTENT migration can make ABI changes because it is the
same as
> > > > > virsh dumpxml $domain > $domain.xml && copy the XML
onto remote host
> > > > > and virsh define $domain.xml. This would also change the
*model*.
> > > > >
> > > > > If this would be considered to be guest ABI stable it would mean
that
> > > > > other changes done by using this flag would be wrong because
they also
> > > > > modifies the persistent XML during migration.
> > > >
> > > > I assume there are very good reasons for persistent
> > > > migration to behave differently, but as a user I find
> > > > it extremely surprising. Do you have any insight on the
> > > > rationale behind allowing ABI changes when performing
> > > > persistent migration?
> > >
> > > Well it's a different kind of migration which is almost similar to
the
> > > virsh dumpxml --inactive && virsh define, with one more feature,
migration
> > > hook that can update the XML before it's defined on the destination.
> > >
> > > I agree that it may seem that we should handle this type of migration to
> > > be guest ABI stable, on the other hand this basically defines new domain
> > > on the destination where we allow to do the ABI updates.
> > >
> > > I'm adding Dan to CC to get his opinion about the guest ABI.
> >
> > So, there's no particular reason why things must be different for
> > live vs persistent migration. The only fundamental requirement is
> > that we must never change ABI underneath a running guest, which
> > mandates the strong ABI guarantee for live migration & save/restore.
> >
> > For persistent/offline migration, there's no running guest, so from
> > a technical POV we have more freedom in some sense. That said, a
> > change in guest hardaware that can cause a guest to become unbootable
> > is not a nice thing todo.
> >
> > For this reason, when you use 'defineXML' libvirt aims to immediately
> > fill in (most) XML elements that would affect guest ABI. ie we assign
> > addresses at define time, not start time, and we expand non-versioned
> > machine types into versioned machine types, etc.
> >
> > So, yes, we should try to *not* change ABI across a dumpxml & define
> > operational.
> >
> > When we first introduced PCI device addressing in libvirt, we had an
> > old state where XML contained no addresses, and upon starting in new
> > libvirt we have to assign addresses which might not be the same as
> > those seen with previous libvirt before addressing was recorded. Once
> > the addresses are assigned though, we must never change them.
>
> This is basically the same issue like with the PCI addresses. We used
"-usb"
> instead of "-device ...", but when we switched to using the "-device
..."
> way we didn't record the default model in the XML. The default model
> is recorded in the XML since Libvirt 2.2.0.
Ok, that makes sense then. Until we are able to use -device we can't
gaurantee stability for an arch.
> > - if we recorded 'pci-ohci' in the XML, we must never change that
> > to 'nec-xhci'.
>
> This will never happen.
>
> > - if we didn't record 'pci-ochi' in the XML, we should fill in
the
> > model that is most likely to match what the previous default
> > behaviour was, which would be 'pci-ochi' not 'nec-xhci'
IIUC.
>
> This patch tries to move from 'pci-ohci' to 'nec-xhci' as a default
USB
> controller because the current default model is probably broken, see [1],
> and will never be fixed. In addition I would like to do this change in
> Libvirt so all other tools that uses Libvirt don't have to implement
> this change by themselves.
>
> In this case I think it's better to change the default from 'pci-ohci'
to
> 'nec-xhci' if you define a new domain where the model is not specified.
If I look at qemu-system-ppc64 -usb, that provides an 'nec-xhci' device.
I'm curious why libvirt picked pci-ohci ? Was -usb previously providing
a pci-ohci device instead of nec-xhci ?
We switched from -usb to -device in Libvirt 1.3.1 but we start to record
the default model in XML in Libvirt 2.2.0 (yes it should have been done
in the Libvirt 1.3.1). So probably in the time of Libvirt 1.3.1 the default
in Qemu was pci-ohci but they've probably switched to nex-xhci as they
decided that they will not fix the issues with pci-ohci.
I guess the thing that could justify the switch is that we don't
guarantee
what default devices you'll get for any given guest. It is dependent on
the particular version of the machine type used. So in that sense if you
have an XML doc which is not fully populated with device info, you are
already liable to see changes in default devices based on which QEMU
you happen to define the XML against first.
I agree with that, if you let Libvirt to chose some default you probably
don't care that much what the default will be.
> There is a concern whether we should allow ABI change for
persistent
> migration which is more general issue that will affect some other changes
> that are already "allowed" for persistent migration. I'm starting to
feel
> that we should remove the "VIR_DOMAIN_DEF_PARSE_ABI_UPDATE" from
> "virDomainDefParseNode" call in "qemuMigrationCookieXMLParse"
function
> which currently allows to do ABI changes for persistent migration.
Pavel
12:05 p.m.
New subject: [libvirt] [PATCH 3/3] qemu_domain: use correct default USB controller on ppc64
On Tue, 2017-03-07 at 16:35 +0100, Pavel Hrdina wrote:
> If I look at qemu-system-ppc64 -usb, that provides an
'nec-xhci' device.
>
> I'm curious why libvirt picked pci-ohci ? Was -usb previously providing
> a pci-ohci device instead of nec-xhci ?
We switched from -usb to -device in Libvirt 1.3.1 but we start to record
the default model in XML in Libvirt 2.2.0 (yes it should have been done
in the Libvirt 1.3.1). So probably in the time of Libvirt 1.3.1 the default
in Qemu was pci-ohci but they've probably switched to nex-xhci as they
decided that they will not fix the issues with pci-ohci.
Yup, the switch from pci-ohci to nec-usb-xhci as QEMU default
happened about a year ago for that very reason, see
https://bugzilla.redhat.com/show_bug.cgi?id=1284333
> I guess the thing that could justify the switch is that we
don't guarantee
> what default devices you'll get for any given guest. It is dependent on
> the particular version of the machine type used. So in that sense if you
> have an XML doc which is not fully populated with device info, you are
> already liable to see changes in default devices based on which QEMU
> you happen to define the XML against first.
I agree with that, if you let Libvirt to chose some default you probably
don't care that much what the default will be.
I completely agree with the argument in general.
That said, libvirt should only make the choice *once* and
store it in the guest XML, which is something that releases
between 1.3.1 and 2.1.0 unfortunately neglected to do.
If at all possible, guests created using one of the affected
releases should not be changed at all, even though switching
from pci-ohci to nec-xhci is something that I don't expect
would actually break any guest.
> > There is a concern whether we should allow ABI change for
persistent
> > migration which is more general issue that will affect some other changes
> > that are already "allowed" for persistent migration. I'm
starting to feel
> > that we should remove the "VIR_DOMAIN_DEF_PARSE_ABI_UPDATE" from
> > "virDomainDefParseNode" call in
"qemuMigrationCookieXMLParse" function
> > which currently allows to do ABI changes for persistent migration.
I think we should really think hard about this and possibly
change our current approach.
While it might be true that offline migration doesn't have
quite as restrictive requirements as live migration, that
doesn't mean we should be updating the guest ABI willy-nilly.
--
Andrea Bolognani / Red Hat / Virtualization
2848
days inactive
2853
days old
14 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrange -
Pavel Hrdina