[libvirt] [PATCH 0/2] Fix a couple of virtio-scsi iothread issues
See the patches for details John Ferlan (2): qemu: Fix error for attach controller config for live guest qemu: Allow no address to be defined for virtio-scsi iothrad attach src/qemu/qemu_domain.c | 7 ++++--- src/qemu/qemu_driver.c | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) -- 2.14.4
Fix the error message to indicate what exactly is failing - that the controller index provided matches an existing controller. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 38ea865ce3..50a7de4598 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8039,8 +8039,9 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, if (controller->idx != -1 && virDomainControllerFind(vmdef, controller->type, controller->idx) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Target already exists")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("controller index='%d' already exists"), + controller->idx); return -1; } -- 2.14.4
On Wed, Jun 06, 2018 at 08:21:25PM -0400, John Ferlan wrote:
Fix the error message to indicate what exactly is failing - that the controller index provided matches an existing controller.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
https://bugzilla.redhat.com/show_bug.cgi?id=1583623 When attaching a virtio-scsi with IOThreads for the config of a live domain, allow the <address> to not be defined thus allowing post parse processing to fill in the address. Make the error a bit clearer that virtio-scsi IOThreads require virtio pci or ccw controller address types. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1fb1ef1deb..97f91ec593 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4736,11 +4736,12 @@ qemuDomainCheckSCSIControllerIOThreads(const virDomainControllerDef *controller, if (!controller->iothread) return true; - if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads only available for virtio pci and " - "virtio ccw controllers")); + _("virtio-scsi IOThreads only available for virtio " + "pci and virtio ccw controllers")); return false; } -- 2.14.4
On Wed, Jun 06, 2018 at 08:21:26PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1583623
When attaching a virtio-scsi with IOThreads for the config of a live domain, allow the <address> to not be defined thus allowing post parse processing to fill in the address.
Make the error a bit clearer that virtio-scsi IOThreads require virtio pci or ccw controller address types.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Typo in commit message. s/iothrad/iothread Katerina
On Wed, Jun 06, 2018 at 08:21:26PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1583623
When attaching a virtio-scsi with IOThreads for the config of a live domain, allow the <address> to not be defined thus allowing post parse processing to fill in the address.
Make the error a bit clearer that virtio-scsi IOThreads require virtio pci or ccw controller address types.
So two different changes.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1fb1ef1deb..97f91ec593 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4736,11 +4736,12 @@ qemuDomainCheckSCSIControllerIOThreads(const virDomainControllerDef *controller, if (!controller->iothread) return true;
- if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
So this check relies on the assignAddresses callback being called before validation, which is only done after parsing the domain, not parsing the individual device. Also, it looks like this change would let the user bypass the check for S390 "address" type by not specifying an address, but that's a corner case. Maybe the proper fix would be to stop relying on postParse in attach config - especially since sometimes we need to make sure the device is suitable for both the persistent definition and the live definition. But in the meantime, this should do. If you split the functional change from the error message change and fix the typo pointed out by Katerina: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads only available for virtio pci and " - "virtio ccw controllers")); + _("virtio-scsi IOThreads only available for virtio " + "pci and virtio ccw controllers")); return false; }
-- 2.14.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
John Ferlan -
Ján Tomko -
Katerina Koukiou