[libvirt] [PATCH] send default USB controller in xml to destination during migration on PPC64

The default USB controller is not sent to destination as the older versions of libvirt(0.9.4 or earlier as I see in commit log of 409b5f54) didn't support them. For some archs where the support started much later can safely send the USB controllers without this worry. For now, changing the checks to allow PPC64 here. If we can track the historical support for other archs, the changes can be extrapolated to them too. The patch fixes a bug that, if the USB controller happens to occupy a slot after disks/interfaces and one of them is hot-unplugged, then the default USB controller added on destination takes the smallest slot number and that would lead to savestate mismatch and migration failure. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9044792..6ecc0e2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2226,7 +2226,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, usb = def->controllers[i]; } } - if (usb && usb->idx == 0 && usb->model == -1) { + if (!ARCH_IS_PPC64(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) { VIR_DEBUG("Removing default USB controller from domain '%s'" " for migration compatibility", def->name); toremove++;

Ping... Can someone review this simple patch ? Thanks, Shiva On Wed, Mar 2, 2016 at 8:39 PM, Shivaprasad G Bhat < shivaprasadbhat@gmail.com> wrote:
The default USB controller is not sent to destination as the older versions of libvirt(0.9.4 or earlier as I see in commit log of 409b5f54) didn't support them. For some archs where the support started much later can safely send the USB controllers without this worry. For now, changing the checks to allow PPC64 here. If we can track the historical support for other archs, the changes can be extrapolated to them too.
The patch fixes a bug that, if the USB controller happens to occupy a slot after disks/interfaces and one of them is hot-unplugged, then the default USB controller added on destination takes the smallest slot number and that would lead to savestate mismatch and migration failure.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9044792..6ecc0e2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2226,7 +2226,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, usb = def->controllers[i]; } } - if (usb && usb->idx == 0 && usb->model == -1) { + if (!ARCH_IS_PPC64(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) { VIR_DEBUG("Removing default USB controller from domain '%s'" " for migration compatibility", def->name); toremove++;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 03/02/2016 10:09 AM, Shivaprasad G Bhat wrote:
The default USB controller is not sent to destination as the older versions of libvirt(0.9.4 or earlier as I see in commit log of 409b5f54) didn't support them. For some archs where the support started much later can safely send the USB controllers without this worry. For now, changing the checks to allow PPC64 here. If we can track the historical support for other archs, the changes can be extrapolated to them too.
The patch fixes a bug that, if the USB controller happens to occupy a slot after disks/interfaces and one of them is hot-unplugged, then the default USB controller added on destination takes the smallest slot number and that would lead to savestate mismatch and migration failure.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9044792..6ecc0e2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2226,7 +2226,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, usb = def->controllers[i]; } } - if (usb && usb->idx == 0 && usb->model == -1) { + if (!ARCH_IS_PPC64(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) { VIR_DEBUG("Removing default USB controller from domain '%s'" " for migration compatibility", def->name); toremove++;
Sorry this didn't receive a timely response. Can you provide the XML config that reproduces this? I wonder if this is something that affects x86 too. And if so we may want to fix it in a different way Thanks, Cole

Thanks Cole.. Here is the XML for a PPC64 guest. https://paste.fedoraproject.org/360574/61826548/ You can see the slot 1 is free as I unplugged a device there. On X86, Q35 seems to use the model ich9-ehci1 by default. So, may not be applicable there. For pc-i440fx, I couldn't force USB to occupy any place other than slot 1 function 2. So, might not be applicable there too. Regards, Shiva On Wed, Apr 27, 2016 at 9:27 PM, Cole Robinson <crobinso@redhat.com> wrote:
On 03/02/2016 10:09 AM, Shivaprasad G Bhat wrote:
The default USB controller is not sent to destination as the older versions of libvirt(0.9.4 or earlier as I see in commit log of 409b5f54) didn't support them. For some archs where the support started much later can safely send the USB controllers without this worry. For now, changing the checks to allow PPC64 here. If we can track the historical support for other archs, the changes can be extrapolated to them too.
The patch fixes a bug that, if the USB controller happens to occupy a slot after disks/interfaces and one of them is hot-unplugged, then the default USB controller added on destination takes the smallest slot number and that would lead to savestate mismatch and migration failure.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9044792..6ecc0e2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2226,7 +2226,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, usb = def->controllers[i]; } } - if (usb && usb->idx == 0 && usb->model == -1) { + if (!ARCH_IS_PPC64(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) { VIR_DEBUG("Removing default USB controller from domain '%s'" " for migration compatibility", def->name); toremove++;
Sorry this didn't receive a timely response. Can you provide the XML config that reproduces this? I wonder if this is something that affects x86 too. And if so we may want to fix it in a different way
Thanks, Cole

On 04/28/2016 03:05 AM, Shivaprasad bhat wrote:
Thanks Cole.. Here is the XML for a PPC64 guest. https://paste.fedoraproject.org/360574/61826548/ You can see the slot 1 is free as I unplugged a device there.
On X86, Q35 seems to use the model ich9-ehci1 by default. So, may not be applicable there. For pc-i440fx, I couldn't force USB to occupy any place other than slot 1 function 2. So, might not be applicable there too.
Thanks for the info. Sounds like we can make the check be !ARCH_IS_X86 then, since 0.9.4 didn't really have actively developed support for anything that isn't x86. Can you send a v2 with that change, and add a comment above the check about what the original purpose was (libvirt <= 0.9.4 migration compat) and that it doesn't apply to non-x86 arch and in fact can cause issues with ppc64 Thanks, Cole
Regards, Shiva
On Wed, Apr 27, 2016 at 9:27 PM, Cole Robinson <crobinso@redhat.com <mailto:crobinso@redhat.com>> wrote:
On 03/02/2016 10:09 AM, Shivaprasad G Bhat wrote: > The default USB controller is not sent to destination as the older versions > of libvirt(0.9.4 or earlier as I see in commit log of 409b5f54) didn't > support them. For some archs where the support started much later can > safely send the USB controllers without this worry. For now, changing the > checks to allow PPC64 here. If we can track the historical support for > other archs, the changes can be extrapolated to them too. > > The patch fixes a bug that, if the USB controller happens to occupy > a slot after disks/interfaces and one of them is hot-unplugged, then > the default USB controller added on destination takes the smallest slot > number and that would lead to savestate mismatch and migration > failure. > > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com <mailto:sbhat@linux.vnet.ibm.com>> > --- > src/qemu/qemu_domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 9044792..6ecc0e2 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -2226,7 +2226,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, > usb = def->controllers[i]; > } > } > - if (usb && usb->idx == 0 && usb->model == -1) { > + if (!ARCH_IS_PPC64(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) { > VIR_DEBUG("Removing default USB controller from domain '%s'" > " for migration compatibility", def->name); > toremove++; >
Sorry this didn't receive a timely response. Can you provide the XML config that reproduces this? I wonder if this is something that affects x86 too. And if so we may want to fix it in a different way
Thanks, Cole
participants (3)
-
Cole Robinson
-
Shivaprasad bhat
-
Shivaprasad G Bhat