New subject: [libvirt] [PATCH 1/9] tests: qemuxml2argv: Remove useless GIC flags

29 Jul 2016

On Mon, 2016-08-08 at 18:30 +0200, Ján Tomko wrote:
> > I think we still want to make a last-ditch effort to provide
> > some sort, any sort, of USB controller. Or rather, that we
> > have to. Honestly, I'd rather just drop the -usb handling
> > altogether and expect users not to mix modern-day libvirt
> > with QEMU versions from 5+ years ago.
> 
> It seems QEMU_CAPS_PIIX3_USB_UHCI was introduced by this QEMU commit:
> 
> commit 556cd09885bec3f69ba78228fe4e46dc1dad145b
> Author:     Markus Armbruster <armbru@redhat.com>
> AuthorDate: 2009-12-09 17:07:53 +0100
> Commit:     Anthony Liguori <aliguori@us.ibm.com>
> CommitDate: 2009-12-12 07:59:38 -0600
> 
>     qdev: Replace device names containing whitespace
> 
>     Device names with whitespace require quoting in the shell and in the
>     monitor.  Some of the offenders are also overly long.  Some have a
>     more convenient alias, some don't.
> 
>     The place for verbose device names is DeviceInfo member desc.  The
>     name should be short & sweet.
> 
>     Signed-off-by: Markus Armbruster <armbru@redhat.com>
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> contains: v0.13.0-rc0~2041
> 
> While I think we could safely drop support for that for x86_64,
> I would prefer if it would be accompanied with an exact version that is
> dropped, just like we did with 0.12.0. That way we could possibly drop
> more code.

0.12.0 was chosen as our cut-off version because it aligns
with currently-supported enterprise Linux releases, eg.
RHEL/CentOS 6, Ubuntu 12.04 and SLES 11. I don't think we
should drop newer versions while the OS vendors keep
supporting them. So we'll be stuck with 0.12.0 for a while :)

> But this is also executed for non-x86 architectures.
> We either do or do not add the <controller>, but emit -usb anyway.
> This could be broken by patch 9/9.
> 
> > > Of course, this breaks migration to pre-0.9.x libvirt which did not
> > > support USB controllers. I am okay with that, but you might want to get
> > > a second opinion on that.
> > 
> > How far back can you reasonably expect to migrate a guest?
> 
> I would expect us not to break it if we can avoid it.
> 
> Since we now drop the controller only for model =-1 and i440fx machine,
> changing the condition to also accept the i440fx model would allow us
> to migrate from newest libvirt to ancient libvirt not supporting USB
> controllers. When migrating back, we would auto-add the correct
> model since it happens at parse time and ABIStabilityCheck would be
> happy.
> 
> Please extend that condition to include PIIX3 model. If we ever want
> to drop it, we can just delete the whole thing from qemuDomainDefFormatBuf.

Okay, that would work since we make sure that piix3-usb-uhci
is always assigned a PCI address of 00:01.2 and -usb will use
that address.

> > How
> > would you even migrate a guest that uses USB controllers to a
> > version of libvirt that doesn't support USB controllers?
> 
> We were autoadding -usb to the command line even if libvirt did not
> support <controller type='usb'>. So passing it in XML unconditionally
> fails when being parsed on the destination, but if it's dropped, the
> destination libvirt just assumes it's there and emits -usb. (might even
> be -device .., I did not look back that far).
> 
> See:
> commit 409b5f549530e7b3a33f4505f2cad2e26896107c
>     qemu: Emit compatible XML when migrating a domain
> contains: v0.9.12-rc2~22
> 
> So, ACK to this patch if you either:
> * extend qemuDomainDefFormatBuf to also drop PIIX3 from migratable XML

I see. I will do that.

> * or get someone else to say: "Let's break this!"
> 
> For 9/9 I think we should not error out on controller model -1 just yet,
> since we do not pick it on XML parsing in all cases.

I think you're referring to this chunk:

@@ -2396,10 +2395,9 @@ qemuBuildUSBControllerDevStr(const virDomainDef *domainDef,
     model = def->model;
 
     if (model == -1) {
-        if ARCH_IS_PPC64(domainDef->os.arch)
-            model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
-        else
-            model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       "%s", _("no model provided for USB controller"));
+        return -1;
     }

Note that the previous code didn't really add anything but
complexity: immediately afterwards, the QEMU capabilities
are checked to see whether pci-ohci or piix3-usb-uhci are
available - but if they are, then we'd already have set the
model accordingly in qemuDomainDeviceDefPostParse().

I'll push both a fixed 8/9 and 9/9 in a while.

-- 
Andrea Bolognani / Red Hat / Virtualization



[libvirt] [PATCH 0/9] qemu: Reflect USB controller model in guest XML

Andrea Bolognani

Andrea Bolognani

Andrea Bolognani

Andrea Bolognani

Ján Tomko

Andrea Bolognani

Andrea Bolognani

Ján Tomko

Andrea Bolognani

Ján Tomko

Andrea Bolognani

Andrea Bolognani

Ján Tomko

Andrea Bolognani

Ján Tomko

Andrea Bolognani

Ján Tomko

Andrea Bolognani

Andrea Bolognani

Ján Tomko

tags

participants (2)