<div dir="ltr">Thanks Jan, Laine for the comments. I'll send the next version with checks to<div>leave out the default USB only for i440fx.</div><div><br></div><div>Regards,</div><div>Shivaprasad</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 3, 2016 at 1:30 PM, Ján Tomko <span dir="ltr"><<a href="mailto:jtomko@redhat.com" target="_blank">jtomko@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, May 02, 2016 at 02:21:41PM -0400, Laine Stump wrote:<br>
> On 05/02/2016 10:41 AM, Ján Tomko wrote:<br>
> > On Fri, Apr 29, 2016 at 01:09:26PM -0400, Cole Robinson wrote:<br>
> >> On 04/29/2016 10:01 AM, Shivaprasad G Bhat wrote:<br>
> >>> The default USB controller is not sent to destination as the older versions<br>
> >>> of libvirt(0.9.4 or earlier as I see in commit log of 409b5f54) didn't<br>
> >>> support them. For some archs where the support started much later can<br>
> >>> safely send the USB controllers without this worry. So, send the controller<br>
> >>> to destination for all archs except x86. Moreover this is not very applicable<br>
> >>> to x86 as the USB controller has model ich9_ehci1 on q35 and for pc-i440fx,<br>
> >>> there cant be any slots before USB as it is fixed on slot 1.<br>
> >>><br>
> >>> The patch fixes a bug that, if the USB controller happens to occupy<br>
> >>> a slot after disks/interfaces and one of them is hot-unplugged, then<br>
> >>> the default USB controller added on destination takes the smallest slot<br>
> >>> number and that would lead to savestate mismatch and migration<br>
> >>> failure. Seen and verified on PPC64.<br>
> >>><br>
> >>> Signed-off-by: Shivaprasad G Bhat <<a href="mailto:sbhat@linux.vnet.ibm.com">sbhat@linux.vnet.ibm.com</a>><br>
> >>> ---<br>
> >>>   src/qemu/qemu_domain.c |    6 +++++-<br>
> >>>   1 file changed, 5 insertions(+), 1 deletion(-)<br>
> >>><br>
> >>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c<br>
> >>> index 6262bfe..963ff35 100644<br>
> >>> --- a/src/qemu/qemu_domain.c<br>
> >>> +++ b/src/qemu/qemu_domain.c<br>
> >>> @@ -2599,7 +2599,11 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,<br>
> >>>                   usb = def->controllers[i];<br>
> >>>               }<br>
> >>>           }<br>
> >>> -        if (usb && usb->idx == 0 && usb->model == -1) {<br>
> >>> +        /*  The original purpose of the check was the migration compatibility<br>
> >>> +         *  with libvirt <= 0.9.4. Limitation doesn't apply to other archs<br>
> >>> +         *  and can cause problems on PPC64.<br>
> >>> +         *<br>
> >>> +        if (ARCH_IS_X86(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) {<br>
> > As of commit fcbfd50:<br>
> >      qemu: only check for PIIX3-specific device addrs on pc-* machinetypes<br>
> > we only check that the controller is on slot 1 for PIIX3 machines,<br>
> > so qemuDomainMachineIsI440FX(def) should also be one of the conditions.<br>
> ><br>
> > It is possible to specify a usb controller with model='-1' on q35 too,<br>
> > but I do not know if anyone actually uses it in the wild.<br>
><br>
><br>
> I have a vague memory of searching through for the possible ways that<br>
> model could be -1 and coming up with the answer "almost never". As for<br>
> Q35 specifically - support was added for PCIe controllers required by<br>
> Q35 in libvirt 1.1.2, so any configuration for a Q35 domain wouldn't<br>
> work if you tried to move it to a system with a libvirt old enough to<br>
> require removing the usb controller from the XML.<br>
><br>
<br>
</div></div>Yes, that's is why I suggested to just check for the machine type.<br>
For i440fx, we can safely leave it out because it was always at slot 1.<br>
For q35, it could be on any other slot and leaving it out does not give<br>
us any benefit.<br>
For isapc, we stopped adding the default USB in commit c66da9d22.<br>
<span class=""><br>
<br>
> This removal was useful at the time it was added, but at this point, do<br>
> we really expect that anyone is going to be successful migrating a<br>
> running virtual machine from a system running libvirt 1.3.4 to a system<br>
> with libvirt that is 0.9.4 or less (or downgrading their libvirt by that<br>
> many revisions)? Is there a use for doing it at all?<br>
<br>
</span>Probably not. It might not work at all. Should we break it on<br>
purpose?<br>
<br>
The oldest QEMU version we support is 0.12.0.<br>
0.12.0-rc2 was released in Dec 2009.<br>
Libvirt 0.9.4 was released in August 2011, that's about 20 months later<br>
(and 4 years 9 months ago.)<br>
<br>
If we want to start breaking backwards compatibility, I would rather<br>
not break libvirt newer than the oldest supposed QEMU. Also, it would<br>
deserve a separate discussion thread with Daniel and Daniel.<br>
<br>
Jan<br>
</blockquote></div><br></div>