Re: [Qemu-devel] [PATCH v3 0/2] block: allow flush on devices with open tray
On 09/16/2016 04:33 AM, Kevin Wolf wrote: Am 16.09.2016 um 01:42 hat John Snow geschrieben: One more try. The move to blk_flush altered the behavior of migration and flushing nodes that are not reachable via the guest, but are still reachable via QEMU and may or may not need to be flushed. This is likely the simplest solution for now until we nail down our policy a bit more. This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt et al being unable to migrate QEMU when the CDROM tray is open. v3: Trying to take a hint from Kevin, reinstating bdrv_flush_all. If it's not what we want, we can try moving back to v2, acknowledging that a nicer solution in the future: (A) Can skip flushing on devices that just don't need it, and (B) Optionally institutes some sort of flush-on-eject policy. Reviewed-by: Kevin WolfThe only part that I'm not completely happy with are the commit messages because there is a theme of "bypassing checks" in there. That's not really what happens because the requests don't come from something attached to the BlockBackends in the first place, and it also completely ignores the case of BDSes that aren't currently attached to any BB and are fixed by these changes as well. I see your point. I just meant to say that it bypasses the checks in contrast to the other interface, but yes, there's a reason we're removing the checks here. Anyway, that's a minor problem as long as the code is right. Let's fix it up. I'll reply with a suggestion. Kevin Thank you.
Re: [Qemu-devel] [PATCH v3 0/2] block: allow flush on devices with open tray
Am 16.09.2016 um 01:42 hat John Snow geschrieben: > One more try. > > The move to blk_flush altered the behavior of migration and flushing > nodes that are not reachable via the guest, but are still reachable > via QEMU and may or may not need to be flushed. > > This is likely the simplest solution for now until we nail down our > policy a bit more. > > This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt > et al being unable to migrate QEMU when the CDROM tray is open. > > v3: > Trying to take a hint from Kevin, reinstating bdrv_flush_all. > If it's not what we want, we can try moving back to v2, > acknowledging that a nicer solution in the future: > (A) Can skip flushing on devices that just don't need it, and > (B) Optionally institutes some sort of flush-on-eject policy. Reviewed-by: Kevin WolfThe only part that I'm not completely happy with are the commit messages because there is a theme of "bypassing checks" in there. That's not really what happens because the requests don't come from something attached to the BlockBackends in the first place, and it also completely ignores the case of BDSes that aren't currently attached to any BB and are fixed by these changes as well. Anyway, that's a minor problem as long as the code is right. Kevin
Re: [Qemu-devel] [PATCH v3 0/2] block: allow flush on devices with open tray
On 16/09/2016 02:09, John Snow wrote: > > > On 09/15/2016 08:06 PM, Paolo Bonzini wrote: >> >> >> On 16/09/2016 01:42, John Snow wrote: >>> One more try. >>> >>> The move to blk_flush altered the behavior of migration and flushing >>> nodes that are not reachable via the guest, but are still reachable >>> via QEMU and may or may not need to be flushed. >>> >>> This is likely the simplest solution for now until we nail down our >>> policy a bit more. >>> >>> This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt >>> et al being unable to migrate QEMU when the CDROM tray is open. >>> >>> v3: >>> Trying to take a hint from Kevin, reinstating bdrv_flush_all. >>> If it's not what we want, we can try moving back to v2, >>> acknowledging that a nicer solution in the future: >>> (A) Can skip flushing on devices that just don't need it, and >>> (B) Optionally institutes some sort of flush-on-eject policy. >> >> If you move flushing from platform_fixed_ioport_writew's blk_flush_all >> to blk_flush calls in pci_piix3_xen_ide_unplug, you can get rid of >> blk_flush_all completely. > > I assume you mean in addition to re-adding bdrv_flush_all for the sake > of vm_stop, we can also get rid of blk_flush_all from xen and avoid the > duplication? Yep. Devices really should not use the *_all() variants. Paolo
Re: [Qemu-devel] [PATCH v3 0/2] block: allow flush on devices with open tray
On 09/15/2016 08:06 PM, Paolo Bonzini wrote: On 16/09/2016 01:42, John Snow wrote: One more try. The move to blk_flush altered the behavior of migration and flushing nodes that are not reachable via the guest, but are still reachable via QEMU and may or may not need to be flushed. This is likely the simplest solution for now until we nail down our policy a bit more. This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt et al being unable to migrate QEMU when the CDROM tray is open. v3: Trying to take a hint from Kevin, reinstating bdrv_flush_all. If it's not what we want, we can try moving back to v2, acknowledging that a nicer solution in the future: (A) Can skip flushing on devices that just don't need it, and (B) Optionally institutes some sort of flush-on-eject policy. If you move flushing from platform_fixed_ioport_writew's blk_flush_all to blk_flush calls in pci_piix3_xen_ide_unplug, you can get rid of blk_flush_all completely. Paolo I assume you mean in addition to re-adding bdrv_flush_all for the sake of vm_stop, we can also get rid of blk_flush_all from xen and avoid the duplication? I can follow up with that if Max and Kevin agree to what I've already posted. --js
Re: [Qemu-devel] [PATCH v3 0/2] block: allow flush on devices with open tray
On 16/09/2016 01:42, John Snow wrote: > One more try. > > The move to blk_flush altered the behavior of migration and flushing > nodes that are not reachable via the guest, but are still reachable > via QEMU and may or may not need to be flushed. > > This is likely the simplest solution for now until we nail down our > policy a bit more. > > This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt > et al being unable to migrate QEMU when the CDROM tray is open. > > v3: > Trying to take a hint from Kevin, reinstating bdrv_flush_all. > If it's not what we want, we can try moving back to v2, > acknowledging that a nicer solution in the future: > (A) Can skip flushing on devices that just don't need it, and > (B) Optionally institutes some sort of flush-on-eject policy. If you move flushing from platform_fixed_ioport_writew's blk_flush_all to blk_flush calls in pci_piix3_xen_ide_unplug, you can get rid of blk_flush_all completely. Paolo